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

len(list) should be summarized in most cases #8

Closed
pytestbot opened this issue Nov 30, 2010 · 11 comments
Closed

len(list) should be summarized in most cases #8

pytestbot opened this issue Nov 30, 2010 · 11 comments
Labels
type: bug problem that needs to be addressed

Comments

@pytestbot
Copy link
Contributor

Originally reported by: BitBucket: wtanksleyjr, GitHub: wtanksleyjr


I have some code which runs:

assert len(mylist) == 2 *** *** 10

When this fails, py.test prints out something like:

  • len([something, more, else, thing, bla, ...]) == (2 *** *** 10)

The problem is that it's impossible to tell exactly why the assert is failing -- the whole list can't reasonably be printed, and truncating the list loses the information that actually matters.

If a list inside a len is truncated, it would make sense to either display another level of simplification OR to print only the length (since you can't display the whole list).


@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


fix issue8 : avoid errors caused by logging module wanting to close already closed streams.

The issue arose if logging was initialized while capturing was enabled
and then capturing streams were closed before process exit, leading
to the logging module to complain.

@pytestbot
Copy link
Contributor Author

Original comment by BitBucket: wtanksleyjr, GitHub: wtanksleyjr:


What you fixed (cool, thanks!) doesn't sound like what I was asking about. (Is this the fix for my other bug report, for asserts in doctest? If so, that's great -- that's a MUCH more debilitating bug.

Attached is a simple test that demonstrates my problem for this report. Notice how it's impossible to diagnose the failure from the printout; now imagine how hard it'd be to figure out if the code wasn't deliberately simple :-).

I'm not changing the state of this issue, because I'm not sure how to test this (I used pip to install, I don't know how to force it to upgrade using bitbucket).

@pytestbot
Copy link
Contributor Author

Original comment by BitBucket: wtanksleyjr, GitHub: wtanksleyjr:


I tested this using the attachment "test3.py" (attached when I wrote the above comment), and confirmed that it is not fixed. I've changed the issue to open, if you don't mind...

@pytestbot
Copy link
Contributor Author

Original comment by Floris Bruynooghe (BitBucket: flub, GitHub: flub):


Hi

I've had a look at this and it seems that the code actually goes out of it's way to hide the actual value the function evaluates for builtin functions as well as for functions which return a boolean. After applying the attached patch your test3.py shows as:

{{{
py.test test3.py
============================= test session starts ==============================
platform linux2 -- Python 2.6.6 -- pytest-2.0.1.dev5
collected 1 items

test3.py F

=================================== FAILURES ===================================
_____________________________ test_show_long_list ______________________________

def test_show_long_list():
    x = list(range(1,2**10))
  assert len(x) == 2**11

E assert 1023 == (2 ** 11)
E + where 1023 = len([1, 2, 3, 4, 5, 6, ...])

test3.py:3: AssertionError
=========================== 1 failed in 0.05 seconds ===========================
}}}

To me this seems like enough, what do others think?

However since this was explicitly done and even mentioned in the comments I wonder if someone still knows what the reason was to do this?

Regards
Floris

@pytestbot
Copy link
Contributor Author

Original comment by Anonymous:


That looks quite sufficient to me. I can't speak for the author, though.

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


Floris: this looks good - care to put it to your upstream repo, with the above example as test, and see that nothing else breaks, so that i can merge it?

@pytestbot
Copy link
Contributor Author

Original comment by Floris Bruynooghe (BitBucket: flub, GitHub: flub):


I'm still a bit hesitant to simply apply this as the code explicitly went out of it's way to do think differently. I guess I need to look some more at how all the failure demos etc look.

@pytestbot
Copy link
Contributor Author

Original comment by BitBucket: wtanksleyjr, GitHub: wtanksleyjr:


I admit that old code looks like it knows what it's doing -- that's a long way to go in order to make things less convenient.
My first guess was that it was trying to avoid some dangerous builtins, like open() and such; but looking at the code I see that it's also trying to avoid expanding functions that return bool! What's going on there? There's nothing dangerous about returning bool.

I give up... I have no idea why this code is there. I only know that while it was there, I had to write and run a manual test (with prints).

-Wm

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


i think it makes sense to go for Floris patch. If no tests break and a new test passes, let's try it out and see if anything unexpected happens.

(i don't currently know what the reason was TBH - the hack was introduced by Armin IIRC and it might actually have to do with PyPy interactions)

@pytestbot
Copy link
Contributor Author

Original comment by Floris Bruynooghe (BitBucket: flub, GitHub: flub):


I've prepared the fix in the "len" patch on https://bitbucket.org/flub/py-queue

All the tests for py as well as pytest pass with this patch applied and all seem to pass correctly (only tested with python2.6 so far). All the examples and failure demos seem to produce good output too.

Any feedback on whether the added test and changelog are sufficient would be appreciated.

@pytestbot
Copy link
Contributor Author

Original comment by Floris Bruynooghe (BitBucket: flub, GitHub: flub):


Committed in https://bitbucket.org/hpk42/py/changeset/71dfb239aa60

@pytestbot pytestbot added the type: bug problem that needs to be addressed label Jun 15, 2015
nicoddemus added a commit to malinoff/pytest that referenced this issue Dec 14, 2016
davidszotten pushed a commit to davidszotten/pytest that referenced this issue Oct 13, 2018
fkohlgrueber pushed a commit to fkohlgrueber/pytest that referenced this issue Oct 27, 2018
Trailing commas after * or ** in a function signature are only safe for Python 3.6
code.  So now Black checks whether the file was already Python 3.6 to begin
with.  If so, trailing commas are used in such cases.  Otherwise, they're not.

When * and ** don't appear in a function signature, the trailing comma is
always safe.

Fixes pytest-dev#8
mgorny pushed a commit to mgorny/pytest that referenced this issue May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

1 participant