--last-failed --last-failed-no-failures none is now collecting much more tests than it did in 5.4.3

[symonk]

Hi, in work I have been recently upgrading from pytest 5.4.3 -> 6.1.2, I observe a behaviour discrepency in the:

pytest_collection_modifyitems hook, in 5.4.3 given our setup we get the following:

pytest --last-failed --last-failed-no-failures none 

    # internal plugin registered via pytest_plugins=[]

    @pytest.hookimpl(trylast=True)
    def pytest_collection_modifyitems(self, items: List[Item]) -> None:
        try:
            for item in items:
                log.debug(f"collected item: {item.name}")
            do_something()
        except Exception as exc:
            pytest.exit(str(exc))

In the above snippet, the import thing is len(items) is 2. inline with this:

{
  "scrubbed/sensitive/info/test_two.py::TestTwo::test_two": true
  "scrubbed/sensitive/info/test_one.py::TestOne::test_one": true
}

I upgraded to 6.1.2 and with the same last failed cache highlighted above, len of the items in my hook implementation above is now pretty much every single test we have, 2600+.

note: This is barebones at the moment, scrubbing sensitive info is going to take a while

[symonk]

p.s i'm aware that 6.X is a massive amount of changes, potentially breaking 👍 i'll run my test with all versions to spot the introduction and study release notes.

[nicoddemus]

A hunch to help your investigation: your hook is no longer being the last hook executed, so it is now getting the full test suite. 🤔

[symonk]

@nicoddemus im rolling back decrementally, the behaviour appears in 6.0.1 but not 5.4.3. Also any idea which plugin of pytests is handling the initial collection? I assume Session ? i wonder if it had a modification to its hookimpl decorator? We have two internal implementations of this hook, both running in order as far as our code is concerned, however i can't guarantee external/core plugins aren't changing that

[symonk]

also @nicoddemus this only really happens with --lf is used, what is the easiest way to see all modifyitems hooks that are executing and in the order, is --debug + grep useful there?

[symonk]

it does exhibit behaviour, almost as if the hook is tryfirst=True rather than trylast=True will debug pluggy code and compare the difference in hook order for modifyitems

[symonk]

lf plugin now has (I assume this is relatable ):

    @pytest.hookimpl(hookwrapper=True, tryfirst=True)
    def pytest_collection_modifyitems(
        self, config: Config, items: List[nodes.Item]
    ) -> Generator[None, None, None]:
        yield 
        # this yield is forcing my plugins hook to run before any items have been modified in place -.-'

vs 5.4.3:

    def pytest_collection_modifyitems(self, session, config, items):
        if not self.active:
            return

initial attempt wrapping my hook in (hookwrapper=True, trylast=True) didnt resolve it, my post-yield hook is executing after the cacheproviders for some reason. If my thoughts are correct is this not a massively breaking changed to --lf functionality for consumer projects, I'd of thought if i make my hook yield and trylast=True, it would come after the LF plugins modifyitems, which doesnt seem to be the case?

from docs, maybe I am misunderstanding:

It’s possible to use tryfirst and trylast also in conjunction with hookwrapper=True in which case it will influence the ordering of hookwrappers among each other.

cc: @nicoddemus any idea?

edit: ah, I would need to make my wrapper, tryfirst=True also?

[symonk]

It works now, i cannot understand how hookwrapper=True, tryfirst=True made this work, shouldn't it of been trylast=True (it was broken still with this), given I wanted my hook to run last, perhaps the rabbit hole of LF/NF plugins modifyitems is not the culprit, but another instance of the hook, from what I gather there are

4x core plugins influencing this:

NFPlugin -> hookwrapper=True, tryfirst=True
LFPlugin -> hookwrapper=True, tryfirst=True
Stepwise -> standard hook no influence
FixtureManager -> standard hook no influence

I suspect I/We must have a dependency plugin that is doing something weird with items in place? we only have 2 implementations of modifyitems and both of them do not influence List[Item] in-place whatsoever.

To me, regardless of what other external / dependency plugins are doing, a:

