-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Eliminate race condition in a test due to xdist runs #58593
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
…_pyproject_tomls due to xdist runs
…non_provider_pyproject_tomls due to xdist runs (apache#58593) (cherry picked from commit 632bfe7) Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
|
Nice! Gooooooood Catch ! |
| AIRFLOWCTL_INIT_PY = AIRFLOW_ROOT_PATH / "airflow-ctl" / "src" / "airflowctl" / "__init__.py" | ||
|
|
||
|
|
||
| @pytest.fixture |
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.
Wait. .... But would not that break things in the other direction?
I can imagine that two tests that expect those versions to be updated might also be broken by this - because fixture wll FIX the versions if the race condition happens?
I think we need to either have some lock for those changes or to avoid in-place modifications - maybe just mock the "read" method for the tests reading the version rather than modify the files.
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.
Good call, thanks for sharing that perspective too. If more tests expect this, it might fail somewhere else, let me take a stab at making. this portion better to avoid such issues.
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.
Alright, better fix here: #58608
…_pyproject_tomls due to xdist runs (apache#58593)
…_pyproject_tomls due to xdist runs (apache#58593)
Saw errors like this in one of the CI runs:
Upon investigating, it seems that this is because of a race condition occurring due to parallel run of tests using pytest xdist.
This is what happens:
3.2.0.dev0instead of3.2.0and because the validation doesn't handle that, this fails.^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.