chmp / ipytest

Pytest in IPython notebooks.
MIT License
314 stars 17 forks source link

Pre-release shows duplicated collection #53

Closed chmp closed 3 years ago

chmp commented 3 years ago

Tests are collected multipled times.

# cell 1
import ipytest
ipytest.autoconfig()

# cell 2
%%run_pytest[clean]

def test_bar():
    pass

def test_foo():
    pass

# runs 4 tests
# cell 1
import ipytest
ipytest.autoconfig(defopts=False)

# cell 2
%%run_pytest[clean]

def test_bar():
    pass

def test_foo():
    pass

# runs 2 tests

Somehow the ".py" suffix seems to interact with pytest's test collection. Can also be reproduced with 0.9.1 and register_module=True.

pmeier commented 3 years ago

Do you already have a plan fixing this? Otherwise would you accept a PR in case I find the cause?

chmp commented 3 years ago

Always happy about contributions :)

My guess is: if the temporary file has a ".py" suffix and the module registered with Python, it is found multiple times, once via the Plugin and once via the normal PyTest mechanism. Probably just not adding the plugin, when module registration has taken place is enough. I.e., not adding plugin here

Potential corner cases:

pmeier commented 3 years ago

@chmp I gave it a go but got nowhere. Debugging pytest is a pain and having the failure only in a notebook makes matters even worse. Here are some pointers for you:

chmp commented 3 years ago

Thanks for trying and the pointers. TBH, everytime I look again into ipytest, I feel like there is way too much magic involved with pytest. That's also why I was quite excited for you to give a try :)

pmeier commented 3 years ago

I had my fair share with debugging pytest plugins. Having hooks for everything is really nice for plugins, but it is a nightmare to debug, because simply entering some functionality adds multiple layers on top of the stack. Plus, many of the internal functions are reused multiple times so just by hitting a break point you can never know at what stage of the run you currently are...

If you are the plugin author you only need to handle pytest, but in my case here I also wrangled with ipytest. Not saying that I wouldn't have found the source and in turn a solution eventually, but I currently don't have that much time to sink into this.

chmp commented 3 years ago

So the issue seems to be that a module with a ".py" file will also be collected by the normal pytest collection. Technically speaking, only if it can be imported as a module, but ipytest will try to make sure this is the case.

Counter examples: when setting the __file__ = 'Untitled.iypnb' there is no double collection. Also if the .py file corresponds to an existing file, that was already imported and contains no test, there is no double collection.

The easiest solution seem to be:

This solution would actually make the implementaiton much simpler, but I have to think about, whether that's really a good idea.

chmp commented 3 years ago

So I implemented these changes in the feature/remove-module-collector branch and released them as ipytest==0.10.0b2. Just in case you would like to give it a go. I will also try myself and see whether the changes break anything.

pmeier commented 3 years ago

Works as expected. Thanks a lot!

chmp commented 3 years ago

Released :)