main: move norecursedir check to main's pytest_ignore_collect

[bluetech]

Fix #11081

[RonnyPfannschmidt]

nice, this also brings the implementation and the config for it back together

[pllim]

Hi. I suspect this might have broken our test collection downstream at https://github.com/astropy/astropy . I see that it is collecting stuff that we explicitly ignored in setup.cfg like this:

[tool:pytest]
norecursedirs =
    "astropy[\/]extern"

Despite that directive, I see:

../../.tox/py311-test-devinfra/lib/python3.11/site-packages/astropy/extern/configobj/configobj.py F
../../.tox/py311-test-devinfra/lib/python3.11/site-packages/astropy/extern/configobj/validate.py F

Is there some changes I need to do downstream to account for this new pytest behavior? Thanks!

Failed log: https://github.com/astropy/astropy/actions/runs/5191708098/jobs/9359883509
pytest-7.4.0.dev118+g1a175390

Last success log: https://github.com/astropy/astropy/actions/runs/5173838762/jobs/9319503854
pytest-7.4.0.dev109+ge1204e1e

[bluetech]

Thanks for testing pytest main @pllim, it helps a lot.

I don't have time to confirm this ATM, but this is almost certainly due to a plugin which "overrides" pytest_ignore_collect. I didn't find one in the astropy repo, but I did find a couple in the astropy organization:

• https://github.com/astropy/pytest-doctestplus/blob/deff8fd34bba614141d5cb34c349d54c29a3c394/pytest_doctestplus/plugin.py#L505
• https://github.com/astropy/pytest-filter-subpackage/blob/44747eb7a6d1a2e22587dd78f69fa04d915b3dad/pytest_filter_subpackage/plugin.py#L21

To explain a bit, pytest_ignore_collect is a firstresult=True hook, which means that the first plugin which returns a non-None result decides the outcome (it is still possible to override using "hook wrappers" but that's not the case here).

The pytest-filter-subpackage impl seems OK to me, it either returns True (want to ignore) or None (don't decide).

The pytest-doctestplus impl however, does return False at the end, which means it explicitly "takes over" and asks not to ignore the path without consulting further plugins, in addition to tryfirst=True, which means it runs first.

So I believe that if you change this line to return None (or just remove it) it will fix the issue:

https://github.com/astropy/pytest-doctestplus/blob/deff8fd34bba614141d5cb34c349d54c29a3c394/pytest_doctestplus/plugin.py#L557

Note that it will change the behavior such that pytest's builtin ignore logic will now run, when it didn't previously. If that's a problem, the only fix is to just copy over the norecursedir logic from pytest.

---

If you can confirm the above that'd be great, I think it will be a good change to make regardless. However, I'll be taking a look at how plugins at large implement pytest_ignore_collect and if it looks like it will cause enough breakage then we can just revert it.

[pllim]

Thank you very much for the thorough analysis and recommendation, @bluetech ! I think your proposed solution looks very promising over at scientific-python/pytest-doctestplus#201 . 😺

[bluetech]

I'll be taking a look at how plugins at large implement pytest_ignore_collect and if it looks like it will cause enough breakage then we can just revert it.

Did this now. Out of the ~700 plugins I have checked out locally, 11 implement pytest_ignore_collect, 1 implements incorrectly (2 if including pytest-doctestplus).

I also did a github search, there aren't many hits, but I'd say it's 10%-20% incorrect implementations.

Overall my impression is that the breakage won't be wide-spread. The breakage will be visible and the fix is backward compatible - no compat issues. In addition to the fact that it is already subtlety broken.

So I think we shouldn't revert. I will however add a note to the changelog entry about this.