Skip to content

Full coverage#4019

Merged
Zac-HD merged 7 commits intoHypothesisWorks:masterfrom
jobh:coverage_fixme
Jun 25, 2024
Merged

Full coverage#4019
Zac-HD merged 7 commits intoHypothesisWorks:masterfrom
jobh:coverage_fixme

Conversation

@jobh
Copy link
Copy Markdown
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 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
Copy Markdown
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.

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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