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

Add Collections#shuffle(List) to forbidden APIs #15287

Closed
jasontedor opened this issue Dec 7, 2015 · 6 comments
Closed

Add Collections#shuffle(List) to forbidden APIs #15287

jasontedor opened this issue Dec 7, 2015 · 6 comments
Assignees
Labels
:Core/Infra/Core Core issues without another label >enhancement

Comments

@jasontedor
Copy link
Member

Several tests are using Collections#shuffle(List) which uses a default source of randomness rather than using Collections#shuffle(List, Random) which enables a reproducible source of randomness. This prevents reproducible build failures. One example is NodeJoinControllerTests but there are many others.

The method Collections#shuffle(List) should be added to forbidden APIs.

@jasontedor jasontedor added >enhancement :Core/Infra/Core Core issues without another label labels Dec 7, 2015
@jasontedor jasontedor self-assigned this Dec 7, 2015
@javanna
Copy link
Member

javanna commented Dec 7, 2015

+1

@nik9000
Copy link
Member

nik9000 commented Dec 7, 2015

The method Collections#shuffle(List) should be added to forbidden APIs.

The list for tests or the list for all classes?

@jasontedor
Copy link
Member Author

The list for tests or the list for all classes?

I intend to completely forbid it lest there still remain non-reproducible sources of randomness. I discussed this with @rmuir and we have a path forward.

@rmuir
Copy link
Contributor

rmuir commented Dec 7, 2015

its the only way. if something is non-test and uses random behavior, give it an optional setting like initialSeed and set it in the test framework.

@nik9000
Copy link
Member

nik9000 commented Dec 7, 2015

its the only way. if something is non-test and uses random behavior, give it an optional setting like initialSeed and set it in the test framework.

I figured as much but its nice to have it confirmed. IIRC its used in a few places.

@jasontedor
Copy link
Member Author

This method is also used inside ESTestCase#randomSubsetOf greatly increasing the surface area of non-reproducible tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >enhancement
Projects
None yet
Development

No branches or pull requests

4 participants