Skip to content

Commit

Permalink
Allow for duplicate tests with --allow-duplicates (#940)
Browse files Browse the repository at this point in the history
* Allow for duplicate tests to run

> tldr; It's useful to run 1 or more tests multiple times when debugging
and I'd like to propose an option to do so with parallel_tests

I often find that I'd like to run 1+ files locally, in parallel, to rule
out certain flakey test behaviors (order tests are being run). Kind of
like a form of "test fuzzing" I guess? The purpose is to generally rule
out the test is flakey.

I often find myself doing something like this in zsh:

    repeat 10 bin/rspec path/to/my_spec.rb

Sometimes it's more than 10, and sometimes it's more than 1 file I'd
like to run at the same time.

I'd like to do this with parallel_tests instead and I'd like to propose
this PR as a potential solution (I didn't see this functionality present
with the current options)

Example usage:

```
export TEST_FILES="
spec/test_1.rb
spec/test_1.rb
spec/test_1.rb
spec/test_1.rb
spec/test_1.rb
"

bin/spring parallel_rspec -n 5 --allow-duplicates -- $(echo $TEST_FILES)

bin/spring parallel_rspec -n 5 -- $(echo $TEST_FILES)
```

There's one _minor_ caveat:

These duplicates will still be unique per process because RSpec _also_
calls uniq in a couple places

[here](https://github.com/rspec/rspec-core/blob/1e661db5c5b431c0ee88a383e8e3767f02dccbfe/lib/rspec/core/configuration.rb#L2202), [here](https://github.com/rspec/rspec-core/blob/1e661db5c5b431c0ee88a383e8e3767f02dccbfe/lib/rspec/core/configuration.rb#L2222), and [here](https://github.com/rspec/rspec-core/blob/1e661db5c5b431c0ee88a383e8e3767f02dccbfe/lib/rspec/core/configuration.rb#L1636)

If there is interest in this PR, I'd be happy to propose a similar
PR to the RSpec team.

One other thought: I was also thinking a companion `--repeat <Integer>`
flag could be useful, where it multiplies the file list N times
and then builds the commands.

    bin/spring parallel_rspec --repeat 100 -- path/to/my_spec.rb

Would happy to hear any thoughts or opinions on that as well. I'd
be happy to do a separate PR

* Update lib/parallel_tests/test/runner.rb

Co-authored-by: Michael Grosser <michael@grosser.it>

* Rubocop cleanup

---------

Co-authored-by: Michael Grosser <michael@grosser.it>
  • Loading branch information
jaydorsey and grosser committed Mar 25, 2024
1 parent 799422e commit bc79c20
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
@@ -1,6 +1,7 @@
# Changelog

## Unreleased
- Add `--allow-duplicates` flag to support re-running 1 spec multiple times

### Breaking Changes

Expand Down
1 change: 1 addition & 0 deletions Readme.md
Expand Up @@ -279,6 +279,7 @@ Options are:
--nice execute test commands with low priority.
--runtime-log [PATH] Location of previously recorded test runtimes
--allowed-missing [INT] Allowed percentage of missing runtimes (default = 50)
--allow-duplicates When detecting files to run, allow duplicates. Useful for local debugging
--unknown-runtime [FLOAT] Use given number as unknown runtime (otherwise use average time)
--first-is-1 Use "1" as TEST_ENV_NUMBER to not reuse the default test environment
--fail-fast Stop all groups when one group fails (best used with --test-options '--fail-fast' if supported
Expand Down
1 change: 1 addition & 0 deletions lib/parallel_tests/cli.rb
Expand Up @@ -278,6 +278,7 @@ def parse_options!(argv)
opts.on("--nice", "execute test commands with low priority.") { options[:nice] = true }
opts.on("--runtime-log [PATH]", "Location of previously recorded test runtimes") { |path| options[:runtime_log] = path }
opts.on("--allowed-missing [INT]", Integer, "Allowed percentage of missing runtimes (default = 50)") { |percent| options[:allowed_missing_percent] = percent }
opts.on('--allow-duplicates', 'When detecting files to run, allow duplicates') { options[:allow_duplicates] = true }
opts.on("--unknown-runtime [FLOAT]", Float, "Use given number as unknown runtime (otherwise use average time)") { |time| options[:unknown_runtime] = time }
opts.on("--first-is-1", "Use \"1\" as TEST_ENV_NUMBER to not reuse the default test environment") { options[:first_is_1] = true }
opts.on("--fail-fast", "Stop all groups when one group fails (best used with --test-options '--fail-fast' if supported") { options[:fail_fast] = true }
Expand Down
7 changes: 5 additions & 2 deletions lib/parallel_tests/test/runner.rb
Expand Up @@ -238,8 +238,9 @@ def find_tests(tests, options = {})
suffix_pattern = options[:suffix] || test_suffix
include_pattern = options[:pattern] || //
exclude_pattern = options[:exclude_pattern]
allow_duplicates = options[:allow_duplicates]

(tests || []).flat_map do |file_or_folder|
files = (tests || []).flat_map do |file_or_folder|
if File.directory?(file_or_folder)
files = files_in_folder(file_or_folder, options)
files = files.grep(suffix_pattern).grep(include_pattern)
Expand All @@ -248,7 +249,9 @@ def find_tests(tests, options = {})
else
file_or_folder
end
end.uniq
end

allow_duplicates ? files : files.uniq
end

def files_in_folder(folder, options = {})
Expand Down
4 changes: 4 additions & 0 deletions spec/parallel_tests/cli_spec.rb
Expand Up @@ -78,6 +78,10 @@ def call(*args)
.to eq(defaults.merge(first_is_1: true))
end

it "parses allow-duplicates" do
expect(call(["test", "--allow-duplicates"])).to eq(defaults.merge(allow_duplicates: true))
end

context "parse only-group" do
it "group_by should be set to filesize" do
expect(call(["test", "--only-group", '1'])).to eq(defaults.merge(only_group: [1], group_by: :filesize))
Expand Down
4 changes: 4 additions & 0 deletions spec/parallel_tests/test/runner_spec.rb
Expand Up @@ -387,6 +387,10 @@ def call(*args)
it "discards duplicates" do
expect(call(['baz', 'baz'])).to eq(['baz'])
end

it "keeps duplicates when allow_duplicates" do
expect(call(['baz', 'baz'], allow_duplicates: true)).to eq(['baz', 'baz'])
end
end

describe ".summarize_results" do
Expand Down

0 comments on commit bc79c20

Please sign in to comment.