Skip to content

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.

Sign up for GitHub

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

Merged
amoghrajesh merged 1 commit into from
Nov 23, 2025
Merged

Eliminate race condition in a test due to xdist runs #58593

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion dev/breeze/tests/test_packages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,23 @@ def test_apply_version_suffix_to_provider_pyproject_toml(provider_id, version_su
AIRFLOWCTL_INIT_PY = AIRFLOW_ROOT_PATH / "airflow-ctl" / "src" / "airflowctl" / "__init__.py"


@pytest.fixture
Copy link
Member

@potiuk potiuk Nov 23, 2025 •
edited
Loading

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.

Copy link
Contributor Author

@amoghrajesh amoghrajesh Nov 24, 2025

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.

Copy link
Contributor Author

@amoghrajesh amoghrajesh Nov 24, 2025

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

def restore_version_files():
# save contents of all version files before test starts
version_files = [
AIRFLOW_CORE_INIT_PY,
TASK_SDK_INIT_PY,
AIRFLOWCTL_INIT_PY,
]
original_contents = {f: f.read_text() for f in version_files if f.exists()}

yield

# restore original contents after test
for fp, content in original_contents.items():
fp.write_text(content)


@pytest.mark.parametrize(
("distributions", "init_file_path", "version_suffix", "floored_version_suffix"),
[
Expand Down Expand Up @@ -428,7 +445,11 @@ def test_apply_version_suffix_to_provider_pyproject_toml(provider_id, version_su
],
)
def test_apply_version_suffix_to_non_provider_pyproject_tomls(
distributions: tuple[str, ...], init_file_path: Path, version_suffix: str, floored_version_suffix: str
restore_version_files,
distributions: tuple[str, ...],
init_file_path: Path,
version_suffix: str,
floored_version_suffix: str,
):
"""
Test the apply_version_suffix function with different version suffixes for pyproject.toml of non-provider.
Expand Down
Loading