Add lint flags to upgrade-test command#1829
Merged
jeremybeard merged 1 commit intomainfrom Apr 8, 2025
Merged
Conversation
jeremybeard
requested review from
kushalmalani,
lzdanski and
neel-astro
as code owners
neel-astro
approved these changes
Apr 8, 2025
| cmd.Flags().BoolVarP(&versionTest, "version-test", "", false, "Only run version tests. These tests show you how the versions of your dependencies will change after you upgrade.") | ||
| cmd.Flags().BoolVarP(&dagTest, "dag-test", "d", false, "Only run DAG tests. These tests check whether your DAGs will generate import errors after you upgrade.") | ||
| cmd.Flags().BoolVarP(&ruffTest, "ruff-test", "r", false, "Only run ruff tests. These tests check whether your DAGs are compatible with Airflow 3.") | ||
| cmd.Flags().BoolVarP(&lintTest, "lint-test", "l", false, "Only run ruff lint tests. These tests check whether your DAGs are compatible with Airflow.") |
Contributor
There was a problem hiding this comment.
Suggested change
| cmd.Flags().BoolVarP(&lintTest, "lint-test", "l", false, "Only run ruff lint tests. These tests check whether your DAGs are compatible with Airflow.") | |
| cmd.Flags().BoolVarP(&lintTest, "lint-test", "l", false, "Run ruff lint tests. These tests check whether your DAGs are compatible with Airflow.") |
Sorry I just noticed it now, but I think the flag is set to true would ensure that the lint tests are run, and won't ensure that other tests don't run.
I think we should also update the description for dag-test and version-text flag, wdyt?
Contributor
Author
There was a problem hiding this comment.
The way it works (not defending it...) is that if all of the *-test flags are false then they all run, otherwise it is only the flags that are set to true. So "only run" is not quite right if you set more than one of them to true, but I wouldn't want to imply one only runs if you set its flag. Bit of a UX headache but being existing behavior I'd rather just leave it as is.
neel-astro
pushed a commit
that referenced
this pull request
Apr 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This changes the recently added ruff tests of the
astro dev upgrade-testcommand:AIR30rules)--lint-deprecationsalso checks for deprecations in Airflow 3 (AIR31rules, which do not yet exist but will very soon)--lint-config-filewhich overrides the ruff config entirely with a local file🧪 Functional Testing
📋 Checklist
make testbefore taking out of draftmake lintbefore taking out of draft