Skip to content

Update CI MacOS to fix notebook test error #6366

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
merged 10 commits into from
Dec 2, 2023
+5 −3
Merged

Update CI MacOS to fix notebook test error #6366

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e1e2f5c
Update CI MacOS to fix notebook test error
smburdick Nov 30, 2023
d292ed6
Remove slow tag from notebook test for PR testing.
smburdick Dec 1, 2023
7cc2917
Merge branch 'main' into notebook-test-fix-macos
smburdick Dec 1, 2023
f118fd6
Update CI daily macos to address failing integ test
smburdick Dec 1, 2023
2accbbe
move quimb import to notebook requirements
smburdick Dec 1, 2023
4306fcc
Restore slow mark to notebook test.
smburdick Dec 1, 2023
79adf36
Merge branch 'main' into notebook-test-fix-macos
smburdick Dec 1, 2023
00a5722
retrigger notebook check
smburdick Dec 1, 2023
388b5fa
Change to only on posix
smburdick Dec 2, 2023
77ecc83
remove sys import
smburdick Dec 2, 2023
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
3 changes: 2 additions & 1 deletion .github/workflows/ci-daily.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ jobs:
pip install \
-r dev_tools/requirements/deps/format.txt \
-r dev_tools/requirements/deps/pylint.txt \
-r dev_tools/requirements/deps/pytest.txt
-r dev_tools/requirements/deps/pytest.txt \
-r dev_tools/requirements/deps/notebook.txt
- name: Pytest check
run: check/pytest -n auto --ignore=cirq-core/cirq/contrib --enable-slow-tests
2 changes: 1 addition & 1 deletion dev_tools/notebooks/notebook_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def env_with_temporary_pip_target():


@pytest.mark.slow
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we temporarily comment out this slow mark to include the test in this PR checks?

If the checks pass, let's put the mark back and merge.

Copy link
Contributor Author

@smburdick smburdick Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what you mean by check the CI here? I thought the PR checks were handling that, but I could be mistaken

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If test is decorated with @pytest.mark.slow it is not included in the CI checks executed for the PR and instead runs in scheduled daily workflows. Removing/commenting the slow mark will run the test in PR checks and we'll know if the fix worked sooner.

@pytest.mark.skipif(sys.platform != "linux", reason="Linux-only test")
@pytest.mark.skipif(sys.platform not in ["linux", "darwin"], reason="Linux/Mac-only test")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@pytest.mark.skipif(sys.platform not in ["linux", "darwin"], reason="Linux/Mac-only test")
@only_on_posix

please reuse the existing decorator from dev_tools.test_utils import only_on_posix
Otherwise LGTM, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@pytest.mark.parametrize("notebook_path", filter_notebooks(list_all_notebooks(), SKIP_NOTEBOOKS))
def test_notebooks_against_cirq_head(
notebook_path, require_packages_not_changed, env_with_temporary_pip_target
Expand Down