Issue with duplicates handling in Pytest 8

[nonatomiclabs]

Following an upgrade to Pytest 8, we are seeing a change in the way duplicate items are handled, which does not seem logical/expected to me.

Current behavior

Let's assume we have the following directory structure:

> tree tests
tests
├── subdirectory
│   └── test_two.py
└── test_one.py

With Pytest 7, if we call pytest test_one.py tests --collect-only, it returns 3
tests:

============================= test session starts ==============================
platform linux -- Python 3.12.2, pytest-7.0.0, pluggy-1.4.0
rootdir: /home
collected 3 items

<Module tests/test_one.py>
  <Function test_one>
<Module tests/test_one.py>
  <Function test_one>
<Module tests/subdirectory/test_two.py>
  <Function test_two>

With Pytest 8, we get only one:

============================= test session starts ==============================
platform linux -- Python 3.12.2, pytest-8.0.2, pluggy-1.4.0
rootdir: /home
collected 1 item

<Dir home>
  <Dir tests>
    <Module test_one.py>
      <Function test_one>

After looking a bit at Pytest's internals, the change looks related to the refactoring done in #11646, where the duplicates handling logic was moved away from the former _collectfile method (which, I guess, operate on a file-per-file basis), to be consolidated in genitems() (in which a single duplicate in a node will result in the complete node being ignored).

Expected behavior

I would expect test_one.py not to prevent the further collection of tests in subdirectory, even though it's a duplicate.

To be honest, I'm also a bit puzzled by the previous behavior in Pytest 7, as I wouldn't have expected to see tests/test_one.py twice (as I didn't pass the --keep-duplicates option).

Are my expectations correct in the first place, or did I misunderstand the changes to the test collection and the behavior is expected?

Additional information

• With a bit of guidance, I would be willing to provide a patch (if it's a real bug of course 🙂)
• Reproduced on Pytest 8.0.2 / Linux & macOS
• I made a repository that contains a Dockerfile to reproduce the issue easily, please let me know if you need any additional details

[bluetech]

Thanks for the bug report @nonatomiclabs, I agree the new behavior is buggy. I will take a look.

The duplicate handling is a bit of a headache and as you said it was quite broken also before #11646.

[bluetech]

I started looking at this, but it's surprisingly tricky to come up with a self-consistent, intuitive and reasonably backward-compatible logic for the duplicate handling, if you think about it. Though I'll keep trying :)

[bluetech]

First, some explanations of the details that are relevant to duplicate handling, then some thoughts on how it should work.

Collection arguments

The collection arguments are the inputs that pytest starts collecting from. These are usually the positional command line arguments but can also be testpaths and others - doesn't matter.

A collection arg has two parts - the path and (optionally) parts within the file.

All of the collection args are given to Session.collect() which collects them and yields the initial set of nodes.

Session.collect()

Suppose the collection arguments are a/aa/aaa.py, /ab/. Then Session.collect() will produce two nodes (the order is important):

0: <Module a/aa/aaa.py>
1: <Dir a/ab>

But remember that nodes form a tree, so to get the full picture we need to look at the parents of the nodes:

<Dir a>
  <Dir a/aa>
    <Module a/aa/aaa.py> (0)
  <Dir a/ab> (1)

Note that the <Dir a> parent is the same for both collection args.

genitems

After Session.collect() takes the collection arguments and returns the initial nodes, the function genitems takes each node and recursively expands by calling collect() on each collector node and yielding item nodes (the leaves, i.e. the tests).

So it can look something like this:

genitems(<Module a/aa/aaa.py>)
   <Module a/aa/aaa.py>.collect() -> <Function test_it>, <Class TestCls>
   genitems(<Function test_it>)
     yield <Function test_it>
   genitems(<Class TestCls>)
      <Class TestCls>.collect() -> <Function test_meth1>, <Function test_meth2>
      genitems(<Function test_meth1>)
        yield <Function test_meth1>
      genitems(<Function test_meth2>)
        yield <Function test_meth2>

keep-duplicates flag

Pytest has a --keep-duplicates flag (off by default) documented here but is mostly unspecified.

I think we can ignore whatever it does currently and make it mean what we want.

How should duplicates work?

I think first we need to decide on the semantics. I quickly wrote a test case with some scenarios, it is incomplete but can be a basis for discussion. It includes roughly the behavior that I think it should have but it will definitely change with more consideration (and there are some more interesting cases I should add).

Test cases

def test_duplicate_handling(pytester: Pytester) -> None:
    pytester.makepyfile(
        **{
            "top1/__init__.py": "",
            "top1/test_1.py": (
                """
                def test_1(): pass

                class TestIt:
                    def test_2(): pass

                def test_3(): pass
                """
            ),
            "top1/test_2.py": (
                """
                def test_1(): pass
                """
            ),
            "top2/__init__.py": "",
            "top2/test_1.py": (
                """
                def test_1(): pass
                """
            ),
        },
    )

    result = pytester.runpytest_inprocess("--collect-only", ".")
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "  <Package top2>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess("--collect-only", "top2", "top1")
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top2>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "  <Package top1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess("--collect-only", "top1", "top1/test_2.py")
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess("--collect-only", "top1/test_2.py", "top1")
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess(
        "--collect-only", "--keep-duplicates", "top1/test_2.py", "top1"
    )
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess(
        "--collect-only", "top1/test_2.py", "top1/test_2.py"
    )
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "    <Module test_2.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess("--collect-only", "top2/", "top2/")
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top2>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "  <Package top2>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess(
        "--collect-only", "top2/", "top2/", "top2/test_1.py"
    )
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top2>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "  <Package top2>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess(
        "--collect-only", "top1/test_1.py", "top1/test_1.py::test_3"
    )
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_1.py>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "      <Function test_3>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess(
        "--collect-only", "top1/test_1.py::test_3", "top1/test_1.py"
    )
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_1.py>",
            "      <Function test_3>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "",
        ],
        consecutive=True,
    )

    result = pytester.runpytest_inprocess(
        "--collect-only",
        "--keep-duplicates",
        "top1/test_1.py::test_3",
        "top1/test_1.py",
    )
    result.stdout.fnmatch_lines(
        [
            "<Dir *>",
            "  <Package top1>",
            "    <Module test_1.py>",
            "      <Function test_3>",
            "      <Function test_1>",
            "      <Class TestIt>",
            "        <Function test_2>",
            "      <Function test_3>",
            "",
        ],
        consecutive=True,
    )

Here are some guidelines I based my expected outcomes on (these are debatable):

1. The order of the collection args matters and should be respected as much as possible
2. A collection arg specified explicitly (i.e. not a descendant of another arg) should always be duplicated
3. Otherwise whether or not to duplicate should depend on --keep-duplicates
4. Emitting multiple items with the same nodeid is fine, but emitting the same Item object itself multiple times should never happen
5. Collectors should be shared as much as possible, unless explicitly specified requires duplication

[Alexander-Shukaev]

Hi, any plan to work on this, someone?