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

Use seed to randomize require order of test files #12442

Merged
merged 2 commits into from
Mar 3, 2023

Conversation

halfdan
Copy link
Contributor

@halfdan halfdan commented Mar 2, 2023

Addresses https://groups.google.com/g/elixir-lang-core/c/yjs-zaFvZJc

Looking for feedback on implementation and testing strategy.

  • Update docs for seed argument

Comment on lines +398 to +399
options = persist_defaults(configuration())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to be outside of Task.async to ensure seed was set when it's fetched in test.ex

assert matches == [["ATest [test/a_test_stale.exs]"], ["BTest [test/b_test_stale.exs]"]]
end)
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Those tests are rather expensive and they rely on implementation details-ish. I am thinking we could just skip them altogether? Other than that, the impl is great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works for me! Let me remove the test.

@halfdan
Copy link
Contributor Author

halfdan commented Mar 3, 2023

@josevalim I considered updating the docs for the --seed argument, but now I'm thinking that they are probably fine as is:

    * `--seed` - seeds the random number generator used to randomize the order of tests;
      `--seed 0` disables randomization so the tests in a single file will always be ran
      in the same order they were defined in

Even with --seed 0 there's no guarantee the modules are loaded in the same order given the parallelisation in how the modules are loaded.

@josevalim josevalim merged commit 31b3a20 into elixir-lang:main Mar 3, 2023
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@halfdan halfdan deleted the fb/shuffle-require branch March 3, 2023 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants