Skip to content

Fix resolution of notebook paths when pytest is not at the repo root #7487

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

Jump to bottom
Merged
merged 1 commit into from
Jul 15, 2025
Merged

Fix resolution of notebook paths when pytest is not at the repo root #7487

merged 1 commit into from
Jul 15, 2025

Conversation

pavoljuhas
Copy link
Collaborator

Create the out directory for notebook outputs at the repository
root regardless of the pytest execution directory.

Follow-up to #7481

@pavoljuhas
Fix resolution of notebook paths when pytest is not at the repo root
26a4c7e 
Create the `out` directory for notebook outputs at the repository
root regardless of the pytest execution directory.

Follow-up to quantumlib#7481
@pavoljuhas pavoljuhas requested review from vtomole and a team as code owners July 15, 2025 00:03
@pavoljuhas pavoljuhas requested a review from maffoo July 15, 2025 00:03
@github-actions github-actions bot added the size: S 10< lines changed <50 label Jul 15, 2025
@pavoljuhas pavoljuhas requested a review from mhucka July 15, 2025 00:06
@codecov Codecov
Copy link

codecov bot commented Jul 15, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 98.68%. Comparing base (89ebad8) to head (26a4c7e).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
dev_tools/notebooks/notebook_test.py 0.00% 3 Missing ⚠️
dev_tools/notebooks/isolated_notebook_test.py 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7487      +/-   ##
==========================================
- Coverage   98.68%   98.68%   -0.01%     
==========================================
  Files        1092     1092              
  Lines       96855    96857       +2     
==========================================
- Hits        95583    95582       -1     
- Misses       1272     1275       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mhucka
mhucka approved these changes Jul 15, 2025
View reviewed changes
Copy link
Contributor

@mhucka mhucka left a comment

Choose a reason for hiding this comment

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

This looks reasonable.

I'm not sure I understand why Codecov is flagging changes in test files. Does it normally do that? Do we need to test the test files?

@pavoljuhas
Copy link
Collaborator Author

I'm not sure I understand why Codecov is flagging changes in test files. Does it normally do that? Do we need to test the test files?

It is a false alarm. The flagged code is for tests marked as slow or weekly so it is not exercised in the pytest coverage run. That code is executed in notebook tests (which do not contribute to coverage report).

@pavoljuhas pavoljuhas added this pull request to the merge queue Jul 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 15, 2025
@pavoljuhas pavoljuhas added this pull request to the merge queue Jul 15, 2025
Merged via the queue into quantumlib:main with commit bb2e86b Jul 15, 2025
34 of 35 checks passed
@pavoljuhas pavoljuhas deleted the fix-nb-output-location branch July 15, 2025 19:10
This was referenced Aug 5, 2025
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhucka mhucka mhucka approved these changes

@vtomole vtomole Awaiting requested review from vtomole vtomole is a code owner

@maffoo maffoo Awaiting requested review from maffoo maffoo is a code owner automatically assigned from quantumlib/cirq-maintainers

Assignees
No one assigned
Labels
size: S 10< lines changed <50
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pavoljuhas @mhucka