testresult: correctly apply verbose word markup and avoid crash

[pbrezina]

The following snippet would have resulted in crash on multiple places since
_get_verbose_word expects only string, not a tuple.

    @pytest.hookimpl(tryfirst=True)
    def pytest_report_teststatus(report: pytest.CollectReport | pytest.TestReport, config: pytest.Config):
        if report.when == "call":
            return ("error", "A",  ("AVC", {"bold": True, "red": True}))

        return None

Traceback (most recent call last):
  File "/home/pbrezina/workspace/sssd/.venv/bin/pytest", line 8, in <module>
    sys.exit(console_main())
             ^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/config/__init__.py", line 207, in console_main
    code = main()
           ^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/config/__init__.py", line 179, in main
    ret: Union[ExitCode, int] = config.hook.pytest_cmdline_main(
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/main.py", line 333, in pytest_cmdline_main
    return wrap_session(config, _main)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/main.py", line 321, in wrap_session
    config.hook.pytest_sessionfinish(
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 122, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/logging.py", line 872, in pytest_sessionfinish
    return (yield)
            ^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 124, in _multicall
    teardown.send(result)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/terminal.py", line 899, in pytest_sessionfinish
    self.config.hook.pytest_terminal_summary(
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/home/pbrezina/workspace/sssd/.venv/lib64/python3.11/site-packages/pluggy/_callers.py", line 124, in _multicall
    teardown.send(result)  # type: ignore[union-attr]
    ^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/terminal.py", line 923, in pytest_terminal_summary
    self.short_test_summary()
  File "/home/pbrezina/workspace/pytest/src/_pytest/terminal.py", line 1272, in short_test_summary
    action(lines)
  File "/home/pbrezina/workspace/pytest/src/_pytest/terminal.py", line 1205, in show_simple
    line = _get_line_with_reprcrash_message(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/terminal.py", line 1429, in _get_line_with_reprcrash_message
    word = tw.markup(verbose_word, **word_markup)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pbrezina/workspace/pytest/src/_pytest/_io/terminalwriter.py", line 114, in markup
    text = "".join(f"\x1b[{cod}m" for cod in esc) + text + "\x1b[0m"
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
TypeError: can only concatenate str (not "tuple") to str

[The-Compiler]

Thank you! This looks reasonable, but will need a test case to avoid similar regressions in the future.

[pbrezina]

Thank you! This looks reasonable, but will need a test case to avoid similar regressions in the future.

Please, could you contribute a test case by pushing to this pull request? I guess I could base something on test_line_with_reprcrash but this will only trigger one of the cases.

[The-Compiler]

I'm afraid I got my hands full with stuff we're currently working on at the development sprint already.

It might be easier to write an integration test using pytester for this. Actually I'm confused: We already have test_report_teststatus_explicit_markup which is very close to what you showed in your original post, but that passes! So the first step would probably be to find out why that passes just fine. Maybe it just needs an additional check for the exit status or something?

I tried minimizing your example to:

def pytest_report_teststatus(report):
    return ("error", "A",  ("AVC", {"bold": True, "red": True}))

and I can still reproduce the failure, yet the test seems to pass just fine, despite doing almost the same:

        pytester.makeconftest(
            """
            def pytest_report_teststatus(report):
                return 'foo', 'F', ('FOO', {'red': True})
        """
        )

edit: No, the run in the test does indeed pass just fine:

> /home/florian/proj/pytest/testing/test_terminal.py(352)test_report_teststatus_explicit_markup()->None
-> breakpoint()
(Pdb) p result
<RunResult ret=0 len(stdout.lines)=11 len(stderr.lines)=0 duration=0.02s>

[webknjaz]

@pbrezina please also compose a change log note/fragment, while you're on it. It's important to explain how the behavior changed to the end-users.

[pbrezina]

@The-Compiler it fails only with error or failed category, not with foo. And if you set error in the test, the line matcher still works since the traceback is in stderr. I fixed the test and added changelog.

[webknjaz]

(I'll let someone else check how this influences other parts of pytest, this review only contains a few simple improvement suggestions)

[pbrezina]

All comments should be addressed now.

[webknjaz]

FTR here's the change note preview: https://pytest--12472.org.readthedocs.build/en/12472/changelog.html#bug-fixes

[webknjaz]

All's good on my side, let's wait for Florian and perhaps somebody else to re-review.

[nicoddemus]

Awesome, thanks!

[pbrezina]

Rebased.

[webknjaz]

@The-Compiler FYI this is waiting for you to unblock.

[patchback]

Backport to 8.2.x: 💚 backport PR created

✅ Backport PR branch: patchback/backports/8.2.x/90459a8fd3e308ba89f4d1a43cb2ee0412db523c/pr-12472

Backported as #12555

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[nicoddemus]

@webknjaz thanks for merging.

In the future consider squashing when the commits are not atomic/make sense, otherwise we end up with a polluted history:

[webknjaz]

In the future consider squashing when the commits are not atomic/make sense, otherwise we end up with a polluted history:

Ah, fair. I usually do this, but this time I forgot to check. I saw that your auto-merge got reset and clicked again w/o noticing that it was in a different mode, not natural merge.