Skip to content

xdist is not executing tests if their parametrizations were collected in a different order #596

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

Closed
pytestbot opened this issue Sep 23, 2014 · 7 comments
Labels
plugin: xdist related to the xdist external plugin type: bug problem that needs to be addressed

Comments

@pytestbot
Copy link
Contributor

Originally reported by: BitBucket: liori, GitHub: liori


Related to #594, but slightly different.

I generated my parametrizations by iterating over a set of strings. However, this iteration depends on the specific values the strings hash to, and these values might be different for every process. Therefore, whereas I was always generating exactly the same parametrizations, they were in different order.

A simple test case that shows the problem:

#!python

import pytest

my_names = {'john', 'kate', 'alfred', 'paul', 'mary'}

@pytest.mark.parametrize('name', list(my_names), ids=list(my_names))
def test_is_name_short(name):
    assert len(name) < 7

Run with PYTHONHASHSEED=random py.test -n 4 to make sure you trigger randomized hashing for strings.

I think that a simple sort in report_collection_diff before running unified_diff might help with this problem.


@pytestbot
Copy link
Contributor Author

Original comment by Bruno Oliveira (BitBucket: nicoddemus, GitHub: nicoddemus):


I don't think that will solve it, because the master node in xdist uses just the indices to distribute the tests among the slaves and if each slave has its own list ordered differently it may end up not running some tests while running others twice.

I'm not sure how to approach this. Unfortunately there's no way for parametrization to realize that its parameters may be random.

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


I think we just need to fail with clear error message if the collections are non-deterministic. The only way how xdist could work there is, if we ran one node and then forked it N times but even that would only work on Unix and only for "-nN", not for cross-host/interpreter distribution etc.

@pytestbot
Copy link
Contributor Author

Original comment by Bruno Oliveira (BitBucket: nicoddemus, GitHub: nicoddemus):


Perhaps we should put a warning in xdist's documentation regarding random collections? Perhaps even pointing to the code you posted in #594 as an possible solution.

@pytestbot
Copy link
Contributor Author

Original comment by BitBucket: liori, GitHub: liori:


Bruno Oliveira, I just wanted to point out that the #594 is not mine ;-) It's just a coincidence that pytry had a similar problem two days ago. Also, it won't work here—you can't move this nondeterminism into the test function itself in this case.

Also, I'd like to underline that the parametrization values here are deterministic (not random like in #594), just the order of them is not. That's why enforcing some order (by sorting the test identifiers) will be sufficient in this case to have exactly the same order in all slaves.

If you say that xdist distributes only indices, then indeed sorting in report_collection_diff will not be enough. However, sorting somewhere before that point should still be OK—somewhere where the order of tests after sorting will be preserved till the test distribution phase. I don't know the xdist codebase, but I'd guess that sorting the test by their identifiers on each slave should enforce the same order everywhere. Then using indices will be perfectly safe.

@pytestbot
Copy link
Contributor Author

Original comment by Bruno Oliveira (BitBucket: nicoddemus, GitHub: nicoddemus):


You're right, if files were sorted in each slave, everyone would end up with the exact test ids and ordering. Not sure how that fits with overall py.test's design thought, as this would change the test execution order significantly.

Personally, I think requiring tests to be collected always in the same order is fine, only that right now the end user doesn't have a clear message stating the problem; if we could solve that, then it is usually a simple matter to just fix the offending tests' parametrization.

@pytestbot
Copy link
Contributor Author

Original comment by BitBucket: liori, GitHub: liori:


I would really love to see the solution for this problem inside xdist instead of my tests—for obvious reasons. But I'll understand if you choose not to implement it there; after all, the multiprocessing code is already quite complex. Anyway, big thanks for the module, it saves me quite a lot of time now.

@pytestbot pytestbot added type: bug problem that needs to be addressed plugin: xdist related to the xdist external plugin labels Jun 15, 2015
refi64 added a commit to refi64/pytest-xdist that referenced this issue Jun 18, 2016
refi64 added a commit to refi64/pytest-xdist that referenced this issue Jun 18, 2016
@RonnyPfannschmidt
Copy link
Member

closing this one as "works as intended" - currently pytest cant support anything else due to allowing duplicate test id's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: xdist related to the xdist external plugin type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

2 participants