Skip to content
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

Re-enable doctest #5742

Merged
merged 11 commits into from
Jul 13, 2022
Merged

Re-enable doctest #5742

merged 11 commits into from
Jul 13, 2022

Conversation

dabacon
Copy link
Collaborator

@dabacon dabacon commented Jul 12, 2022

Getting this PR out for review.

There is a broken test due to a pending deprecation.

Fixes #5678

@CirqBot CirqBot added the size: L 250< lines changed <1000 label Jul 12, 2022
Comment on lines 228 to 233
excluded = [
'cirq-google/cirq_google/engine/client/',
'cirq-google/cirq_google/cloud/',
'cirq-google/cirq_google/api',
]
file_names = [f for f in file_names if not any(f.startswith(x) for x in excluded)]
Copy link
Collaborator

@pavoljuhas pavoljuhas Jul 12, 2022

Choose a reason for hiding this comment

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

This avoids hard failure in cirq-google/cirq_google/optimizers/optimize_for_sycamore_test.py.
I guess we can skip doctesting docstrings in tests.

Suggested change
excluded = [
'cirq-google/cirq_google/engine/client/',
'cirq-google/cirq_google/cloud/',
'cirq-google/cirq_google/api',
]
file_names = [f for f in file_names if not any(f.startswith(x) for x in excluded)]
excluded = [
'cirq-google/cirq_google/engine/client/',
'cirq-google/cirq_google/cloud/',
'cirq-google/cirq_google/api/',
]
file_names = [f for f in file_names if not any(f.startswith(x) for x in excluded)]
file_names = [f for f in file_names if not f.endswith('_test.py')]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh yeah good, idea skipping tests. Added skipping.

@@ -161,6 +174,7 @@ def exec_tests(
failed, attempted = 0, 0
error_messages = []
for test in tests:
print(test.file_name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check if the printout should be in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

definitely a left over print debug. removed

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM if CI checks pass.

@dabacon dabacon added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 13, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 13, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jul 13, 2022

Automerge cancelled: A required status check is not present.

Missing statuses: ['Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 13, 2022
@@ -230,17 +229,19 @@ def main():
'cirq-google/cirq_google/cloud/',
'cirq-google/cirq_google/api',
Copy link
Collaborator

@pavoljuhas pavoljuhas Jul 13, 2022

Choose a reason for hiding this comment

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

To be on a safe side - let's replace api --> api/ as for above paths.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@dabacon dabacon merged commit 8ffe776 into quantumlib:master Jul 13, 2022
@pavoljuhas pavoljuhas added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 13, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Tells CirqBot to sync and merge this PR. (If it's running.) size: L 250< lines changed <1000
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doctest = don't-test
3 participants