fix(vendor): pin except BaseException and correct a false docstring clause

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible -- N/A, changelog 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?

• fix — corrects a defect

Description of your pull request and other information

Closes #875. Three follow-ups from #873's review that arrived after it merged as 687721091. No behaviour in the merged code was wrong; one invariant was unpinned and two pieces of prose/lint were inaccurate.

1. except BaseException: in _snapshot_and_restore is now pinned. Mutating it to except Exception: previously left tests/vendor/test_vendor_snapshot_restore_unit.py fully green (run=3 failures=0 errors=0), while the measured consequence of a KeyboardInterrupt mid-crawl under that mutant is a truncated const file and an orphaned .const.py.*.bak inside pcapkit/const/. The new test_a_keyboard_interrupt_still_restores_and_propagates asserts all three halves of the invariant — content restored byte-for-byte, no .bak surviving, and the KeyboardInterrupt still propagating rather than being converted into a False return. It discriminates: real code run=4 failures=0 errors=0; against the except Exception: mutant run=4 failures=1, failing '' != 'GOOD CONTENT\n', with the predicted ['.const.py.<random>.bak', 'const.py'] left behind.

2. A false clause in _snapshot_and_restore's docstring is corrected. It claimed a crawler whose _dest_path cannot be resolved "goes on to fail loudly anyway once Vendor.__init__ calls the same _dest_path itself". That holds for one of its two enumerated triggers and not the other: an instance-method _dest_path raises TypeError on the class call this function makes, so the snapshot is skipped, but the instance call inside __init__ binds self and resolves fine — so the target runs unprotected and succeeds silently (run() → True, content replaced, nothing warns). Measured with a constructed instance-method-only stub. test_vendor_snapshot_restore_unit.py's own module docstring already described this correctly, so the two no longer disagree.

3. The # pylint: disable=no-else-raise pragma is KEPT. This was going to be a third fix, on the belief that R1720 never applies to try/except/else. That was wrong, and the measurement that appeared to support it was a message-control artefact: --disable=all --enable=no-else-raise does not re-enable the check, where --enable=R does. Under the Makefile's own PYLINT_FLAGS — what CI runs — pylint 4.0.8 does flag it:

pragma absent : pcapkit/vendor/__main__.py:143:4: R1720: Unnecessary "else" after "raise" ...
pragma present: (no R1720)

The else: it objects to is deliberate and the comment at :153-163 explains why: it is what makes the cleanup unreachable from the except, and deleting the raise that would otherwise be load-bearing is a mutation the suite must catch. So the suppression stays.

Deliberately out of scope: deleting os.close(fd) survives as an untested fd leak (harmless here — the path still exists and copy2 reopens it), and swapping the else: back to an unindented statement while keeping raise also survives, correctly, since the two forms are semantically identical and no test can distinguish them.

One commit on top of 687721091. tests/vendor/test_vendor_snapshot_restore_unit.py is run=4 failures=0 errors=0 and tests/vendor/test_vendor_exit_code_unit.py is run=5 failures=0 errors=0, both under PYTHONSAFEPATH=1 with the editable finders evicted and pcapkit.__file__ asserted against the tree under test.

[JarryShaw]

Adjudicating the pylint disagreement, since I published the original claim and the worker measured against it.

The worker reported that removing the pragma does reintroduce R1720 at that line, with the whole-file score dropping 9.47 → 9.34 — and flagged it rather than silently complying, which is right. I re-measured on the same interpreter (pylint 4.0.8, astroid 4.0.4):

without pragma, --disable=all --enable=no-else-raise   -> 10.00/10, no R1720
with pragma restored, + --enable=useless-suppression   -> I0021: Useless suppression of 'no-else-raise' (line 128)

So R1720 does not fire on this try/except/else, and pylint itself calls the suppression useless. The worker's 9.47 → 9.34 delta came from a run with other checkers enabled, so the score moved for a different message — an easy conflation, and exactly why the targeted --disable=all --enable=<checker> form is the one to use when adjudicating a single message. The pragma stays removed.

[JarryShaw]

NEEDS CHANGES on 0331e2fe3 — cross-review (opus; author sonnet). Items 1 and 2 are correct and well pinned. Item 3 is wrong, and the error is mine: my earlier adjudication above was the artefact, not the worker's measurement.

R1720 does fire on this try/except/else under what CI actually runs. Measured with the Makefile's own PYLINT_FLAGS, same file, only line 143 differing:

pragma ABSENT : pcapkit/vendor/__main__.py:143:4: R1720: Unnecessary "else" after "raise" ...   9.34/10
pragma PRESENT: (no R1720)                                                                      9.47/10

That is exactly the 9.47 → 9.34 the worker reported. My --disable=all --enable=no-else-raise reading of 10.00/10 was a message-control artefact — enabling by message name after --disable=all does not re-enable the check, where --enable=R or --enable=refactoring does — and the I0021 Useless suppression I cited only appears in that same broken configuration, where R1720 was off to begin with. pylint 4 extends no-else-raise to a try/except/else whose except ends in raise. The worker was right to flag the disagreement rather than comply silently.

So: restore # pylint: disable=no-else-raise on line 143 and drop item 3 from this PR. The commit subject's "drop an inert pragma" is false as written and needs rewording too. The pragma is suppressing a real message on a structure the adjacent comment at :153-163 explicitly defends as deliberate — which is what it was there for. Keeping it costs nothing: I is not in PYLINT_FLAGS, so no I0021 appears in CI either way. I will fix the PR body and title once the commit lands.

Items 1 and 2, confirmed: the new KeyboardInterrupt test is killed by the except BaseException: → except Exception: mutation and by nothing else, which is exactly its coverage claim; deleting os.replace fails 2 methods, deleting raise fails 3, deleting the whole else: clause fails 1. The assertRaises(KeyboardInterrupt) is not decorative — I had guessed it was, and that was wrong too: under the raise-deleted mutant the content and listing assertions both still pass, and assertRaises is the only one that catches it (AssertionError: KeyboardInterrupt not raised). The mock is correctly scoped — it captures real_print before patching and raises only for the rendered content, with run()'s own stderr print demonstrably passing through. The reworded docstring clause matches measurement clause for clause: class call raises TypeError, mkstemp spy shows 0 calls versus 1 for a classmethod control, run() → True, file replaced, zero warnings recorded.

One honest limitation, not blocking: if a KeyboardInterrupt fired before the snapshot, that single method would pass vacuously. Unreachable from its own injection point, since open(const_file, 'w') truncates before print(context, file=file) raises.

[JarryShaw]

GOOD TO GO on f302eafb5. Re-granted by diff rather than by re-review: git diff 0331e2fe3 f302eafb5 is one file, +1/−1, and it is exactly the pragma going back onto the try: at :143. Scope against main is unchanged — pcapkit/vendor/__main__.py and tests/vendor/test_vendor_snapshot_restore_unit.py, nothing else — and the commit subject is now fix(vendor): pin except BaseException and correct a false docstring clause, with the false "inert pragma" claim gone.

The worker re-measured under the real PYLINT_FLAGS and reports 9.87 with the pragma versus 9.74 without, the absent case flagging R1720 at :143:4. That is the same delta I measured on bare flags (9.47 → 9.34), from the same cause.

Everything in the 0331e2fe3 verdict carries over: the KeyboardInterrupt test is killed by the except BaseException: → except Exception: mutation and by nothing else, assertRaises(KeyboardInterrupt) is load-bearing rather than decorative, and the reworded docstring clause matches measurement. I have rewritten item 3 of the PR body to record that the pragma is kept and why. Two of the three items from #875 land here; the third is withdrawn as mistaken — #875 stays open until I close it with a note, since its own item 3 is equally wrong.