Repository navigation
Conversation
The Oct 22 simplicity pass found an unread stderr ring buffer and getattr hedges on attributes pytest>=7.4 already guarantees. Vulture misses the buffer because append and len count as uses, and mypy --strict does not flag getattr-with-default. AST checks now fail on those examples and pass once the buffer is deleted and the attributes are read directly. Drop the remaining hedges that check still finds: item.name, report.failed, report.skipped, and report.nodeid. Co-authored-by: Nate Vick <natevick@users.noreply.github.com>
pytest 9.2 reads nodeid from _id. Assigning _nodeid no longer changes the id, so the newest CI job collected the original ids and then raised KeyError in _full_description. Write both fields, and do the same in the tests that inject ids. Co-authored-by: Nate Vick <natevick@users.noreply.github.com>
The newest-cell failure is on main too. pytest 9.2 reads nodeid from _id, and main still assigns _nodeid. That is a new pytest release, not this PR. Leave the breakage visible. Co-authored-by: Nate Vick <natevick@users.noreply.github.com>
Contributor
Author
|
Closing for now. We're keeping this guard only on vitest; we'll revisit if these patterns come back. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Selective's Oct 22 simplicity pass (head
6fa455b) kept finding the same two habits: code nothing calls, and hedges around attributes pytest 7.4 already guarantees. This adds one CI check for each. Both fail on the review's example and pass once that example is fixed the way the review recommends.selectiveci/selective-python-pytestpull request 1 from that review is gone (the API returned 404, andmainis the only branch). The client onmainalready has the review's fixes for the stderr buffer, the dead plugin lookups, and thepath/location/durationhedges. This draft targetsmain.Dead code / no caller
Architecture cannot stop a later edit from adding an unread attribute, and vulture does not flag the review's buffer:
appendandlenstill count as uses, sostderr_linesis invisible at every confidence. The check is an AST scan (tools/check_dead_code.py). It treats assign, append, and a length cap as writes, and it also rejects finding #1's dead lookups (hasattr(..., "node_reporters"),try/except Exception: passaroundget_plugin/stash.get, andget_plugin("html")).Red, finding #4, buffer reintroduced into
transport.pyinside a copy ofsrc/:Green, buffer deleted (the repo, and the review's
stderr=subprocess.DEVNULLshape, which the same test locks):Red, finding #1, the old junit/html lookups:
Green, stash key plus the live
_reports/_reportscan:Defensive checks on what pytest guarantees
pyproject.tomlalready requirespytest>=7.4. mypy--strictdoes not flaggetattr(item, "path", None)or aTypeErrorretry ofteardown_exact; those calls type-check. The check istools/check_pytest_contract.py. Its getattr error says to access the attribute directly because pytest>=7.4 guarantees it. The arity error says to callteardown_exactwith one argument.Red, findings #7 and #5:
Green, direct access and
teardown_exact(None)underexcept Exception(the review's finish_hooks):The same check was red on
mainfor four hedges of the same kind, and is green after this change:item.nameinstead ofgetattr(item, "name", "")report.failedandreport.skippedinstead ofgetattr(report, "failed"/"skipped", False)report.nodeidinstead ofgetattr(report, "nodeid", None)getattr(item, "originalname", item.name)and theconfig.optionhedges for xdist, stepwise, setuponly, and cov stay. Those attributes are not part of the pytest>=7.4 item/report contract. AGENTS.md marks them not enforced.test (3.14, newest)is a pre-existing pytest 9.2 breakHead
672e711failed that cell on both the push run and the pull_request run. Every other test and conformance cell passed. This PR does not touchitem._nodeid = wired. The same assignment is onmain(a6aae8d,plugin.pyassigns_nodeidand the tests inject ids the same way).Monday's scheduled run on
mainpassedtest (3.14, newest)because that job installed pytest 9.1.1: https://github.com/selectiveci/selective-python-pytest/actions/runs/40280624068The integration cannot rerun that workflow (
run 40280624068 cannot be rerun; Resource not accessible by integration). A push ofmain's tree (empty commit8d34c16, treef733f663, identical toa6aae8d) fails the same cell and no other: https://github.com/selectiveci/selective-python-pytest/actions/runs/40711705872That job installed pytest 9.2.0. pytest 9.2 reads
nodeidfrom_id, so assigning_nodeidno longer changes the id. Same lines as on672e711:Fourteen tests failed there, the same fourteen as on
672e711(https://github.com/selectiveci/selective-python-pytest/actions/runs/40710016716). The nodeid workaround that was briefly on this branch is reverted. This draft does not paper over it.Tests
tests/test_simplicity_checks.pylocks the red and green shapes above. CI runs the same two commands in thelintjob: