-
Notifications
You must be signed in to change notification settings - Fork 27
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
Support parallel_test options: pattern, exclude-pattern & group-by #41
base: master
Are you sure you want to change the base?
Conversation
9492d22
to
91c7e63
Compare
There could be one issue around |
I would also suggest another PR to handle properly Fuubar #23 depending on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution, @inkstak!
Let me think if we want turbo_tests
to depend more on parallel_tests
.
Hey, @ilyazub! Do you have a decision about the level of dependency on NOTE: as I know, these options belong to |
+1, this doesn't increase the dependency on Any chance you could merge, @ilyazub? |
FYI: For example, we use a CI script to launch our specs : if parallel
command = "bundle exec parallel_rspec"
command = "bundle exec turbo_tests" if parallel == "turbo_tests"
command += " --exclude-pattern spec/system" if arg == "unit"
command += " --pattern spec/system" if arg == "system"
else
command = "bundle exec rspec"
command += " --exclude-pattern 'system/**/*_spec.rb'" if arg == "unit"
command += " --pattern 'system/**/*_spec.rb'" if arg == "system"
end In this PR, I forward those arguments to parallel_tests so it uses its implementation and not the one from RSpec. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inkstak @kalashnikovisme @steobrien Thanks for your work and sorry for not reviewing this PR sooner.
Let's record runtime log only when --group-by runtime
is passed.
Because we want to support most of the ParallelTests options, may we call ParallelTests::CLI.send(:parse_options!, @argv)
in TurboTests::CLI#run
?
turbo_tests/lib/turbo_tests/cli.rb
Line 20 in b4b8856
parallel_options = {} |
b4b8856
to
ddedb01
Compare
PR updated ! You can pass any options supported by parallel_tests after bundle exec turbo_tests -n 4 -- --only-group 1 --pattern spec/system Because OptionParser raises an exception on unknown options, I found it easier to split options like this.
|
) | ||
|
||
setup_tmp_dir | ||
|
||
subprocess_opts = { | ||
record_runtime: use_runtime_info | ||
record_runtime: @parallel_options[:group_by] == :runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's record runtime log only when
--group-by runtime
is passed.
Not sure if this line is still relevant...
ddedb01
to
7860d71
Compare
This PR add suport for some parallel_test options: