pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
12.2k stars 2.7k forks source link

Tweak `--tb=auto` to be equivalent to `--tb=short` #9731

Open mitchhentges opened 2 years ago

mitchhentges commented 2 years ago

There's earlier discussion (and the origin of --tb=auto) over in #364.

I'm not sure that the behaviour of --tb=auto is the best default behaviour that we can provide our users:

  1. It implies un-proportional importance of the first and last frame in a traceback, since they're the only ones whose functions are printed
    • A significant amount of time, the last frame is very unhelpful in a traceback because it's down in 3rd-party or standard-library code.
    • As projects get larger, the majority of code (and the majority of test risk) is in the code being orchestrated, not the test itself. So, the first frame (the test itself) becomes less likely to be the cause of the failure than the rest of the code running in the test.
  2. It can be tricky and a little time-consuming to find the "meat of the traceback", especially if the first/last frames in the traceback are within very large functions (I promise I didn't write them! I'm just the unlucky new maintainer :stuck_out_tongue:)

If we adopt --tb=short to be the default instead:

  1. It's more familiar, because it's similar to a regular Python exception traceback (though more concise, which I really appreciate. Great work team!)
  2. The entire stack is visible at once without any scrolling, which makes it easier to understand the shape of the failure.

Alternative Solutions

One way to improve the current situation without anything too radical would be to expand the last first-party code frame in the stack, leaving all 3rd-party and standard-library frames collapsed by default. However, it can be tricky to identify "first-party vs third-party", especially when sys.path modifications are happening (hello from Firefox's build tools! :wave: :smile: ).

Additional context

I have a bug reported downstream with additional rationale, some of it project-specific.

nicoddemus commented 2 years ago

Thanks @mitchhentges for bringing this up!

I'm not too fond of adopting --tb=short by default: although --tb=auto has its downsides, I think it still produces nice output in a good number of cases (I would say most), but of course this depends on the codebase.

One way to improve the current situation without anything too radical would be to expand the last first-party code frame in the stack, leaving all 3rd-party and standard-library frames collapsed by default.

This I'm much more a big fan of, and would be a huge improvement to all users.

The question is, how do we detect what to show and what to hide.

While we can think of a number of heuristics for that, none come to mind which is bullet proof in all source layouts and installation scenarios.

But how about if we let the user configure it explicitly? While having the user configure something to obtain the benefit is not ideal, as ideally we would love for pytest to magic guess/detect things, it might be the easiest and safest path to at least introduce the feature.

I propose we add two new options:

By default none are defined, then the current behavior for --tb=auto remains.

If they are defined however, they provide a way for users to tweak which entries are shown in the traceback with --tb=auto.

For example, suppose a project has its code under app, and app.vendor contains vendored code which is not interesting to show in tracebacks. Also the user wants django code to show in tracebacks for some reason.

The user can then configure the options like this:

[pytest]
tb_auto_collapse = app.vendor.*
tb_auto_expand = 
  app
  django

This will expand only tracebacks that match app or django, but not app.vendor.*, and collapse everything else.

However I just realized that this information is actually about filtering tracebacks, and not what tb mode is used. I can see the same options being used to collapse/expand entries for --tb=long for example. We might in that case rename the options to something like tb_entry_collapse/tb_entry_expand.


While not perfect, I think this is a good starting point anyway because: