Increase truncation threshold with -v, disable with -vv #8391

nicoddemus · 2021-03-03T00:49:12Z

This helps with the problem described in #6682.

~~Decided to go with -vvv and -vvvv because I believe they are rarely used, so this should really affect users that want to disable truncation, but would like to hear opinions.~~

As discussed in #8403, changed the verbosity to 2400 for -v, and unlimited for -vv and up.

Fix #6682
Fix #8403

bluetech

The feature request and proposed solution LGTM. I tried briefly to think of alternatives but since we already started with -v, -vv, pushing it more seems best in terms of user expectation, although I hope 4 would be the end of it 😀

Is there any documentation/help text that needs to be updated? Also I wonder if the Full output truncated, use '-vv' to show wants to mention the higher levels as well (probably not). Basically, offer some way for a user to figure out that disabling the ellipsis is possible.

bluetech · 2021-03-03T13:08:25Z

src/_pytest/_io/saferepr.py

@@ -75,15 +90,15 @@ def safeformat(obj: object) -> str:
        return _format_repr_exception(exc, obj)


-def saferepr(obj: object, maxsize: int = 240) -> str:
+def saferepr(obj: object, maxsize: Optional[int] = 240) -> str:


Is it possible to synergize this 240 with _DEFAULT_REPR_MAX_SIZE?

Sure, will do!

bluetech · 2021-03-03T13:15:59Z

src/_pytest/assertion/rewrite.py

