--deselect does not work with tests inside classes

[omnilinguist]

FYI @uSpike @nicoddemus @RonnyPfannschmidt @blueyed @Thisch

In #3201, the test case only seems to test --deselections of the form path/to/test_file.py::test_name. I validated that this works, but --deselections of the form path/to/test_file.py::TestClass::test_name do not work. How challenging would this be to fix?

[blueyed]

A failing test would be a good start.
Looks like it is not mached in https://github.com/pytest-dev/pytest/pull/3201/files#diff-5b415004cf343fc3c36aebb8aad93882R221 then.

In this regard I've also noticed that sometimes (with failures?) you get path/file.py::TestClass::()::test_name (note the extra ()) - but have never investigated. It can be used with the extra ::() then for running it, so maybe that is the case here already, i.e. you need the extra ::() to deselect it.

btw: it would be nice if globs could be used here (i.e. not just startswith)

[omnilinguist]

oh wow...that --deselection (path/file.py::TestClass::()::test_name) actually worked! 😮

I think that is an acceptable workaround for now, but depends on some fairly arcane tribal knowledge / black magic about the extra ::(). I can backlog an item to investigate this, but I suspect someone who is much more familiar with pytest internals would be much faster at it.

[blueyed]

Cool.
I think / assumed the "::()" is not really intentional, or at least it looks like it to me.

[nicoddemus]

If we search for "::()" in the code base, we see at least three places that do a "parens normalization":

pytest/src/_pytest/terminal.py

Line 678 in d46b6b2

res = mkrel(nodeid).replace("::()", "") # parens-normalization

I believe we should extract that to a function, change the duplicated places to use that function, including where @blueyed pointed out previously:

pytest/src/_pytest/main.py

Lines 295 to 296 in d46b6b2

	if colitem.nodeid.startswith(deselect_prefixes):
	deselected.append(colitem)

It is kind of hackish, but it is simple to do and should improve the current state of things.

[RonnyPfannschmidt]

@nicoddemus this is one of the really cringe-worthy structural problems of the collection tree

[nicoddemus]

@RonnyPfannschmidt yep, I'm aware 😬

[blueyed]

It is coming via

pytest/src/_pytest/terminal.py

Line 690 in eb0c733

res = mkrel(nodeid).replace("::()", "") # parens-normalization

.

And gets also removed/skipped here:
https://github.com/blueyed/pytest/blob/eb0c733a8494e0b4c12f407a49ac1c0d1be01d17/src/_pytest/python.py#L271-L272

If replacing/normalizing it, couldn't that be done in Node then, or a more central place?

But I wonder why the Class => Instance is necessary after all?

At least the following gets not collected anyway:

class TestFoo():
    @classmethod
    def test_foo(cls):
        pass

I've given it a quick shot at #4357.