Skip to content

Commit

Permalink
CI - deflake Isolated pytest Ubuntu (#6603)
Browse files Browse the repository at this point in the history
Problem: `test_isolated_packages.py` is still flaky and can fail on
parallel builds of a local `cirq_core` wheel.

Solution: Use per-worker copy of the cirq-core sources so that parallel
builds do not have conflicting build files.

Follow-up to #6593
  • Loading branch information
pavoljuhas authored May 16, 2024
1 parent cf86dda commit bf67f29
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 6 deletions.
2 changes: 1 addition & 1 deletion dev_tools/notebooks/isolated_notebook_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
"papermill",
"jupyter",
# assumed to be part of colab
"seaborn~=0.11.1",
"seaborn~=0.12",
]


Expand Down
14 changes: 12 additions & 2 deletions dev_tools/notebooks/notebook_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import pytest

from dev_tools import shell_tools
from dev_tools.modules import list_modules
from dev_tools.notebooks import filter_notebooks, list_all_notebooks, rewrite_notebook
from dev_tools.test_utils import only_on_posix

Expand Down Expand Up @@ -63,9 +64,18 @@ def require_packages_not_changed():
Raise AssertionError if the pre-existing set of Python packages changes in any way.
"""
packages_before = set((d.name, d.version) for d in importlib.metadata.distributions())
cirq_packages = set(m.name for m in list_modules()).union(["cirq"])
packages_before = set(
(d.name, d.version)
for d in importlib.metadata.distributions()
if d.name not in cirq_packages
)
yield
packages_after = set((d.name, d.version) for d in importlib.metadata.distributions())
packages_after = set(
(d.name, d.version)
for d in importlib.metadata.distributions()
if d.name not in cirq_packages
)
assert packages_after == packages_before


Expand Down
12 changes: 10 additions & 2 deletions dev_tools/packaging/isolated_packages_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,22 @@
# the "isolation" fails and for example cirq-core would be on the PATH
@mock.patch.dict(os.environ, {"PYTHONPATH": ""})
@pytest.mark.parametrize('module', list_modules(), ids=[m.name for m in list_modules()])
def test_isolated_packages(cloned_env, module):
def test_isolated_packages(cloned_env, module, tmp_path):
env = cloned_env("isolated_packages", *PACKAGES)

if str(module.root) != "cirq-core":
assert f'cirq-core=={module.version}' in module.install_requires

# TODO: Remove after upgrading package builds from setup.py to PEP-517
# Create per-worker copy of cirq-core sources so that parallel builds
# of cirq-core wheel do not conflict.
opt_cirq_core = (
[str(shutil.copytree("./cirq-core", tmp_path / "cirq-core"))]
if str(module.root) != "cirq-core"
else []
)
result = shell_tools.run(
f"{env}/bin/pip install --no-clean ./{module.root} ./cirq-core".split(),
[f"{env}/bin/pip", "install", f"./{module.root}", *opt_cirq_core],
stderr=subprocess.PIPE,
check=False,
)
Expand Down
2 changes: 1 addition & 1 deletion dev_tools/requirements/deps/notebook.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ papermill~=2.3.2
-r ../../../cirq-core/cirq/contrib/requirements.txt

# assumed to be part of colab
seaborn~=0.11.1
seaborn~=0.12

0 comments on commit bf67f29

Please sign in to comment.