@@ -427,7 +427,22 @@ def _saferepr(obj: object) -> str:
    sequences, especially '\n{' and '\n}' are likely to be present in
    JSON reprs.
    """
-    return saferepr(obj).replace("\n", "\\n")
+    maxsize = _get_maxsize_for_saferepr(util._config)


This consults the Config at the time the test is run. Alternative is to consult the config at the time the test is rewritten, that is, make the AssertionRewriter get the maxsize, add a maxsize argument to _saferepr and make AssertionRewriter pass it as a constant in the AST. Do you have any preference? The reason I'm talking about that is trying to avoid the util._config mutable global. These tend to be not great...

Alternative is to consult the config at the time the test is rewritten, that is, make the AssertionRewriter get the maxsize

I thought that too, however I realized assertion rewritten is cached, so if you invoke pytest the first time, the .pyc file is rewritten and cached; if you invoke pytest -v next, the .pyc file is cached so assertion rewriting is not triggered. Unfortunately this information needs to be available at runtime.

I injected the Config object in the same way we inject the comparison functions today, so I think this at least aligns with how things work today, however I'm definitely open for alternative ideas. 👍

testing/io/test_saferepr.py

testing/test_assertrewrite.py

nicoddemus · 2021-03-03T14:00:14Z

The feature request and proposed solution LGTM. I tried briefly to think of alternatives but since we already started with -v, -vv, pushing it more seems best in terms of user expectation, although I hope 4 would be the end of it 😀

I hope too!

As for alternatives, we have discussed separating verbosity of terminal output (how tests are printed as they are executed) and verbosity of assertions (how assertion messages are printed), however that change is analogous to this one.

Is there any documentation/help text that needs to be updated?

Good point. I didn't find a place in the docs that discussed verbosity in particular, so I think we will need to add one (will do so in this PR later).

src/_pytest/_io/saferepr.py

ssbarnea

I tested this manually on an I am happy to say that I now see an unabridged AssertionError that is ~1500 characters long, regardless the verbosity level (tried 0, 1, 2). IMHO, ~2-4KB default should be considered safe for any consumer, we should remember that these are errors, so is quite likely that the user would want to see them.

The outcome allow user to identify the what went wrong, before that it was a more like a single line which almost never included enough info to identify the issue.

nicoddemus · 2021-03-03T17:52:47Z

I tested this manually on an I am happy to say that I now see an unabridged AssertionError that is ~1500 characters long, regardless the verbosity level (tried 0, 1, 2)

Thanks for testing!

Hmm something went wrong however, because my patch should only have an effect on verbosity >= 3... I will take another look at the code later, but is there anything in particular to that output that might help finding the cause? 🤔

ssbarnea · 2021-03-03T18:21:06Z

In case you do not want to increase the length for lower verbosity levels, how are we going to be able to configure a custom limit? If that is again hardcoded, it may not be of much use for me.

RonnyPfannschmidt · 2021-03-03T18:25:29Z

The limit will be 1mb for utterly verbose, 2.4k for very verbose

nicoddemus · 2021-03-03T18:33:57Z

In case you do not want to increase the length for lower verbosity levels, how are we going to be able to configure a custom limit? If that is again hardcoded, it may not be of much use for me.

Not sure I follow, but -vvvv disables the limit completely.

I will write docs to make this explicit later, but here is a quick verbosity/limit table:

0	240
1	240
2	240
3	2400
4+	No limit

@ssbarnea however it is not clear to me yet if you can see full output with low verbosity levels; as I commented, my patch should produce the same results up to verbosity<3, but you commented:

I tested this manually on an I am happy to say that I now see an unabridged AssertionError that is ~1500 characters long, regardless the verbosity level (tried 0, 1, 2)

ssbarnea · 2021-03-03T18:53:18Z

For me 240ch is not enough and v0-v3 is exactly what most users are using and I do not want to force people to tune verbosity level in order to be able to read a failure... not to mention that if this comes from a flaky test, they may not even be able to reproduce it once they increase the verbosity. Tuning verbosity is tricy for CI/CD, especially when the user encountering the error may not be the maintainer of the project, so he might not even know how to increase verbosity.

Ideally we should have an ini option that defines the censoring threshold. I would personally set it to ~4000 for projects where I need it while pytest by default can use the default proposed default of 240.

Did I explain the use-case well? -- Is anyone is curious about how a random test failure displays, take a look at https://github.com/ansible-community/ansible-lint/pull/1422/checks?check_run_id=2024534247#step:8:652 -- basically the "meaty" bit was censored.

nicoddemus · 2021-03-03T19:04:22Z

Did I explain the use-case well?

You did, thanks!

I agree the current defaults can be tweaked, I also think 240 is a tad limiting. We also discussed previously about separating terminal output from assertion output in the configuration file.

Regardless I think this would be a step in the right direction, because at least now the config object can be accessed by the assertion rewriting functions, which paves the way for further improvements.

However can you clarify this comment:

I tested this manually on an I am happy to say that I now see an unabridged AssertionError that is ~1500 characters long, regardless the verbosity level (tried 0, 1, 2)

This should not happen with my patch, that's why I want to understand how you got that result.

nicoddemus · 2021-03-20T11:43:39Z

As discussed in #8403, changed the verbosity to 2400 for -v, and unlimited for -vv and up.

bluetech

LGTM, great doc.

I suspect we will need to bring back some safety limit, but let's wait and see :)

src/_pytest/assertion/util.py

doc/en/how-to/usage.rst

src/_pytest/assertion/__init__.py

Fix pytest-dev#6682 Fix pytest-dev#8403

This was changed during the current docs restructing and we missed that it changed the label.

nicoddemus · 2021-03-26T10:28:20Z

This PR also fixes the current docs (c1e0570).

ssbarnea · 2021-03-26T10:33:13Z

Thanks everyone! This fix will clearly make it easier to debug test failures.

nicoddemus force-pushed the ellipsis-verbose-6682 branch from 5b87a59 to 3d4c3d8 Compare March 3, 2021 00:52

ssbarnea mentioned this pull request Mar 3, 2021

Document way to customize abbreviation (elipsis) on AssertionError #6682

Closed

bluetech reviewed Mar 3, 2021

View reviewed changes

RonnyPfannschmidt reviewed Mar 3, 2021

View reviewed changes

src/_pytest/_io/saferepr.py Show resolved Hide resolved

nicoddemus commented Mar 3, 2021

View reviewed changes

src/_pytest/_io/saferepr.py Outdated Show resolved Hide resolved

ssbarnea approved these changes Mar 3, 2021

View reviewed changes

nicoddemus mentioned this pull request Mar 5, 2021

Truncation threshold at different verbosity levels #8403

Closed

Base automatically changed from master to main March 9, 2021 20:40

nicoddemus force-pushed the ellipsis-verbose-6682 branch from 3d4c3d8 to d8965da Compare March 20, 2021 11:42

nicoddemus changed the title ~~Increase truncation threshold with -vvv, disable with -vvvv~~ Increase truncation threshold with -v, disable with -vv Mar 20, 2021

nicoddemus changed the title ~~Increase truncation threshold with -v, disable with -vv~~ Increase truncation threshold with -v, disable with -vv [needs squash] Mar 20, 2021

nicoddemus requested review from bluetech and RonnyPfannschmidt March 20, 2021 11:48

bluetech approved these changes Mar 20, 2021

View reviewed changes

nicoddemus commented Mar 25, 2021

View reviewed changes

src/_pytest/assertion/util.py Outdated Show resolved Hide resolved

nicoddemus commented Mar 25, 2021

View reviewed changes

doc/en/how-to/usage.rst Outdated Show resolved Hide resolved

nicoddemus commented Mar 25, 2021

View reviewed changes

src/_pytest/assertion/__init__.py Outdated Show resolved Hide resolved

Increase truncation threshold with -v, disable with -vv

be8d63e

Fix pytest-dev#6682 Fix pytest-dev#8403

nicoddemus force-pushed the ellipsis-verbose-6682 branch from 74201e6 to be8d63e Compare March 26, 2021 10:06

Fix plugin-list label and script

c1e0570

This was changed during the current docs restructing and we missed that it changed the label.

nicoddemus changed the title ~~Increase truncation threshold with -v, disable with -vv [needs squash]~~ Increase truncation threshold with -v, disable with -vv Mar 26, 2021

nicoddemus merged commit ddbc00d into pytest-dev:main Mar 26, 2021

nicoddemus deleted the ellipsis-verbose-6682 branch March 26, 2021 10:29

ssbarnea mentioned this pull request Jul 24, 2021

Abridged assertions still cannot be disabled by using -vv (ellipses) #8940

Closed

ssbarnea added this to the 7.0 milestone Jul 26, 2021

ssbarnea added the topic: reporting related to terminal output and user-facing messages and errors label Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase truncation threshold with -v, disable with -vv #8391

Increase truncation threshold with -v, disable with -vv #8391

nicoddemus commented Mar 3, 2021 •

edited

Loading

bluetech left a comment

bluetech Mar 3, 2021

nicoddemus Mar 3, 2021

bluetech Mar 3, 2021

nicoddemus Mar 3, 2021

nicoddemus commented Mar 3, 2021

ssbarnea left a comment •

edited

Loading

nicoddemus commented Mar 3, 2021 •

edited

Loading

ssbarnea commented Mar 3, 2021

RonnyPfannschmidt commented Mar 3, 2021

nicoddemus commented Mar 3, 2021 •

edited

Loading

ssbarnea commented Mar 3, 2021 •

edited

Loading

nicoddemus commented Mar 3, 2021

nicoddemus commented Mar 20, 2021

bluetech left a comment

nicoddemus commented Mar 26, 2021

ssbarnea commented Mar 26, 2021

Increase truncation threshold with -v, disable with -vv #8391

Increase truncation threshold with -v, disable with -vv #8391

Conversation

nicoddemus commented Mar 3, 2021 • edited Loading

bluetech left a comment

Choose a reason for hiding this comment

bluetech Mar 3, 2021

Choose a reason for hiding this comment

nicoddemus Mar 3, 2021

Choose a reason for hiding this comment

bluetech Mar 3, 2021

Choose a reason for hiding this comment

nicoddemus Mar 3, 2021

Choose a reason for hiding this comment

nicoddemus commented Mar 3, 2021

ssbarnea left a comment • edited Loading

Choose a reason for hiding this comment

nicoddemus commented Mar 3, 2021 • edited Loading

ssbarnea commented Mar 3, 2021

RonnyPfannschmidt commented Mar 3, 2021

nicoddemus commented Mar 3, 2021 • edited Loading

ssbarnea commented Mar 3, 2021 • edited Loading

nicoddemus commented Mar 3, 2021

nicoddemus commented Mar 20, 2021

bluetech left a comment

Choose a reason for hiding this comment

nicoddemus commented Mar 26, 2021

ssbarnea commented Mar 26, 2021

nicoddemus commented Mar 3, 2021 •

edited

Loading

ssbarnea left a comment •

edited

Loading

nicoddemus commented Mar 3, 2021 •

edited

Loading

nicoddemus commented Mar 3, 2021 •

edited

Loading

ssbarnea commented Mar 3, 2021 •

edited

Loading