klrmn / pytest-random

randomize your py.test run
20 stars 10 forks source link

Ensure sort before shuffling #7

Open jdunck opened 9 years ago

jdunck commented 9 years ago

Also bump version to 0.03

klrmn commented 9 years ago

i'm curious what behavior you saw that prompted you to do this, and whether that behavior can be tested for.

jdunck commented 9 years ago

On CircleCI, I have 3 nodes. I am using pytest-circleci to run a subset of the suite on each node, and also using pytest-random to reorder the subset of the suite.

It turns out that --random-seed=1 is not sufficient to guarantee the same ordering on each node. After looking into it a bit, I saw that the items list was not starting in the same order, so that shuffling had different outcomes for a given seed. I think the non-determinism of the original items order is caused by inode ordering or something in recursed directories.

>>> L1 = [1,2,3,4]
>>> L2 = [2,1,3,4]
>>> from random import Random
>>> random = Random()
>>> random.seed(0)
>>> random.shuffle(L1)
>>> random.seed(0)
>>> random.shuffle(L2)
>>> L1 == L2
False

This makes re-running with the same seed unhelpful, and makes reproducing order-related failures difficult. Pre-sorting the items list before shuffling fixes that introduction of non-determinism.

>>> L1.sort()
>>> L2.sort()
>>> random.seed(0)
>>> random.shuffle(L1)
>>> random.seed(0)
>>> random.shuffle(L2)
>>> L1 == L2
>>> True

I had to use the .sort(key=...) on items because in pytest core, Item doesn't have a cmp or lt defined. I have an open PR on core to address that issue. :)

https://bitbucket.org/hpk42/pytest/pull-request/233/add-node__lt__-for-stable-item-sorts/diff

klrmn commented 9 years ago

so in theory there should be a way to modify https://github.com/klrmn/pytest-random/blob/master/tests/test_functionality.py#L8 such that it would cause https://github.com/klrmn/pytest-random/blob/master/tests/test_functionality.py#L132 to fail (without your fix)...but it sounds like you aren't sure what it is. CircleCI seems to be using item.location as some kind of sorting mechanism https://github.com/micktwomey/pytest-circleci/blob/master/pytest_circleci/plugin.py#L26.

i'm trying to decide if this needs a flag of it's own, and whether it conflicts with --random-group. it can't be dependent on the --random-seed flag, because the first run might not have it set, but the user could pull the seed out of the output to send to the second run.

jdunck commented 9 years ago

Thanks for the attention, Leah. -circleci doesn't do any sorting, just filtering. I did that patch to -circleci so that it works regardless of item ordering -- previously it would filter based on item position, but of course that meant that a randomized list on different nodes might all drop the same item.

You're right that using --random-seed (or not) isn't related to the problem. I hadn't considered the interplay here with --random-group as I'm not using it.

Looking at the implementation of --random-group, though, I think it would have a similar sort of instability, because the order of groups.values() isn't guaranteed by the runtime. Calling .values() more than once might return differently-ordered lists (in the same run, or in different runs of the same suite).

If you'd like, I can make a PR so that --random-group is also stable given the same --random-seed?

klrmn commented 9 years ago

it's beginning to sound like i should ask you if you'd be interested in taking over this project. i stopped using py.test over a year and a half ago, and changing gears anytime someone submits a bug / patch to pytest-random or pytest-rerunfailures is challenging =).

jdunck commented 9 years ago

Hmm, If you're happy with me taking over, I'm happy to do it -- but I don't want you to feel like I'm horning in. I definitely understand the switching cost problem - I maintain a couple of libraries that I don't actively use. Test coverage on those is good, but coverage on a plugin with randomization is... not straightforward.

If you do want me to take over, can you add me as a PyPI owner? (username is jdunck there as well).

klrmn commented 9 years ago

no horning in necessary. i'm happy to have help.

i've added you as a collaborator on github...pypi doesn't seem to think jdunck exists. (granted, i also had to reset my pypi password...they had apparently invalidated it at some point.)

jdunck commented 9 years ago

PyPI: weird. I'm definitely jdunck over there. I logged in just now to test it, and it worked. I have had problems doing some administration on it before. Anyway, assuming you're OK pushing versions to PyPI, we can work around their bugs.

klrmn commented 9 years ago

hey cool, pypi worked this time. let me know if the maintainer role doesn't let you do what you want to do.