Add detail to traced values where str() is ambiguous - #729
Open
RonnyPfannschmidt wants to merge 7 commits into
Open
Add detail to traced values where str() is ambiguous#729RonnyPfannschmidt wants to merge 7 commits into
RonnyPfannschmidt wants to merge 7 commits into
Conversation
RonnyPfannschmidt
force-pushed
the
trace-value-heuristic
branch
2 times, most recently
from
September 12, 2026 20:51
41acb1b to
566041a
Compare
Tracing could turn a working hook call into a failing one in two ways: an object whose __repr__/__str__ raises propagated that exception out of the hook call (pytest-dev#424), and a lone surrogate in a hook argument or return value produced a message the writer could not encode (pytest-dev#681). Both are now handled in one place. _safe_repr()/_safe_str() wrap the conversion the way pytest's saferepr does -- KeyboardInterrupt and SystemExit still propagate, anything else is rendered as an unpresentable-object marker -- and escape lone surrogates with backslashreplace afterwards, which also covers surrogates that come out of an object's own __repr__. Traced values -- hook kwargs and the hook result -- now use repr() so their type is visible in the log; structural labels such as the hook name and the finish/--> markers keep using str() and stay unquoted. Supersedes pytest-dev#627, pytest-dev#666, pytest-dev#684 and pytest-dev#716. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
The previous commit also switched traced values from str() to repr(), following the design distilled in pytest-dev#681. That is a user visible change to pytest's --debug output, and it makes that output worse to read. The worst case is a value that is meant to be read as a block. With enable_assertion_pass_hook, pytest passes the assertion explanation to pytest_assertion_pass as a multi line string. Under str() the trace shows it as written: expl: {'x': [0, 1, ...} == {'x': [0, 1, ...} Omitting 2 identical items, use -vv to show Use -v to get more diff Under repr() the same value becomes one escaped line: expl: "{'x': [0, 1, ...} == {'x': [0, 1, ...}\n \n Omitting 2 identical items, use -vv to show\n Use -v to get more diff" The rest is quieter but hits every run: of 439 traced kwarg values in a real pytest --debug run, 123 render differently, and 115 of those are nothing but quotes added around strings that were already readable -- every plugin registration line turns plugin_name: lfplugin into a quoted string. 105 of the 674 lines in the sampled trace change, so anything parsing that output breaks as well. The trace is pytest UX. A fix for a crash that nobody hits in normal use must not degrade the daily reading experience of everyone who does not hit it. The cases where repr() genuinely helps are real -- PosixPath vs py.path.local for two arguments that print the same path, ExitCode vs a bare int -- but they are 15 lines out of 674, and they do not pay for the other 115 plus the escaped blocks. The crash fixes never depended on repr(): _safe_str() guards the conversion and escapes lone surrogates just as well, so pytest-dev#424 and pytest-dev#681 stay fixed while pytest --debug output is byte for byte what it was before (verified: 674 trace lines, 0 differences). The type visibility idea is not rejected, only unbundled -- it can be argued on its own in pytest-dev#681, as a deliberate output change with its own changelog entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
The guards around str() were only exercised for the simple case of a broken __str__. The exception explaining that failure can be just as broken, and Ctrl-C has to stay reliable at both levels, so cover: an exception whose repr fails, an exception whose repr and str both fail, and KeyboardInterrupt raised from the value and from the explanation. _tracing.py is at 100% statement and branch coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
BrokenStr's __repr__ was never called -- _safe_str reaches for __str__, and the failure message is built from the type name -- so it only showed up as an uncovered line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
Trace output stays str() based, because that is what makes it readable, but str() hides things a reader needs often enough to be worth fixing case by case: - an empty string is indistinguishable from no value at all, which is exactly wrong for a comparison trace showing left and right - a string carrying whitespace has no visible boundaries - an IntEnum prints as a bare number, losing the member name - a path prints as text, so a PosixPath and a py.path.local argument pointing at the same place look identical - a multi line value runs into column 0 and reads as a trace line of its own rather than as the value of its key So values that read unambiguously as themselves -- a non empty printable string without spaces, and every type whose repr adds nothing -- stay bare, and the rest gain quotes, their type, or a block. Multi line values are drawn as a box, each line prefixed with | and the last with \\, so the extent of the value is visible at a glance. Measured on a real pytest --debug run, this changes 45 of 1329 trace lines, against 216 for rendering every value with repr(). Every changed line carries information the previous rendering dropped. Builds on pytest-dev#728, which must land first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
_safe_repr is only reached for enums, paths and quoted strings, which all have working reprs in the existing tests, so its guards were dead in coverage. A path-like with a broken repr is the pytest-dev#424 scenario applied to a value the heuristic sends through repr. _tracing.py is back at 100% statement and branch coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
RonnyPfannschmidt
force-pushed
the
trace-value-heuristic
branch
from
September 12, 2026 21:48
566041a to
6015a75
Compare
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.
Stacked on #728 — that must land first. Until it does, the diff here includes its
two commits; the change proposed by this PR is the last commit,
Add detail to traced values where str() is ambiguous.Why
#728 fixes the two tracing crashes without touching the output, and says explicitly that
the type-visibility idea from #627/#681 should be argued on its own. This is that
argument.
Rendering every value with
repr(), as #681 proposed, changes 216 of 1329 lines in areal
pytest --debugrun, and the overwhelming majority of that is quotes added aroundstrings that were already readable. But
str()does drop things a reader needs:left:/right:val: with spaceexitstatus: 1IntEnumprints as a bare number, the member name is lostcollection_path: /xpath: /xPosixPathand apy.path.localpointing at the same place look identicalThe rule
repr()enum.Enumor anos.PathLikegetsrepr()str(), unchangedWhat it costs
Measured on a real
pytest --debugrun: 45 of 1329 lines change (3.4%), against 216for blanket
repr(). By category: 21 paths, 11 whitespace strings, 3 empty strings, 2enums, 2 blocks.
Multi-line values
Today a multi-line value breaks the layout — the continuation escapes to column 0 and
reads as a top-level trace line:
With this change the value is boxed, so its extent is visible at a glance:
Rough edges, for review
nodeid: test_mix.py::test_p[with space]gains quotes, because the parameter idcontains a space. The rule fires correctly, but on something that is not really "a
string with whitespace" in spirit.
orig: n or n == ""becomesorig: 'n or n == ""'— nested quotes on source text.'\ud800'rather than bare, since they are notisprintable(). Arguably clearer; it does change two tests added in Make hook tracing unable to fail a hook call #728.enabled — around 0.5 ms across a whole pytest run.
Testing
uv run pytest— 187 passed.uv run pre-commit run -a— all hooks pass._tracing.pyat 100% statement and branch coverage.newly reaches — a path-like with a broken repr, and
KeyboardInterruptraised from arepr.
pytest --debugoutput diffed againstmain: 45 changed lines, each one listed above.🤖 Generated with Claude Code