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

Weird failure with pytest and multiline assert #61

Open
alexmojaki opened this issue Nov 4, 2022 · 8 comments
Open

Weird failure with pytest and multiline assert #61

alexmojaki opened this issue Nov 4, 2022 · 8 comments

Comments

@alexmojaki
Copy link
Owner

See gruns/icecream#134

@15r10nk not sure if this is related to your recent work

@15r10nk
Copy link
Collaborator

15r10nk commented Nov 4, 2022

  • The first bug happens because pytest is changing some global code (some imports).
  • I don't understand why the test_icecream_fine works. It should not because of the assert.
  • test_icecream_bug is also interresting.

I will try to reproduce it later in some executing tests.

@15r10nk
Copy link
Collaborator

15r10nk commented Nov 4, 2022

I reduced the problem to the following code, which is reproducible without icecream.

from executing import Source
import inspect


print("\n\nFirst bug")
print(Source.executing(inspect.currentframe()).node)


def test_icecream_fine():
    print("No bug")
    print(Source.executing(inspect.currentframe()).node)
    assert (True)
    1

def test_icecream_also_fine():
    print("No bug")
    print(Source.executing(inspect.currentframe()).node)
    assert (True
    )
    

def test_icecream_bug():
    print("Second bug")
    print(Source.executing(inspect.currentframe()).node)

    assert (True # this linebreak is important
    ) 

    1 # this is also important

output

❯ pytest test_icecream.py -s
========================================= test session starts ==========================================
platform linux -- Python 3.10.0, pytest-7.2.0, pluggy-1.0.0
rootdir: /home/frank/projects/executing
collecting ...

First bug
None
collected 3 items

test_icecream.py No bug
<ast.Call object at 0x7f8013ba36d0>
.No bug
<ast.Call object at 0x7f8013ba32b0>
.Second bug
None
.

========================================== 3 passed in 0.02s ===========================================

Can this line have something to do with this behaviour?

if not self.is_pytest:
checks += [
attrgetter('co_names'),
attrgetter('co_varnames'),
]

I don't know how the SentinelNodeFinder works. Any hints?

@15r10nk
Copy link
Collaborator

15r10nk commented Nov 4, 2022

The previous code checked for pytest in the source to detect assert rewriting. I think this would not detect this case because there is no pytest in the code.

Theory:

  • self.is_pytest was False before i integrated pytest.
  • The SentinelNodeFinder found the correct Node.
  • some strange behavior gets now triggered because is_pytest is actually True

@alexmojaki
Copy link
Owner Author

Can this line have something to do with this behaviour?

That line is saying to be less strict about things being the same in the presence of pytest. Detecting pytest should make it more likely to find the node, not less.

The previous code checked for pytest in the source to detect assert rewriting. I think this would not detect this case because there is no pytest in the code.

Look at the change in 777ee37, particularly the previous code:

        self.is_pytest = any(
            'pytest' in name.lower()
            for group in [code.co_names, code.co_varnames]
            for name in group
        )

It wasn't "is pytest mentioned in the source code", it was whether it was in any of the variable names. In particular that was meant to also include the names created by pytest magic. When I dis.dis(test_icecream_bug) I see a name @pytest_ar which should be detected by both my old method and your new method.

Anyway, when I run either your example or the icecream one on my machine, I only get the first bug, not the second. This is with both Python 3.8.5 and 3.11, and with pytest 7.2.0.

@15r10nk
Copy link
Collaborator

15r10nk commented Nov 4, 2022

You are right, is_pytest is True for the old and the new latest version of executing.
The check should not be the problem.
I get also the same error.

I use python 3.10.0 and pytest 7.2.0

@alexmojaki
Copy link
Owner Author

Right, I also see the failure in 3.10. Things went bad in 3.10, some optimisations were added that were very hard to handle. If #42 ever happens it might help. I see that find_new_matching isn't yielding anything but I don't think it's worth investigating why.

@15r10nk
Copy link
Collaborator

15r10nk commented Nov 4, 2022

I working towards #42. I work currently on #55. But I can make no promises on the timeline.

A workaround for this issue might be to disable the assert rewriting for the specific file.

"""
module docstring ...

PYTEST_DONT_REWRITE
"""

@alexmojaki
Copy link
Owner Author

I working towards #42. I work currently on #55.

🎉 🎉 🎉 🎉 🎉
🔥 🔥 🔥 🔥 🔥
🎆 🎆 🎆 🎆 🎆

But I can make no promises on the timeline.

no worries, no pressure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants