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

Full coverage #4019

Merged
merged 7 commits into from
Jun 25, 2024
Merged

Full coverage #4019

merged 7 commits into from
Jun 25, 2024

Conversation

jobh
Copy link
Contributor

@jobh jobh commented Jun 24, 2024

Fixes #4003

Adds coverage or better-considered coverage exclusions for the earlier FIXMEs.

In the process, one unrelated flake is addressed; plus pyodide CI execution is parallelized.

jobh added 5 commits June 25, 2024 09:14

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@jobh jobh force-pushed the coverage_fixme branch from f5be161 to 2d7cf3f Compare June 25, 2024 07:16
@jobh jobh force-pushed the coverage_fixme branch 7 times, most recently from 7da3025 to 5250359 Compare June 25, 2024 12:37
if not path.exists():
suffix = binascii.hexlify(os.urandom(16)).decode("ascii")
tmpname = path.with_suffix(f"{path.suffix}.{suffix}")
tmpname.write_bytes(value)
Copy link
Contributor Author

@jobh jobh Jun 25, 2024

Choose a reason for hiding this comment

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

I've seen this write_bytes fail occasionally with "file not found" in a pandas runner, here and in other PRs.

I don't know why it happens, or why it seems to happen mostly with pandas, but it looks like silently ignoring IO failures here is in the spirit of things.

elif (
self.failed_normally or self.failed_due_to_deadline
): # pragma: no cover # FIXME
phase = "shrink"
else: # pragma: no cover # in case of messing with internals
Copy link
Contributor Author

@jobh jobh Jun 25, 2024

Choose a reason for hiding this comment

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

Presumably this "messing with internals" is not totally unexpected, otherwise we wouldn't have code to handle it. But I couldn't find any tests or obvious way to enter this method at all without _runner set.

We could consider just dropping this handling, and require that whoever messes also attaches a mock runner?

@jobh jobh force-pushed the coverage_fixme branch from 5250359 to 4156ce0 Compare June 25, 2024 12:55
@jobh jobh force-pushed the coverage_fixme branch from 4156ce0 to adb0554 Compare June 25, 2024 12:59
# pyodide can't run multiple processes internally, so parallelize explicitly over
# discovered test files instead (20 at a time)
TEST_FILES=$(python -m pytest -p no:cacheprovider --setup-plan hypothesis-python/tests/cover | grep ^hypothesis-python/ | cut -d " " -f 1)
parallel --max-procs 100% --max-args 20 --keep-order --line-buffer \
Copy link
Contributor Author

@jobh jobh Jun 25, 2024

Choose a reason for hiding this comment

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

With all the other tests parallelized through xdist, pyodide was running by itself long after the other tests finish.

This splits the file list to execute batches of N=20 files at a time, in parallel, which brings the total execution time down to the same level as the other tests.

The test output is acceptable IMO, although split into 5-6 separate batches. I was pleasantly surprised by discovering the --keep-order --line-buffer combo, which orders output lines as-if-sequential but still outputs each line as soon as possible.

@jobh jobh marked this pull request as ready for review June 25, 2024 13:16
Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

😍

@Zac-HD Zac-HD merged commit 0c885e7 into HypothesisWorks:master Jun 25, 2024
56 checks passed
@jobh jobh deleted the coverage_fixme branch June 25, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve our internal coverage tests
2 participants