From d84f1f08d8949e4c443b0f2c1d51b923ffc8fbf1 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 23 Sep 2014 22:09:44 -0300 Subject: [PATCH] fix issue 594: xdist is not executing tests parametrized with random values Now xdist properly reports the collection errors instead of silently failing to execute the test suite. --- testing/acceptance_test.py | 22 ++++++++++++++++++++++ testing/test_dsession.py | 30 +++++++++++++++++++++--------- xdist/dsession.py | 24 +++++++++++++++++------- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index a2c262d..0dcb418 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -485,6 +485,28 @@ def test_fixture_scope_caching_issue503(testdir): ]) +def test_issue_594_random_parametrize(testdir): + """ + Make sure that tests that are randomly parametrized display an appropriate + error message, instead of silently skipping the entire test run. + """ + p1 = testdir.makepyfile(""" + import pytest + import random + + xs = list(range(10)) + random.shuffle(xs) + @pytest.mark.parametrize('x', xs) + def test_foo(x): + assert 1 + """) + result = testdir.runpytest(p1, '-v', '-n4') + assert result.ret == 1 + result.stdout.fnmatch_lines([ + "Different tests were collected between gw* and gw*", + ]) + + class TestNodeFailure: def test_load_single(self, testdir): diff --git a/testing/test_dsession.py b/testing/test_dsession.py index bda5893..4785018 100644 --- a/testing/test_dsession.py +++ b/testing/test_dsession.py @@ -144,24 +144,36 @@ class TestLoadScheduling: crashitem = sched.remove_node(node) assert crashitem == collection[0] - def test_schedule_different_tests_collected(self): + def test_different_tests_collected(self, testdir): """ - Test that LoadScheduling is logging different tests were - collected by slaves when that happens. + Test that LoadScheduling is reporting collection errors when + different test ids are collected by slaves. """ + class CollectHook(object): + """ + Dummy hook that stores collection reports. + """ + + def __init__(self): + self.reports = [] + + def pytest_collectreport(self, report): + self.reports.append(report) + + collect_hook = CollectHook() + config = testdir.parseconfig() + config.pluginmanager.register(collect_hook, "collect_hook") node1 = MockNode() node2 = MockNode() - sched = LoadScheduling(2) - logged_messages = [] - py.log.setconsumer('loadsched', logged_messages.append) + sched = LoadScheduling(2, config=config) sched.addnode(node1) sched.addnode(node2) sched.addnode_collection(node1, ["a.py::test_1"]) sched.addnode_collection(node2, ["a.py::test_2"]) sched.init_distribute() - logged_content = ''.join(x.content() for x in logged_messages) - assert 'Different tests were collected between' in logged_content - assert 'Different tests collected, aborting run' in logged_content + assert len(collect_hook.reports) == 1 + rep = collect_hook.reports[0] + assert 'Different tests were collected between' in rep.longrepr class TestDistReporter: diff --git a/xdist/dsession.py b/xdist/dsession.py index 8bcdacd..1370a8c 100644 --- a/xdist/dsession.py +++ b/xdist/dsession.py @@ -1,4 +1,5 @@ import difflib +from _pytest.runner import CollectReport import pytest import py @@ -88,8 +89,9 @@ class EachScheduling: elif self._removed2pending: for deadnode in self._removed2pending: if deadnode.gateway.spec == node.gateway.spec: - if collection != self.node2collection[deadnode]: - msg = report_collection_diff(self.collection, + dead_collection = self.node2collection[deadnode] + if collection != dead_collection: + msg = report_collection_diff(dead_collection, collection, deadnode.gateway.id, node.gateway.id) @@ -175,9 +177,10 @@ class LoadScheduling: :log: A py.log.Producer instance. + :config: Config object, used for handling hooks. """ - def __init__(self, numnodes, log=None): + def __init__(self, numnodes, log=None, config=None): self.numnodes = numnodes self.node2collection = {} self.node2pending = {} @@ -187,6 +190,7 @@ class LoadScheduling: self.log = py.log.Producer("loadsched") else: self.log = log.loadsched + self.config = config @property def nodes(self): @@ -376,8 +380,9 @@ class LoadScheduling: def _check_nodes_have_same_collection(self): """Return True if all nodes have collected the same items. - If collections differ this returns False and logs the - collection differences as they are found. + If collections differ, this method returns False while logging + the collection differences and posting collection errors to + pytest_collectreport hook. """ node_collection_items = list(self.node2collection.items()) first_node, col = node_collection_items[0] @@ -390,8 +395,12 @@ class LoadScheduling: node.gateway.id, ) if msg: - self.log(msg) same_collection = False + self.log(msg) + if self.config is not None: + rep = CollectReport(node.gateway.id, 'failed', longrepr=msg, + result=[]) + self.config.hook.pytest_collectreport(report=rep) return same_collection @@ -494,7 +503,8 @@ class DSession: numnodes = len(self.nodemanager.specs) dist = self.config.getvalue("dist") if dist == "load": - self.sched = LoadScheduling(numnodes, log=self.log) + self.sched = LoadScheduling(numnodes, log=self.log, + config=self.config) elif dist == "each": self.sched = EachScheduling(numnodes, log=self.log) else: