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

Fix testing tests command to allow passing test as extra arg #43529

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Oct 30, 2024

When providers have been moved in #42505 broke passing parameters when provider tests were passed as extra args of "testing tests" command of breeze.

Previously all tests were under "tests" folder and there was an exclusion to disable "All" tests when any test was passed as parameter. But after moving tests to "providers" this stopped working.

Additional exclusion needs to be added for "providers/tests" and "providers/tests_sdk/".

This PR also adds autocompletion for tests passed this way by setting the click type to Path for the extra args (but without the need for the Path to exist).

Also during this check it turned out that "All" tests are not working in the intended way - but this should not impact our CI only local runs.

Appropriate comment has been added and it's captured in #42632


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk potiuk requested a review from o-nikolas October 30, 2024 23:42
@potiuk potiuk mentioned this pull request Oct 30, 2024
@potiuk potiuk requested a review from kaxil October 30, 2024 23:44
When providers have been moved in apache#42505 broke passing parameters
when provider tests were passed as extra args of "testing tests"
command of breeze.

Previously all tests were under "tests" folder and there was
an exclusion to disable "All" tests when any test was passed as
parameter. But after moving tests to "providers" this stopped working.

Additional exclusion needs to be added for "providers/tests" and
"providers/tests_sdk/".

This PR also adds autocompletion for tests passed this way by
setting the click type to Path for the extra args (but without
the need for the Path to exist).

Also during this check it turned out that "All" tests are not
working in the intended way - but this should not impact our CI
only local runs.

Appropriate comment has been added and it's captured in
apache#42632
@potiuk potiuk force-pushed the fix-specifying-tests-in-testing-tests-command branch from 08b3fe0 to 756ffb9 Compare October 31, 2024 08:34
@potiuk potiuk requested review from eladkal and o-nikolas October 31, 2024 08:34
@potiuk potiuk merged commit b3e902d into apache:main Oct 31, 2024
77 checks passed
@potiuk potiuk deleted the fix-specifying-tests-in-testing-tests-command branch October 31, 2024 09:17
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
…43529)

When providers have been moved in apache#42505 broke passing parameters
when provider tests were passed as extra args of "testing tests"
command of breeze.

Previously all tests were under "tests" folder and there was
an exclusion to disable "All" tests when any test was passed as
parameter. But after moving tests to "providers" this stopped working.

Additional exclusion needs to be added for "providers/tests" and
"providers/tests_sdk/".

This PR also adds autocompletion for tests passed this way by
setting the click type to Path for the extra args (but without
the need for the Path to exist).

Also during this check it turned out that "All" tests are not
working in the intended way - but this should not impact our CI
only local runs.

Appropriate comment has been added and it's captured in
apache#42632
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants