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

Check RSpec's config.order when determining if a seed is used #63

Closed
wants to merge 1 commit into from

Conversation

aprescott
Copy link

If a test suite has config.order = :random in its RSpec config, then the test ordering is random, but turbo_tests would not print a seed value. In order to get the correct behavior, a redundant --seed value needed to be given to turbo_tests.

The RSpec configuration of config.order = :random should now be correctly respected, so that running turbo_tests without --seed correctly prints the random seed used the same way that rspec does without giving --seed.

Note that this specifically does not try to handle the case where a custom ordering strategy has been given. Only the predefined strategy of :random is checked.

If a test suite has `config.order = :random` in its RSpec config, then
the test ordering is random, but turbo_tests would not print a seed
value. In order to get the correct behavior, a redundant `--seed` value
needed to be given to `turbo_tests`.

The RSpec configuration of `config.order = :random` should now be
correctly respected, so that running `turbo_tests` without `--seed`
correctly prints the random seed used the same way that `rspec` does
without giving `--seed`.

Note that this specifically does not try to handle the case where a
custom ordering strategy has been given. Only the predefined strategy of
`:random` is checked.
@aprescott
Copy link
Author

This should address #60.

@aprescott
Copy link
Author

As a meta note, I noticed some awkwardness in the repo while working on this change. Let me know if you'd like a separate issue for it:

The repo has a .rspec file. This file affects both the repo's own test suite and the exercised code under test. In other words, the 2 are coupled. For example, if you wanted to remove --color from .rspec to stop turbo_tests's own test output from including color, then tests would break because they rely on --color being present for bundle exec turbo_tests some_example_test_file_spec.rb. It might be better to clearly separate the repo's own test suite config from the config used by the spec files exercised by the test suite. (The latter shouldn't be so reliant on the shared global implicit state.)

@aprescott aprescott force-pushed the order-rand-checking branch 2 times, most recently from a3f894f to fff3c1b Compare September 3, 2024 15:04
@aprescott aprescott marked this pull request as draft September 7, 2024 07:04
@aprescott
Copy link
Author

It occurred to me that the code here may not be sufficient. The actual config check logic is correct I belief, but the tests use an explicit --require to turbo_tests which doesn't actually prove an app's real config is in use. Moved to a draft PR for now.

@aprescott
Copy link
Author

Can't get this working in a way that avoids problems with RSpec's global RSpec.configuration state, even in a happy path default case. (It ends up interfering with turbo_tests's own test config.) If there's a way to refer to the same RSpec config that RSpec itself would end up using, without modifying global state, then the diff here might be useful.

@aprescott aprescott closed this Sep 7, 2024
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.

1 participant