Skip to content

ci: bound unittest-leg reimport growth and add stall diagnostics (#1052) - #1059

Merged
JarryShaw merged 1 commit into
mainfrom
ci/1052-unittest-leg-isolation
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
ci/1052-unittest-leg-isolation

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • ci — workflows or build tooling

Description of your pull request and other information

Part of #1052 (A's faulthandler half, B, D). No workflow edits.

A. run_unittest_leg.py arms faulthandler.dump_traceback_later (--stall-dump, or $PCAPKIT_UNITTEST_STALL_DUMP; default 1080s, 0 off), cancelled on exit.

B, as a collect rather than a restore. Any sys.modules restore hides #981. On its reproduction (32bcfba15^, test_http_unit + test_base_class_contract): no restore gives 1 F + 3 E; restoring per module or per test gives 0 + 0; gc.collect() after each test keeps 1 F + 3 E. --gc-every N (default 10). protocols/internet measured locally (446 tests, all OK):

run wall peak RSS
main, and this branch without the collect (×3) 420 / 502 / 432 s 1008 / 1650 / 781 MiB
--gc-every 10 (default) 420 s 491 MiB
--gc-every 1 500 s 320 MiB

The 1650 MiB run reproduced the slow tail: test_mh_unit tests at up to 7.3 s instead of 1.2-1.8 s.

D. pytest-timeout in test; timeout = 900, timeout_method = "thread". The slowest test measured was 21 s, and 15× that is 315 s. signal is out because its pending alarm fails TimeLimitTests.test_nothing_is_re_armed_when_nothing_was_pending. Every pytest job already installs .[test…]. On its own, thread killed every test runner at 13% (e8db64a9c). pytest-timeout keeps each item's Timer alive, and each Timer holds on to the tbtrim excepthooks that one import pcapkit installed, which pins that generation. tests/conftest.py now drops the timer, and tests/const under -n 4 peaks at 1979 MiB (9179 MiB and killed before the fix, 1939 MiB without the plugin).

@JarryShaw JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
- util/run_unittest_leg.py: arm faulthandler.dump_traceback_later (--stall-dump /
  $PCAPKIT_UNITTEST_STALL_DUMP, default 1080s) and cancel it on exit.
- util/run_unittest_leg.py: gc.collect() every N tests (--gc-every, default 10),
  bounding re-import garbage without touching sys.modules (#981 stays visible).
- pyproject.toml: pytest-timeout in the test extra; timeout = 900, method thread.
- tests/conftest.py: drop the timer pytest-timeout keeps on each item, which
  pinned one pcapkit generation per test via the captured excepthooks.
- tests/project: cover the stall dump, the collect and the conftest hook.
@JarryShaw
JarryShaw force-pushed the ci/1052-unittest-leg-isolation branch from e8db64a to 87c229e Compare October 6, 2026 01:44
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 87c229eda: GOOD TO GO (ran on Sonnet; author Opus)

  • Root cause reproduced. test_const_registry_protocol.py with pytest-timeout 2.4.0 in thread mode:

    Run Result Peak RSS
    With the hook passed 281 MiB
    Hook neutralised MemoryError after 66 tests 7237 MiB
    Plugin off passed 283 MiB

    pytest_timeout_set_timer stores item.cancel_timeout and never clears it, and pytest_timeout_cancel_timer is a real hookspec.

  • Timeouts still fire. A 5 s sleep under timeout = 2 fails with a stack dump. Earlier timers are cancelled.

  • test: three modules pass alone but fail together, and pytest reports it green #981 detection survives --gc-every 0, 1 and 10. It still reports 1 failure and 3 errors on the 32bcfba15^ repro.

  • Signal mode is correctly excluded. It fails test_nothing_is_re_armed_when_nothing_was_pending with 900 != 0.

  • New tests fail when the change is reverted: all 14 of them, under nine separate mutations.

  • CI: the Python 3.10–3.14 legs take 14m51s to 18m55s.

Non-blocking nits:

  • PCAPKIT_UNITTEST_STALL_DUMP set to nan or a negative value silently disables the dump, and inf raises OverflowError.
  • The "just under a 20-minute step cap" comment is stale: that step has no cap of its own; only the job's 45-minute cap applies.
  • Under xdist, a hung test crashes its worker. It is still reported by name, but without the stack dump.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw
JarryShaw merged commit d2ba04c into main Oct 6, 2026
75 of 76 checks passed
@JarryShaw
JarryShaw deleted the ci/1052-unittest-leg-isolation branch October 6, 2026 03:35
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant