Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add @FlywayTarget annotation to migration tests to control flyway upg… #2035

Merged
merged 2 commits into from
Jul 18, 2022

Conversation

collado-mike
Copy link
Collaborator

Problem

Changes in the database schema can break the Java migration scripts if all flyway migrations are applied prior to running unit tests. This allows tests to specify fixed migration versions so that no future migrations are applied for that test. Also updated backfill tests to stop relying on sql in DAOs since they change with future migrations.

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, depending on the change, this may not be necessary)
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

@collado-mike collado-mike requested a review from wslulciuc July 14, 2022 18:47
@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #2035 (2eb2bf1) into main (610f18a) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #2035      +/-   ##
============================================
- Coverage     78.92%   78.83%   -0.09%     
+ Complexity     1018     1015       -3     
============================================
  Files           199      199              
  Lines          5557     5557              
  Branches        421      421              
============================================
- Hits           4386     4381       -5     
- Misses          724      726       +2     
- Partials        447      450       +3     
Impacted Files Coverage Δ
...b/migrations/V44_2__BackfillAirflowParentRuns.java 87.27% <ø> (ø)
...pi/src/main/java/marquez/db/mappers/JobMapper.java 66.66% <0.00%> (-12.13%) ⬇️
api/src/main/java/marquez/service/models/Job.java 70.00% <0.00%> (-3.34%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

…rades

Updated backfill tests to fix flyway migration version and update to stop relying on sql in DAOs since they change with future migrations

Signed-off-by: Michael Collado <collado.mike@gmail.com>
Signed-off-by: Michael Collado <collado.mike@gmail.com>
@collado-mike collado-mike force-pushed the flyway_fixed_migration branch from 977a203 to 2eb2bf1 Compare July 18, 2022 22:46
Copy link
Member

@wslulciuc wslulciuc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@wslulciuc wslulciuc merged commit ccbdb96 into main Jul 18, 2022
@wslulciuc wslulciuc deleted the flyway_fixed_migration branch July 18, 2022 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants