-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add Pre-commit check for airflowctl tests #58856
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
Conversation
bugraoz93
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks, Steve! One comment related to assets, what is the blocker for those?
|
Thanks, just added the fix and went through the logs of the commands again, assets materialize fails with id=1 I believe due to collision with asset id 1. I've ended up adding and removing more commands that were either silently failing or passed when tested. An improvement to the integration tests will give us better accuracy in the future imo. |
|
Still some issues :) |
Backport failed to create: v3-1-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 80ca9f5 v3-1-testThis should apply the commit to the v3-1-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
* pre-commit adds airflowctl int check * cleanup * remove asset materialize & others * dags update param, mege conflict, cicd error
|
Amazing, thanks Steve! |
* pre-commit adds airflowctl int check * cleanup * remove asset materialize & others * dags update param, mege conflict, cicd error
* pre-commit adds airflowctl int check * cleanup * remove asset materialize & others * dags update param, mege conflict, cicd error
Followup
New CLI commands can be added to airflowctl/api/operations.py without corresponding integration test coverage that can lead to a drift in untested commands.
Solution
Add a pre-commit hook that statically analyzes all test-able CLI commands to be included in test coverage. The hook fails if any command is missing coverage and not explicitly listed in the excluded commands list
Stats:
Related Issues