Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/5516.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Cache node splitting function which can improve collection performance in very large test suites.
7 changes: 5 additions & 2 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import warnings
from functools import lru_cache

import py

Expand All @@ -13,6 +14,7 @@
tracebackcutdir = py.path.local(_pytest.__file__).dirpath()


@lru_cache(maxsize=None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a cache of mutable return values is asking for trouble -- if this returned tuples I'd be more comfortable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it would be safer. @Nepherhotep would you mind changing that so we can see how it fares?

Copy link
Contributor Author

@Nepherhotep Nepherhotep Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicoddemus, a simple converting response format from list to tuple caused 1500 errors. Most of them failed with the error "fixture testdir not found", some of them failed with repr error.
I need to dig into "testdir" fixture issue a bit deeper.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised... strange.

Copy link
Member

@nicoddemus nicoddemus Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you missed the early return:

    if nodeid == "":
        # If there is no root node at all, return an empty list so the caller's logic can remain sane
        return []

So sometimes it would return a tuple, others a list.

I've reproduced it locally, returning a tuple at both places worked fine here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad, thanks for pointing to that!

def _splitnode(nodeid):
"""Split a nodeid into constituent 'parts'.

Expand All @@ -30,11 +32,12 @@ def _splitnode(nodeid):
"""
if nodeid == "":
# If there is no root node at all, return an empty list so the caller's logic can remain sane
return []
return ()
parts = nodeid.split(SEP)
# Replace single last element 'test_foo.py::Bar' with multiple elements 'test_foo.py', 'Bar'
parts[-1:] = parts[-1].split("::")
return parts
# Convert parts into a tuple to avoid possible errors with caching of a mutable type
return tuple(parts)


def ischildnode(baseid, nodeid):
Expand Down