Skip to content

test: drive collect_until's late-finaliser test off a fake clock (#2016) - #2017

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2016-collect-until-grace
Oct 21, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2016-collect-until-grace

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy; make isort not run)

  • make test passes, and a test case covers the change (only tests/foundation/test_extraction_ownership.py was run locally; CI covers the rest)

  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Closes #2016.

collect_until itself is sound: it checks the predicate before the deadline. The trouble was the test, which ran the engine's real gc.collect against a 5 s wall-clock grace. It now patches gc.collect, time.monotonic and time.sleep with fakes, each collection costing one fake second. It also covers two edges that were untested: a predicate that holds with no grace (CPython's case), and one that never holds and stops once the grace is spent.

Evidence, with gc.collect patched to sleep 3 s: the old test fails with False is not true, matching the GraalPy leg, and the new one passes in 0.05 s. The new test also fails against a mutant that checks the deadline first and against one that ignores the grace. FINALISER_GRACE is left untouched, so this does not conflict with #2006.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 21, 2026
The test called the engine's real gc.collect under a wall-clock grace of
5 s, so GraalPy on a loaded runner could not fit the three collections
the predicate needs. Patch gc.collect, time.monotonic and time.sleep with
fakes (each collection costing a fake second), so the test checks the
contract rather than engine speed, and cover the two edges it left
untested: a predicate that holds with no grace, and one that never holds
and must stop once the grace is spent.
@JarryShaw
JarryShaw force-pushed the fix/2016-collect-until-grace branch from 8d17724 to 18d3284 Compare October 21, 2026 15:08
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one test Pull requests that add or correct tests (test: subject prefix) labels Oct 21, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 18d3284b8.

A Sonnet cross-review:

  • confirmed the fakes patch gc.collect, time.monotonic and time.sleep at the names collect_until actually calls, and undo them when the with block exits;
  • slowed gc.collect to 3 s: the old test failed after 9.27 s, the new one passed in 0.05 s;
  • swapped in a collect_until that checks the deadline before the predicate, and the new no-grace case caught it;
  • found the FINALISER_GRACE line untouched;
  • got 19 passed on the PR head.

GraalPy itself was not run; the graalpy-25.4 leg will be the first real check.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.55% (unit tier, Python 3.15, 18d3284b8, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19597 881 2618 867 92.11%
pcapkit/corekit 2542 71 810 37 96.54%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2930 23 1022 25 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17501 222 4612 187 98.13%
pcapkit/toolkit 872 36 298 10 95.21%
pcapkit/utilities 523 2 158 4 99.12%
pcapkit/vendor 4431 1995 1018 167 51.73%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 09a1f40 into main Oct 21, 2026
59 checks passed
@JarryShaw
JarryShaw deleted the fix/2016-collect-until-grace branch October 21, 2026 15:31
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 21, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

GraalPy: collect_until's late-finaliser test outruns its 5 s grace

1 participant