@pytest.hookimpl(hookwrapper=True, trylast=True (with a post-yield payload only) should have my desired effect, its almost as if a plugin is adding items to the list of items before it gets to my hook, as mine should definitely be running after all of the built ins. Will debug pluggy _multicall on Monday morning to see whats going on, any advice in advance would be appreciated in where best to hunt

[symonk]

on further inspection, this also exists & one in main but they are standard hooks; not wrappers or tryfirst/last

def pytest_collection_modifyitems(items: "List[Item]", config: Config) -> None:
    deselect_by_keyword(items, config)
    deselect_by_mark(items, config)

main:

def pytest_collection_modifyitems(items: List[nodes.Item], config: Config) -> None:
    deselect_prefixes = tuple(config.getoption("deselect") or [])
    if not deselect_prefixes:
        return

    remaining = []
    deselected = []
    for colitem in items:
        if colitem.nodeid.startswith(deselect_prefixes):
            deselected.append(colitem)
        else:
            remaining.append(colitem)

    if deselected:
        config.hook.pytest_deselected(items=deselected)
        items[:] = remaining

It seems that when leaving the NFPlugin post-yield, the items are then correctly filtered and this is the culprit. Should I be passing a -m to --lf? we never used to in 5.4.3 etc, I'm trying to pinpoint if its something in our setup causing the strangeness.

Another discrepency (I think) is my root conftest.py hook (not the one shown in the original hook, this one is avery simple implementation) has the following:

@pytest.hookimpl(hookwrapper=True, tryfirst=True)
def pytest_collection_modifyitems(items):

yet it is executing before the NF/LF plugin wrappers, so I have:

-> root conftest.py modifyitems hook (hookwrapper=True, tryfirst=True)
-> root conftest.py loads pytest_plugins=['myplugin']
-> myplugin has a modifyitems hook (hookwrapper=True, tryfirst=True)
-> Yet pluggy is invoking the 4x hook wrappers like so:
  -> Pre yield [myplugin]
  -> Pre yield [NFplugin]
  -> Pre yield [LFPlugin]
  -> Pre yield [conftest.py]

That seems strange, I would assume conftest/myplugin are loaded at a similar time (after core plugins?) and would be next to each other as far as order is concerned. I will put together a small POC recreatable to follow this post.

[symonk]

@nicoddemus I put together a simple POC here, that does not highlight the issue, which is a similar basic setup to mine in work, the important thing to note here with this small poc is that when entering the _multicall for pytest_collect_modifyitems the items are already filtered correctly before even a hookwrapper has a chance at modifying them in place, from this i can conclude that the change in behaviour must be to do with our layout and something between 5.4.3 -> 6.0.1 changed what pytest is collecting etc for our given setup, now just to figure out what that is exactly...? and why @hookwrapper=True, tryfirst=True makes it even work, it seems insane.

# lib/myplugin.py
import pytest


@pytest.hookimpl
def pytest_collection_modifyitems(items):
    breakpoint()
    print(items)

# root conftest.py
pytest_plugins = ['lib.myplugin']

import pytest


@pytest.hookimpl
def pytest_collection_modifyitems(items):
    breakpoint()
    print(items)

# tests/test_one.py


def test_bar():
    assert False

# test_two.py



def test_foo():
    assert True

with lastfailed cache:

{
  "tests/test_one.py::test_bar": true
}

In this POC, when we enter _multicall, not a single collection_modifyitems hook has executed (even pre-yield at this point) and we see:

<HookImpl plugin_name='nfplugin', plugin=<_pytest.cacheprovider.NFPlugin object at 0x10a8cf650>>
(Pdb) pp args
[[<Function test_bar>]]

This is completely in contrast to my work setup, seems like a fundamental change to how its collecting tests in our project, I've noticed some of the (now 2600) tests listed, some of them are tests that never got collected before we moved to pytest 6.x (and weirdly why does hookwrapper=True, trylast=True in our plugin make it work? :/ can you point me where to look at what is building the items before handing them off to collection_modifyitems? as I feel thats where the fundamental difference lies here.

Contrast the above to my work scenario, once again no pre-yield hookwrappers have even fired at this point:

(Pdb) hook_impl
<HookImpl plugin_name='nfplugin', plugin=<_pytest.cacheprovider.NFPlugin object at 0x111525650>>
(Pdb) len(args[0])
2590

[symonk]

cc: @bluetech any ideas where to start hunting this one down? I really need to look at our packaging/layout in work, maybe its weird

[bluetech]

I haven't grokked everything you wrote yet but some notes:

The behavior changed in d530d70 which was first released in pytest 6.0.0rc1. But sorry, I don't really know why it was changed.

Regarding the ordering,

    @pytest.hookimpl(hookwrapper=True, tryfirst=True)
    def pytest_collection_modifyitems(
        self, config: Config, items: List[nodes.Item]
    ) -> Generator[None, None, None]:
        yield 

The hookwrappers always run before non-wrapper hooks; and tryfirst=True runs before non-tryfirst hookwrappers, so it tries really hard to run before everything else. Now, teardowns (code after the yield) run in reverse order, meaning this tries really hard to run after everything else, which is what you're seeing.

What happens when you use hookwrapper=True, tryfirst=True? I think in this case, it depends on the order in which the plugins (builtins, entry points, conftests) are registered.

[symonk]

@bluetech it works with hookwrapper=True, tryfirst=True, items are filtered correctly into the plugins collection_modifyitems, tho it seems a bizarre change from 5.4.3. I notice before any collection_modifyitems hook is invoked on 5.4.3 in work, items are correctly filtered for --lf / --no-failures. yet on 6.0.1, the item list is huge when entering the hookimpl