Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Feb 13, 2023

The change #29449 made a distinction between "all" and "available" tests and it turned out that "All" test type uses "available" tests. The name "all" is actually wrong now. It should be renamed to be "all-with-tests" or similar, but this should be done in a separate PR


^ 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.

The change apache#29449 made a distinction between "all" and "available"
tests and it turned out that "All" test type uses "available" tests.
The name "all" is actually wrong now. It should be renamed to be
"all-with-tests" or similar, but this should be done in a separate PR
@potiuk
Copy link
Member Author

potiuk commented Feb 13, 2023

cc: @ferruzzi

@potiuk potiuk merged commit 72c3817 into apache:main Feb 13, 2023
@potiuk potiuk deleted the fix-all-integration-to-run-all-subset branch February 13, 2023 23:10
@ferruzzi
Copy link
Contributor

Sorry about that,I didn't run into it in my testing. Is there a gap in the testing I should plug?

I'll take renaming the variable in another PR either today or tomorrow.

@potiuk
Copy link
Member Author

potiuk commented Feb 13, 2023

Sorry about that,I didn't run into it in my testing. Is there a gap in the testing I should plug?

No problem. that's a bnit of complexity of commiter vs. non-commiter run. They are optimized differently.

I'll take renaming the variable in another PR either today or tomorrow.

I am just about to create a PR :)

@ferruzzi
Copy link
Contributor

I am just about to create a PR :)

Well fine then 😛 👍

potiuk added a commit to potiuk/airflow that referenced this pull request Feb 16, 2023
The "all" alias outgrew original meaning with adding statsd integration
in apache#29449 which resulted in a problem fixed by apache#29517. The "all" tests
as result was not "all" but "all that could be tested".

This PR adds "all-testable" alias which as opposed to "all" does not
contain "statsd" - it also renames some of the vars/arrays to reflect
it.7
potiuk added a commit that referenced this pull request Feb 17, 2023
The "all" alias outgrew original meaning with adding statsd integration
in #29449 which resulted in a problem fixed by #29517. The "all" tests
as result was not "all" but "all that could be tested".

This PR adds "all-testable" alias which as opposed to "all" does not
contain "statsd" - it also renames some of the vars/arrays to reflect
it.7
ephraimbuddy pushed a commit that referenced this pull request Mar 8, 2023
The change #29449 made a distinction between "all" and "available"
tests and it turned out that "All" test type uses "available" tests.
The name "all" is actually wrong now. It should be renamed to be
"all-with-tests" or similar, but this should be done in a separate PR

(cherry picked from commit 72c3817)
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.

4 participants