Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect "collected items" report when specifying tests on the command-line #2468

Merged

Conversation

nicoddemus
Copy link
Member

It turns out pytest was triggering the pytest_collectreport hook when collecting the nodes in order to find the item specified in the command-line, but this is incorrect given that those nodes might not end up in the final list of items that will actually be run.

As a bonus this now only reports the exact number of items that will be executed:

class TestClass(object):
    
    def test_main(self):
        assert 1
        
    def test_bla(self):
        assert True
$ pytest .tmp\test_foo.py::TestClass::test_main
============================= test session starts =============================
platform win32 -- Python 3.6.0, pytest-2.0.1, py-1.4.33, pluggy-0.4.0
rootdir: C:\pytest, inifile: tox.ini
plugins: hypothesis-3.7.0
collected 1 item

.tmp\test_foo.py .

========================== 1 passed in 0.01 seconds ===========================

cc @rev112

Fix #2464

@@ -763,7 +763,6 @@ def _matchnodes(self, matching, names):
if not has_matched and len(rep.result) == 1 and x.name == "()":
nextnames.insert(0, name)
resultnodes.extend(self.matchnodes([x], nextnames))
node.ihook.pytest_collectreport(report=rep)
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 wondering if this could break some expectations plugins had in some way? If so, maybe it should go to features?

Copy link
Member Author

@nicoddemus nicoddemus Jun 3, 2017

Choose a reason for hiding this comment

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

I wondered about that myself... but I suspect everyone expects that an item seen by pytest_collectreport to also appear later on pytest_collection_modifyitems (for example), which was not True previously.

But I doubt this will break anything because this change only enters in effect if the user is passing a test in the command line explicitly (test_foo.py::test_foo).

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.133% when pulling 46d157f on nicoddemus:collection-report-2464 into 5ee9793 on pytest-dev:master.

@nicoddemus
Copy link
Member Author

@The-Compiler @RonnyPfannschmidt gentle ping. 😁

@The-Compiler
Copy link
Member

Looks okay to me, but I don't really understand how the hook call change changes things, so I'd rather have @RonnyPfannschmidt take a look too 😉

@RonnyPfannschmidt RonnyPfannschmidt merged commit 4e57a39 into pytest-dev:master Jun 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants