Skip to content

Fix Config.get_terminal_writer() crash when terminalreporter is unregistered#14378

Open
antoineleclair wants to merge 1 commit intopytest-dev:mainfrom
antoineleclair:fix/assertrepr-missing-terminalreporter
Open

Fix Config.get_terminal_writer() crash when terminalreporter is unregistered#14378
antoineleclair wants to merge 1 commit intopytest-dev:mainfrom
antoineleclair:fix/assertrepr-missing-terminalreporter

Conversation

@antoineleclair
Copy link
Copy Markdown

Closes #14377.

The fix

Config.get_terminal_writer() had a hard assert terminalreporter is not None that crashes when a plugin (e.g. pytest-tap in streaming mode) unregisters the terminal reporter. It now falls back to create_terminal_writer(self) — the same factory TerminalReporter.__init__ uses internally — so the happy path is unchanged and callers keep working when the reporter is gone.

This fixes the reported assertion-diff crash, and also covers runner.show_test_item (--collect-only -q) and setuponly._show_fixture_action (--setup-only/--setup-plan), which hit the same assert in their respective modes.

Does this change the behavior?

For the use case of pytest-tap, I don't think it changes the behavior for the assertion path — assertrepr_compare only uses _highlight, which is a pure string transformation; nothing gets written to stdout, so pytest-tap's TAP output is unaffected. In practice, it's a slight behavior change, so please let me know if you think this could have bad side effects, e.g. for other plugins.

For show_test_item / setuponly the fresh writer would actually emit to stdout, but those paths used to crash, so this seems strictly better. Happy to revisit if you'd rather silence them in that case.

Tests

  • test_config.py::TestConfigAPI::test_get_terminal_writer_without_terminalreporter — direct contract test on get_terminal_writer() with the plugin unregistered.
  • test_assertion.py::test_assertrepr_compare_without_terminalreporterpytester integration test reproducing the pytest-tap scenario end-to-end.

Both fail without the fix and pass with it.

Checklist

  • New tests added.
  • Allow maintainers to push and squash when merging my commits.
  • closes #14377.
  • AI was used to assist; credited in the Co-authored-by trailer. I reviewed every line and own the change.
  • changelog/14377.bugfix.rst added.
  • Added myself to AUTHORS in alphabetical order.

When a plugin such as pytest-tap in streaming mode unregisters the
terminalreporter plugin, Config.get_terminal_writer() crashed with an
internal AssertionError that masked the real test failures. Fall back
to create_terminal_writer(self) — the same factory TerminalReporter
uses internally — so assertion rewriting, show_test_item, and setuponly
keep working.

Fixes pytest-dev#14377.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Apr 10, 2026
Copy link
Copy Markdown
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

I would rather avoid a solution like this, it seems like something that might cause unexpected issues. It's also a bit wasteful to create a fresh instance on each assert.

I would rather look at removing the dependency on TerminalWriter from assertrepr_compare.

Seems to me that ideally assertrepr_compare would return some structured format that is then "rendered" externally. But the list[str] is pretty ingrained for now (there's a pytest_assertrepr_compare hook which returns it), so for now let's say this solution is out.

A question is whether highlighting should be performed when terminalreporter is disabled:

If yes, we should separate a "Highlighter" from TerminalWriter, and have both TerminalWriter and assertrepr_compare depend on it.

If no, then we should make assertrepr_compare not highlight when terminal reporter is disabled.

My thinking is "no", since a custom rendered probably doesn't want terminal escape codes. But maybe there are plugins which depend on it already, and plugins which don't want it just disable highlighting. So probably safer to go with "yes" solution.

Let me know if you want to try it, if not, I'll try looking into it myself.

@@ -0,0 +1 @@
Fixed :meth:`pytest.Config.get_terminal_writer` crashing with an internal ``AssertionError`` when the terminal reporter plugin has been unregistered (for example, by :pypi:`pytest-tap` in streaming mode). A fresh terminal writer is now created on demand.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_terminal_writer function is private, so you can't ref it. Also, let's remove the last sentence describing the solution since that's not relevant for a changelog.

Suggested change
Fixed :meth:`pytest.Config.get_terminal_writer` crashing with an internal ``AssertionError`` when the terminal reporter plugin has been unregistered (for example, by :pypi:`pytest-tap` in streaming mode). A fresh terminal writer is now created on demand.
Fixed a crash from `Config.get_terminal_writer` when the terminalreporter plugin has been unregistered (for example, by :pypi:`pytest-tap` in streaming mode).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bluetech.

I would rather look at removing the dependency on TerminalWriter from assertrepr_compare.

I think you're right. I was proposing a solution, mostly to highlight the problem and in case the solution would be acceptable (after all... it's crashing without the change, and it's not crashing with the change).

Let me know if you want to try it, if not, I'll try looking into it myself.

I think you have more knowledge of the code base and will do a better job with less effort. So if you're OK, I'd leave that one to you. We can close the PR whenever you want.

Or I may try to tackle it in a few months if I see it's still not been touched (which would be totally understandable).

Thank you for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Config.get_terminal_writer() crashes with AssertionError when terminalreporter is unregistered (pytest-tap streaming + non-test file asserts)

2 participants