-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Regression in 7.1 ? Windows CI started to fail with "assert mod not in mods" since 7.1 #9765
Comments
Thanks for the report, I will take a look when I get a chance. 7.1 had some internal conftest refactorings. If you manage to provide a reproduction that'd be great. |
One thing that seems somewhat suspicious is how IPython's |
Yeah, it might be something on our end, we have to do some pretty weird things seeing we also an interpreter. So I would not be surprised if the problem is from our plugins. |
It does seem like removing the plugin fixes the issue, Feel free to close, worse case I'll run the plugin only on linux CI. |
Not sure if it helps, I know it's not a slim repro setup, but this repo was just pinned to |
@gonzalocasas and @bashtage I see both your CI install IPython in them, could you try pinning IPython to |
@Carreau I get the same error when I |
I can also get this error when I reun specific tests:
|
I've got the same error, but on linux! Can't run any test on 7.1.x, reverting back to 7.0.x fixes the issue:
|
FYI: I also get this in an rpmbuild of asdf for openSUSE Linux. I attribute it to some import mismatch for the conftest.py installed into the sitelib. Fails:Have asdf installed in sitelib but collect from asdf in source directory. This worked with pytest < 7
Works:move the asdf source directory away and directly test with
|
Any chance this can be related to finding multiple So we've reverted and are pinning pytest to 7.0.1 for now. Would be great to see this fixed, or provide a migration path for how to deal with it in 7.1.0. We are in a monorepo, and use the pytest runner for the entire repo (at the root). Has never been an issue in the past. |
'assert mod not in mods' when run on Windows. See pytest-dev/pytest#9765
This could very well be the case. I had a colleague who had the same issue on Windows. It turned out to be triggered by him creating his the conda test environment as a subfolder inside the test folder (Not recommended) So pytest was picking up an extra |
Just to add to this, the error occurs in my repo (issue 162) even when conda is set up properly (not as a subfolder like @melund describes). My repo only has a single conftest.py too. Leads me to believe this issue is happening in conda environments where installed dependencies also have conftest.py files. Maybe pytest is incorrectly grabbing conftest files from package dependencies. Is it that a possibility? |
conftests are only ever searched locally, so unless you put evil magic in your tests folders you shouldnt see this |
lol good to know. I must have done some evil magic without realizing it 😅 |
@h-vetinari no, that's not right, the CI job that disables isolation is passing |
I believe the wheels builds do disable build isolation though, because we pull in NumPy pre-release in anticipation of NumPy 2.0.0. Just not that specific CI job that uses Python 3.11 in our regular CI. |
@pllim can you try with (cherry-picked the commit but using the 0.8.x branch):
and post the output? I guess @tylerjereddy you can do this as well? |
I agree with this, thanks for looking into it and your very clear summary! I realised only now there was a question at the end of your summary post 😅. As far as I am concerned, 1. seems like the first straightforward thing to do. Without being super familiar with the pytest code, it is a bit hard to guess if 3. needs to be done but I feel this can be done in a second step.
With @fcharras we'll try to work on this PR on Monday. I recently have worked on turning my reproducer into a pytest test so I think we should be close to having something mergeable. |
@lesteve , thanks for the updated branch! I got a log but I don't understand how this error is related to "assert mod not in mods". I have not seen this error before as far as pytest 8.0rc is concerned. https://github.com/spacetelescope/jdaviz/actions/runs/7439536734/job/20239554633?pr=2654 |
@pllim not sure what is going on, ideally you would cherry-pick my simple debug commit on top of whichever version you were seeing the |
@tylerjereddy looking closely at the Scipy CI logs, it could potentially be fixed by #11708, this is a
Here is one relevant excerpt from your Python 3.10 log. You need to look at
|
@lesteve , I ended up forking 8.0rc1 on my fork and then cherry-pick your commit directly on top of that. I think this is the log you are requesting? Does this mean anything to you? Thanks! https://github.com/spacetelescope/jdaviz/actions/runs/7450389588/job/20269180187?pr=2654 |
@pllim this seems very similar to the Scipy one,
I think this could be fixed by #11708. Hopefully we manage to push the PR forward tomorrow. |
Thanks, @lesteve ! I will be happy to test that PR when it is ready. |
@pllim and maybe @h-vetinari or @tylerjereddy, can you try installing the code from #11708 and see whether that fixes your issue 🤞?
|
This comment was marked as outdated.
This comment was marked as outdated.
@lesteve , I applied the patch from fcharras to 8.0rc1 directly at https://github.com/pllim/pytest/tree/debug-8.0rc1 (pllim@22949f2). It went further this time but errored out because it cannot find test fixtures. We have a root level Log: https://github.com/spacetelescope/jdaviz/actions/runs/7465954413/job/20316247352?pr=2654 For completeness, I can confirm that this problem only still happens on Windows. OSX passed and Linux failed with some transient remote data access problem that is unrelated to pytest itself. |
OK, so at least you avoid the Your debug branch looks fine to me. Right now, I don't really understand how such a change could cause some fixtures to not be registered. It could potentially be another pytest 8.0rc1 regression, hard to tell since I am by no means a pytest expert ... |
Yes, I agree that the "assert mod not in mods" problem is solved in #11708 |
I also just found out that this "mod not in mods" problem on Windows is not a problem at all if I switch from Python 3.10 to Python 3.11 🤪 🤷♀️ |
Refs pytest-dev#9765. While this is probably not needed for fixing the `assert mod not in mods` bug (previous commit takes care of it), I believe this is the right thing to do to avoid other inconsistencies (see discussion in issue).
For reference, in case this needs to be reproduced (Windows 11, python 3.8.10):
This works fine (well there is an issue with the
|
@woutdenolf this is similar to I think this is fixed in I double-checked and I don't get the
|
For reference, the simple "Solution 3" normalize-as-early-as-possible approach (implemented here) does not work -- the problem is that, while |
The link is broken. What do you mean by "normalize"? Normalize what and how? I don't see the problem. You cannot import I'm asking because I made a proposal that normalizes |
The correct link for the commit is 203889b |
Now sorry I know this is not a minimal reproducer, but I just want to log this here, and will investigate later:
You can see one Failure here
Now I'm not sure what this does, but my quick understanding is that it checks whether
conftest.py
itself is not in the list of collected modules ? We might definitely do something wrong on IPython side, but it's strange as other non-windows CI are passing and regardless of whether this is something I did wrong, can the assert get an explanation messages as second argument ?Downgrading to
<7.1
fixes the issue.Again I'll try to get a MVP/small example, but it may take me some time, and I guessed maybe one of you will immediately know why this is happening.
pip list
from the virtual environment you are usingThe text was updated successfully, but these errors were encountered: