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 sometime failing test #780

Merged
merged 2 commits into from
Mar 22, 2018
Merged

Fix sometime failing test #780

merged 2 commits into from
Mar 22, 2018

Conversation

russelldb
Copy link
Member

The test assumed that the shuffled order of the list would
be X because it assumed no other calls to random took place.
Maybe some recent changes added a call to random, but the
test fails on my ubuntu machine when make eunit is run.

This change calls seed in the test to ensure that "determinsitic random"
behaviour occurs for the test.

The test assumed that the shuffled order of the list would
be X because it assumed no other calls to random took place.
Maybe some recent changes added a call to `random`, but the
test fails on my ubuntu machine when `make eunit` is run.

This change calls `seed` in the test to ensure that "determinsitic random"
behaviour occurs for the test.
Copy link

@gwelr gwelr left a comment

Choose a reason for hiding this comment

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

As discussed, if you method is list_shuffle, the only valid test would be to prove it is correctly shuffle each time. This test try to prove the opposite. I would remove this test. If you absolutly need a test, run twice the list_shuffle call with the same In and make sure that Out1 and Out2 are different.

@russelldb
Copy link
Member Author

Yeah, I think since running it twice is only probably going to pass. It's a bad test. To copy Goldblum in JP: I was so preoccupied with whether or not I could fix it, I didn’t stop to think if I should.

@russelldb russelldb merged commit da26900 into develop-2.2 Mar 22, 2018
@russelldb russelldb deleted the rdb/random-shuffle-fix branch March 22, 2018 14:43
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.

2 participants