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 pytest db_test markers to our tests #35264

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Oct 30, 2023

This PR is separated out from #35160 in order to make the big refactor and splitting out our test harness to DB and Non-DB tests far more easy to review.

This change contains purely adding db_tests markers to the tests that need them, and as such it requires very litte effort to review. Once this one is merged hoever, the #35160 will become way smaller in terms of number of files to review, which will make it far easier to review using GitHub Interface.


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

This PR is separated out from apache#35160 in order to make the big
refactor and splitting out our test harness to DB and Non-DB
tests far more easy to review.

This change contains purely adding db_tests markers to the tests
that need them, and as such it requires very litte effort to
review. Once this one is merged hoever, the apache#35160 will become
way smaller in terms of number of files to review, which will
make it far easier to review using GitHub Interface.
@potiuk
Copy link
Member Author

potiuk commented Oct 30, 2023

This one should ONLY contain adding db_test markers, so it should be super-easy to review even locally with git diff - once this is merged, the #35160 should start becoming reviewable.

@potiuk
Copy link
Member Author

potiuk commented Oct 30, 2023

This one should be NO-OP for the current code - i.e. the db_test markers are added but do nothing.

@potiuk
Copy link
Member Author

potiuk commented Oct 30, 2023

Would love to merge it to make #35160 look reviewable (and on a good path to get back with stability and improve speed of the CI overall) :D

@potiuk
Copy link
Member Author

potiuk commented Oct 30, 2023

BTW. The easiest way to review it is to do git diff HEAD^ after locally checking out :) ... you will see that there are only markers added

@potiuk
Copy link
Member Author

potiuk commented Oct 30, 2023

My magic script to review that one in a few seconds:

git diff HEAD^ | grep '^[+-][^+-]' | grep -v "import pytest" | grep -v "pytest.mark.db_test"

@potiuk potiuk merged commit 1aa91a4 into apache:main Oct 30, 2023
49 checks passed
@potiuk potiuk deleted the add-db-test-markers branch October 30, 2023 15:51
@potiuk potiuk added this to the Airflow 2.7.3 milestone Nov 1, 2023
@potiuk potiuk added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 1, 2023
ephraimbuddy pushed a commit that referenced this pull request Nov 1, 2023
This PR is separated out from #35160 in order to make the big
refactor and splitting out our test harness to DB and Non-DB
tests far more easy to review.

This change contains purely adding db_tests markers to the tests
that need them, and as such it requires very litte effort to
review. Once this one is merged hoever, the #35160 will become
way smaller in terms of number of files to review, which will
make it far easier to review using GitHub Interface.

(cherry picked from commit 1aa91a4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants