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

Seeded order is platform-dependent #971

Closed
threedaymonk opened this issue Apr 18, 2016 · 5 comments · Fixed by #974
Closed

Seeded order is platform-dependent #971

threedaymonk opened this issue Apr 18, 2016 · 5 comments · Fixed by #974

Comments

@threedaymonk
Copy link
Contributor

When using --order=random with a seed, the order is determined by Ruby's Array#shuffle method.

Seeded Random produces consistent results across different versions of Ruby and JRuby, but shuffle is implemented differently in different interpreters:

Ruby 2.2.4:

[1,2,3].shuffle(random: Random.new(4))
=> [2, 1, 3]
[1,2,3].shuffle(random: Random.new(5))
=> [1, 2, 3]

Ruby 1.9.3 and JRuby 1.7.23:

[1,2,3].shuffle(random: Random.new(4))
=> [1, 2, 3]
[1,2,3].shuffle(random: Random.new(5))
=> [3, 2, 1]

As I discovered whilst working on #970, this makes it difficult to write specifications that will work across all supported platforms. The current randomize.feature effectively works only by accident because it shuffles only two items.

This also means that it's not always possible to replicate a particular seeded order on a different interpreter.

RSpec has a similar random ordering feature that uses a platform-independent sort on a hash of the item index and the seed. This seems like a more robust thing for cucumber-ruby to do, although it would come at the cost of invalidating any existing use of seeds.

mattwynne referenced this issue in rspec/rspec-core Apr 20, 2016
When you’re troubleshooting an order dependent
failure, you want to get the repro case down to
a minimal run that loads and runs as few specs
as possible. With the old random ordering implementation,
that was hard to achieve because while rerunning with
a given seed produced the same order when the exact
same set of examples were loaded, the ordering would
be completely different when a subset was loaded.

By ordering by `hash(seed + example_id)` it ensures
that the ordering of any two examples should stay
consistently regardless of how many other examples are
loaded.

Jenkins or MD5 is significantly slower than `shuffle`,
but I think the tradeoff is worth it here. This isn’t
a hot spot.
@myronmarston
Copy link

FWIW, I didn't realize that Array#shuffle is platform-dependent. The reason we switched to sorting by hash(item + seed) is to provide stable random ordering. %w[ a b c d ].shuffle may order c before b, but %w[ b c d ].shuffle may order c after b even if you use the same random seed. When you are trying to isolate an ordering dependency, it's essential that the examples are always ordered the same relative to each other, even as the user runs smaller and smaller subsets of the whole suite. Our --bisect feature would be impossible without it.

@mattwynne
Copy link
Member

Thanks for the insight Myron. I covet your bisect feature, so this is worth bearing in mind. Thanks!

@threedaymonk
Copy link
Contributor Author

That's really interesting. Thanks @myronmarston.

@mattwynne I'll add a scenario to cover stability. The scenario path/line is one obvious candidate as a basis for ordering, but it might need a bit of tweaking to ensure that it's not affected by the delimiter (i.e. that it works the same on Windows).

@mattwynne
Copy link
Member

That's great, thanks @threedaymonk

@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants