Skip to content

test: keep the per-class pcapkit import in 12 classes that re-imported per test (#1538) - #1557

Merged
JarryShaw merged 1 commit into
mainfrom
test/1538-p2-per-class-imports
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
test/1538-p2-per-class-imports

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

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

Refs #1538 (P2). Twelve classes in seven modules, 66 tests, called reimport_once_per_class() in setUp and then purged pcapkit in tearDown. That emptied the class's import before _keep_lazy_imports could record it, so every test re-imported the package. This PR drops the tearDown purge. The proposal's 13 classes and 72 tests counts are wrong: an AST scan of main finds 12 and 66. test_const_enum_builtin_parity is untouched.

Measured, serial pytest, mean of 3:

Run Before After
The 7 modules in one process 71.8 s 15.4 s
Sum of 7 per-module processes 72.0 s 22.6 s

Both runs gave 68 passed and 63 subtests. Line and branch coverage of pcapkit is identical (27,861 lines, 1,307 branches).

Plain unittest, one run each:

Leg Before After
run_unittest_leg.py protocols --exclude protocols/internet 330 s 286 s
run_unittest_leg.py utilities 63 s 48 s

Both legs had 0 failures and 0 errors. These figures are lower bounds, because the baseline was a non-git tree that skipped 57 git-dependent root tests.

CI estimate, not measured: about 2.4-3.7 runner-minutes per PR, using #1538's unverified 1.4-2.2x xdist factor and its x1.57 coverage leg.

…d per test (#1538)

- 12 classes in 7 modules call reimport_once_per_class() in setUp and purged
  pcapkit in tearDown. tearDown runs before _keep_lazy_imports records the
  class's import, so every test re-imported (#1070 converted setUp only).
  Drop the tearDown purge.
- SchemaMetaAbcCacheTests and DefaultDescriptorResolutionTests use
  restore=True, which keeps their documented confinement (the ABC-cache reset,
  the reloaded Raw) at class scope; their comments are rewritten to match.
- test_dispatch_keys_did_not_move_with_the_modules takes a fresh import via
  isolate_modules(): a dispatch hit writes the resolved class back into the
  registry, so on the shared import it failed whenever a parsing test ran first.
- New tests/project/test_per_class_import_guard.py AST-scans tests/ for the
  pairing. On main it fails and lists the 12 classes.

The 7 modules in one serial pytest process: 71.8 s -> 15.4 s (mean of 3), with
68 passed and 63 subtests both times and pcapkit coverage unchanged.
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet test Pull requests that add or correct tests (test: subject prefix) labels Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 62b96d993. Cross-reviewed on Sonnet; the author ran Opus.

  • Scope: an independent AST scan of main finds the same 12 classes and 66 tests. The verifier's 13/72 included SchemaMetaReservedClassKwargsTests, which has no tearDown purge. After the change, the scan finds 0 such classes. ConstEnumRegisterFallbackTests is untouched.
  • Order independence: each multi-test class passes 7 orderings, under both plain unittest and pytest (84/84 and 77/77). The two restore=True classes still leave 0 pcapkit modules behind. A Schema subclasses share one ABCMeta cache on Python <=3.10, so isinstance/issubclass return whichever answer was asked first #439 revert mutant fails the same 5 of 7 schema tests on main and on the branch.
  • CI legs unchanged: protocols --exclude protocols/internet runs 1,231 tests and utilities runs 312, with identical per-test outcomes to main. Executed coverage lines are identical (27,861).
  • Saving: the 7 modules in one pytest process went from 64–67 s to about 13 s.

Not blocking:

@JarryShaw JarryShaw removed the review: running A cross-review is in flight against the current head - no verdict yet label Oct 10, 2026
@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 90.02% (unit tier, Python 3.14, 62b96d993, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 842 2356 756 92.09%
pcapkit/corekit 2204 67 712 34 96.26%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2797 132 986 48 94.18%
pcapkit/interface 108 7 36 5 91.67%
pcapkit/protocols 16992 229 4546 190 98.03%
pcapkit/toolkit 635 98 198 5 83.31%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw
JarryShaw merged commit cf6e78a into main Oct 10, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the test/1538-p2-per-class-imports branch October 10, 2026 03:38
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 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.

1 participant