Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow customizing showlocals cutoff #2382

Closed
emil-petersen opened this issue Apr 26, 2017 · 15 comments
Closed

Allow customizing showlocals cutoff #2382

emil-petersen opened this issue Apr 26, 2017 · 15 comments
Labels
status: help wanted developers would like help from experts on this topic type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@emil-petersen
Copy link

The -l flag - showlocals - for PyTest allows a test to include the state of local variables in case of test failure.

However, it seems there is a hard-coded limit of 70 characters in this output. From diving through the code, I believe this happens in the SafeRepr object in py.

This is an issue in my case because the local variables are dictionaries, and trying to understand a test failure is heavily reliant on the exact content of those variables.

I can log them myself, but doing so defeats the purpose of having -l available.

Therefore, I would like to request the ability to configure the cutoff for showing locals. Thank you.

@emil-petersen emil-petersen changed the title Allow specifying showlocals cutoff Allow customizing showlocals cutoff Apr 26, 2017
@RonnyPfannschmidt RonnyPfannschmidt added type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch status: help wanted developers would like help from experts on this topic type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature labels Apr 27, 2017
@nicoddemus nicoddemus added type: enhancement new feature or API change, should be merged into features branch and removed type: enhancement new feature or API change, should be merged into features branch labels Sep 14, 2017
@ApaDoctor
Copy link
Contributor

What is the best way to make possible to configure the unlimited locals output?
In config or as command line argument?

@nicoddemus
Copy link
Member

Today -l is a bool flag, I believe we can change it to be an int flag instead, so users can still pass -l and obtain the default 70 characters cuf-off, or -l 200 to set the cut-off to 200.

@emil-petersen
Copy link
Author

@nicoddemus Assuming it could easily be set in pytest.ini, that would be a great solution from my perspective.

@The-Compiler
Copy link
Member

Today -l is a bool flag, I believe we can change it to be an int flag instead, so users can still pass -l and obtain the default 70 characters cuf-off, or -l 200 to set the cut-off to 200.

I don't think that'd work - presumably -l would then always eat the following argument and show an error if it isn't an int?

@nicoddemus
Copy link
Member

@The-Compiler yeah when I proposed I wasn't sure it would work, I would say it is worth trying.

If that doesn't work we will need a separate variable for that, I suggest -l-cutoff=X which sets the cut-off to X and implies -l.

@ApaDoctor
Copy link
Contributor

Cut-off to 200 is problem. Because cutoff implemented in py module.
And _strrepr allows to change only string length, but not other objects.
So to implement possibility to change cut-off limits we need to change 'py' module first and then provide changes to pytest.
I think great solution is to allow to off cutoff.
And after release of py changes we can make possible to change cut-off size in oytest.

@nicoddemus
Copy link
Member

Cut-off to 200 is problem. Because cutoff implemented in py module.

I believe this is implemented in pytest._code:

def repr_locals(self, locals):
if self.showlocals:
lines = []
keys = [loc for loc in locals if loc[0] != "@"]
keys.sort()
for name in keys:
value = locals[name]
if name == '__builtins__':
lines.append("__builtins__ = <builtins>")
else:
# This formatting could all be handled by the
# _repr() function, which is only reprlib.Repr in
# disguise, so is very configurable.
str_repr = self._saferepr(value)
# if len(str_repr) < 70 or not isinstance(value,
# (list, tuple, dict)):
lines.append("%-10s = %s" % (name, str_repr))
# else:
# self._line("%-10s =\\" % (name,))
# # XXX
# py.std.pprint.pprint(value, stream=self.excinfowriter)
return ReprLocals(lines)

Although here the 70 limit is commented out. Hmm this warrants more investigation.

@ApaDoctor
Copy link
Contributor

ApaDoctor commented Oct 4, 2017

Yes, but at line 554 we call self._saferepr()
Which using py

    def _saferepr(self, obj):
        return py.io.saferepr(obj)

This is i talking about

@nicoddemus
Copy link
Member

@ApaDoctor I see thanks. Indeed py.io.saferepr has some hard-coded values.

@ApaDoctor
Copy link
Contributor

@nicoddemus exactly:)
So letls return to my proposal:))

So to implement possibility to change cut-off limits we need to change 'py' module first and then provide changes to pytest.
I think great solution is to allow to off cutoff.
And after release of py changes we can make possible to change cut-off size in pytest

@nicoddemus
Copy link
Member

nicoddemus commented Oct 5, 2017

We might consider just copying the saferepr code and modify it as needed; pylib is in maintenance mode and has to be 2.6 compatible, while in pytest we have no such restriction. The py.io.saferepr module has only 70~ lines after all. @RonnyPfannschmidt what do you think about this approach?

@RonnyPfannschmidt
Copy link
Member

as we start to copy more and mnore sublibs of pylib into pytest i start to seriously question that approach

@nicoddemus
Copy link
Member

@RonnyPfannschmidt in your opinion should we change pylib's py.io.reprlib then?

@blueyed
Copy link
Contributor

blueyed commented Apr 23, 2018

Relevant code: https://github.com/pytest-dev/py/blob/5f1f794f5c5aa25802ea61b4430648438c8d4b93/py/_io/saferepr.py#L59-L71 - it uses maxsize=240 by default.
Python's Repr's has some more limits: https://github.com/python/cpython/blob/20d68dfcc07bd389ce2ea7b0773c44d97ebeb68d/Lib/reprlib.py#L36-L49
It might be hard to transform --showlocals-length into that, but it could be applied just on the result then, and maxing out all other limits.
Something to consider test there then is recursion, i.e. without any limit (which does not seem to be supported by Repr itself, but could be hacked by using a never-decreasing int maybe) it might run into a RecursionError.

Apart from that I would say that with -v or -vv it should not apply limits - similar to other places where you need -vv to get the whole diff with assertion errors.

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2019

This should work with -vv via #3681 by now at least.
See #3962 for references / ideas to have a distinct option to have this not be bound to verbosity.

@blueyed blueyed closed this as completed Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted developers would like help from experts on this topic type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

No branches or pull requests

6 participants