fix(test): gate the re.sub positional-flag DeprecationWarning test on Python 3.13 - #820
Conversation
… Python 3.13 main was red on Python 3.11/3.12 (and their Engines legs): the positional form's DeprecationWarning is a CPython 3.13 addition, and the test added by #813 asserted it unconditionally, with no version guard (#819). - Invert the assertion instead of skipping it: on Python 3.13+ the positional form must warn; below 3.13 it must NOT warn, which is a true, testable fact rather than a comment -- so no leg goes dark. Factored the branching into `_assert_positional_flag_shape` so both the real test and its self-test exercise identical logic against a boundary parameter. - Added a self-test proving the boundary constant is load-bearing: forcing the wrong branch on the running interpreter makes the assertion raise the exact "DeprecationWarning not triggered" symptom #819 hit, and the correct boundary holds regardless of which side of 3.13 is running. - Fixed a stale cross-reference to the renamed test method in the module docstring. Verified: `python -m unittest tests.vendor.test_re_sub_positional_flag_unit` green on Python 3.11.15, 3.12.14, and 3.14.7 (repo venv, pcapkit resolved from this tree); full `tests/vendor/` (85 tests) green on 3.14. Closes #819
|
Verified by me on three real interpreters, not by monkeypatch — the fail-before reproduces exactly and the fix holds: Interpreters used: It inverted rather than skipped, which is what I asked for and is the right call at this scale — a if sys.version_info >= boundary:
with self.assertWarns(DeprecationWarning): ...
else:
with warnings.catch_warnings(record=True) as caught: ...
self.assertFalse(any(issubclass(w.category, DeprecationWarning) for w in caught), ...)
And its self-test finding is worth recording, because it corrects my own brief. I suggested proving the constant load-bearing by changing 13 → 12. The author did that literally and found the experiment is interpreter-dependent: on 3.11 and 3.14 the mutation causes no failure, since both sit strictly on one side of the 12/13 gap and the wrong boundary still agrees with real behaviour there. Only on 3.12 — the interpreter actually inside the gap — does it reproduce the original It also caught a stale
Labelled |
|
Cross-review verdict on the change: GOOD TO GO (haiku; author was sonnet). But I am NOT flipping the label yet, because this PR's own CI is incomplete — the reviewer withheld handover for exactly that reason, and it is the right call. This is the rule I adopted after clearing #813 on a partial run: It found a 3.10 interpreter that neither the author nor I had, and ran five versions. The vacuity trap is defeated three ways, and the third is decisive. Repetition (3 consecutive runs in one process, all raising on 3.13/3.14, so registry caching is not suppressing re-emission); a deliberately poisoned So It generalised my caveat into something reusable: a boundary mutated from B to B′ is detected only by interpreters The rename is clean — One thing worth fixing before merge, prose only: the new comment says the breakage hit "every leg below 3.13 (measured: Python 3.11, 3.12 …)", omitting 3.10 — which reproduces the partial-run scope error that #819's own correction retracted, in the file whose purpose is to stop that class of mistake. Two other wording nits: " Noted, pre-existing, not actionable now: Staying |
|
CI is now complete and green — All six legs that That is the first time this session a This is the PR to merge first. The one prose item stands and does not block: the new comment enumerates "Python 3.11, 3.12" and omits 3.10, reproducing the partial-run scope error that #819's own correction retracted. Worth a one-line fix whenever convenient, in the file whose purpose is to prevent that class of mistake. |
|
Scope correction, so merging this is not mistaken for making From So |
pylint10/10,mypyno new errors,isortclean)make testpasses, and a test case covers the changeWhat is the purpose of your pull request?
fix— corrects a defectDescription of your pull request and other information
Closes #819.
mainwas red on Python 3.11/3.12 (and their Engines legs): #813 added atest asserting
re.sub's positional-countDeprecationWarningunconditionally, but that warning is a CPython 3.13 addition — 3.11/3.12
never raise it.
Inverted the assertion rather than skipping it, per the issue's preferred
fix: on 3.13+ the positional form must warn; below 3.13 it must not,
pinning the boundary as a tested fact rather than a comment (mirrors
#788's
test_the_shim_switches_at_the_version_that_added_the_dunder).Added a self-test proving the boundary constant is load-bearing, and fixed
a stale docstring cross-reference to the renamed method.
Verified on Python 3.11.15, 3.12.14, and 3.14.7 (repo venv) — all green,
and confirmed the wrong boundary reproduces the exact
mainfailure on3.12. Did not run
make test(whole-suite OOM risk); ran the fulltests/vendor/tree (85 tests) on 3.14 instead, all green.