From 8ca2d01f9a1cacbd14378e2e5f62d81107fa4d92 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 12:23:59 -0400 Subject: [PATCH 01/15] allow multiple isolated processes --- lib/parallel_tests/cli.rb | 14 ++++++++++++++ lib/parallel_tests/grouper.rb | 24 +++++++++++++++++++----- spec/fixtures/rails51/Gemfile.lock | 4 ++-- spec/fixtures/rails52/Gemfile.lock | 4 ++-- spec/parallel_tests/grouper_spec.rb | 4 ++++ spec/parallel_tests/test/runner_spec.rb | 20 ++++++++++++++++++++ 6 files changed, 61 insertions(+), 9 deletions(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index 089cd838..4590de10 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -192,6 +192,12 @@ def parse_options!(argv) options[:isolate] = true end + opts.on("--isolate-n [PROCESSES]", + Integer, + "How many processes to reserve for isolated singles. By default it's one.") do |n| + options[:isolated_count] = n + end + opts.on("--only-group INT[, INT]", Array) { |groups| options[:only_group] = groups.map(&:to_i) } opts.on("-e", "--exec [COMMAND]", "execute this code parallel and with ENV['TEST_ENV_NUMBER']") { |path| options[:execute] = path } @@ -255,6 +261,14 @@ def parse_options!(argv) raise "--group-by #{allowed.join(" or ")} is required for --only-group" end + if options[:isolate] + if options[:isolated_count].to_i > 0 + raise "--isolate-n must be less than n" if options[:isolated_count] >= options[:count] + else + options[:isolated_count] = 1 + end + end + options end diff --git a/lib/parallel_tests/grouper.rb b/lib/parallel_tests/grouper.rb index 0db018a7..7bbbdb66 100644 --- a/lib/parallel_tests/grouper.rb +++ b/lib/parallel_tests/grouper.rb @@ -12,16 +12,30 @@ def by_scenarios(tests, num_groups, options={}) end def in_even_groups_by_size(items, num_groups, options= {}) + all_items = items + groups = Array.new(num_groups) { {:items => [], :size => 0} } # add all files that should run in a single process to one group - (options[:single_process] || []).each do |pattern| - matched, items = items.partition { |item, _size| item =~ pattern } - matched.each { |item, size| add_to_group(groups.first, item, size) } + single_process_patterns = options[:single_process] || [] + + single_items, items = items.partition do |item, _size| + single_process_patterns.any? { |pattern| item =~ pattern } end - groups_to_fill = (options[:isolate] ? groups[1..-1] : groups) - group_features_by_size(items_to_group(items), groups_to_fill) + isolated_count = options[:isolate] ? (options[:isolated_count] || 1) : 0 + + if isolated_count >= 1 + isolated_groups, other_groups = groups[0..(isolated_count - 1)], groups[isolated_count..-1] + + # add all files that should run in a multiple isolated processes to their own groups + group_features_by_size(items_to_group(single_items), isolated_groups) + group_features_by_size(items_to_group(items), other_groups) + else + # add all files that should run in a single non-isolated process to first group + single_items.each { |item, size| add_to_group(groups.first, item, size) } + group_features_by_size(items_to_group(items), groups) + end groups.map! { |g| g[:items].sort } end diff --git a/spec/fixtures/rails51/Gemfile.lock b/spec/fixtures/rails51/Gemfile.lock index 8733a076..c0502edd 100644 --- a/spec/fixtures/rails51/Gemfile.lock +++ b/spec/fixtures/rails51/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (2.32.0) + parallel_tests (3.2.0) parallel GEM @@ -65,7 +65,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.1) + parallel (1.19.2) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) diff --git a/spec/fixtures/rails52/Gemfile.lock b/spec/fixtures/rails52/Gemfile.lock index 1044f004..b43c95da 100644 --- a/spec/fixtures/rails52/Gemfile.lock +++ b/spec/fixtures/rails52/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (2.32.0) + parallel_tests (3.2.0) parallel GEM @@ -72,7 +72,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.1) + parallel (1.19.2) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) diff --git a/spec/parallel_tests/grouper_spec.rb b/spec/parallel_tests/grouper_spec.rb index d8710d95..a022342f 100644 --- a/spec/parallel_tests/grouper_spec.rb +++ b/spec/parallel_tests/grouper_spec.rb @@ -54,6 +54,10 @@ def call(num_groups, options={}) expect(call(2, :single_process => [/1|2|3|4/])).to eq([["1", "2", "3", "4"], ["5"]]) end + it "groups single items into specified isolation groups" do + expect(call(3, :single_process => [/1|2|3|4/], :isolate => true, :isolated_count => 2)).to eq([["1", "4"], ["2", "3"], ["5"]]) + end + it "groups single items with others if there are too few" do expect(call(2, :single_process => [/1/])).to eq([["1", "3", "4"], ["2", "5"]]) end diff --git a/spec/parallel_tests/test/runner_spec.rb b/spec/parallel_tests/test/runner_spec.rb index 90a6fcf6..5e9e5987 100644 --- a/spec/parallel_tests/test/runner_spec.rb +++ b/spec/parallel_tests/test/runner_spec.rb @@ -146,6 +146,26 @@ def call(*args) expect(valid_combinations).to include(actual) end + + it "groups by size and use specified number of isolation groups" do + skip if RUBY_PLATFORM == "java" + expect(ParallelTests::Test::Runner).to receive(:runtimes). + and_return({"aaa1" => 1, "aaa2" => 3, "aaa3" => 2, "bbb" => 3, "ccc" => 1, "ddd" => 2}) + result = call(["aaa1", "aaa2", "aaa3", "bbb", "ccc", "ddd", "eee"], 4, isolate: true, isolated_count: 2, single_process: [/^aaa/], group_by: :runtime) + + isolated_1, isolated_2, *groups = result + expect(isolated_1).to eq(["aaa2"]) + expect(isolated_2).to eq(["aaa1", "aaa3"]) + actual = groups.map(&:to_set).to_set + + # both eee and ccs are the same size, so either can be in either group + valid_combinations = [ + [["bbb", "eee"], ["ccc", "ddd"]].map(&:to_set).to_set, + [["bbb", "ccc"], ["eee", "ddd"]].map(&:to_set).to_set + ] + + expect(valid_combinations).to include(actual) + end end end From d33b04ea9825ed254e5910ddf1e3522935509d16 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 12:27:07 -0400 Subject: [PATCH 02/15] fix wf --- lib/parallel_tests/cli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index 4590de10..228a9f82 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -264,7 +264,7 @@ def parse_options!(argv) if options[:isolate] if options[:isolated_count].to_i > 0 raise "--isolate-n must be less than n" if options[:isolated_count] >= options[:count] - else + else options[:isolated_count] = 1 end end From 77320ccbfb3937ee3f3d9823376537c1abd8b1e3 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 12:27:18 -0400 Subject: [PATCH 03/15] add comments --- lib/parallel_tests/grouper.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/parallel_tests/grouper.rb b/lib/parallel_tests/grouper.rb index 7bbbdb66..aed31033 100644 --- a/lib/parallel_tests/grouper.rb +++ b/lib/parallel_tests/grouper.rb @@ -12,8 +12,6 @@ def by_scenarios(tests, num_groups, options={}) end def in_even_groups_by_size(items, num_groups, options= {}) - all_items = items - groups = Array.new(num_groups) { {:items => [], :size => 0} } # add all files that should run in a single process to one group @@ -26,14 +24,14 @@ def in_even_groups_by_size(items, num_groups, options= {}) isolated_count = options[:isolate] ? (options[:isolated_count] || 1) : 0 if isolated_count >= 1 - isolated_groups, other_groups = groups[0..(isolated_count - 1)], groups[isolated_count..-1] - # add all files that should run in a multiple isolated processes to their own groups - group_features_by_size(items_to_group(single_items), isolated_groups) - group_features_by_size(items_to_group(items), other_groups) + group_features_by_size(items_to_group(single_items), groups[0..(isolated_count - 1)]) + # group the non-isolated by size + group_features_by_size(items_to_group(items), groups[isolated_count..-1]) else # add all files that should run in a single non-isolated process to first group single_items.each { |item, size| add_to_group(groups.first, item, size) } + # group all by size group_features_by_size(items_to_group(items), groups) end From 41f203ba9aba4ff58b9d3766a4a42dd37a319d97 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 12:30:54 -0400 Subject: [PATCH 04/15] undo rails5x Gem changes --- spec/fixtures/rails51/Gemfile.lock | 4 ++-- spec/fixtures/rails52/Gemfile.lock | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/fixtures/rails51/Gemfile.lock b/spec/fixtures/rails51/Gemfile.lock index c0502edd..8733a076 100644 --- a/spec/fixtures/rails51/Gemfile.lock +++ b/spec/fixtures/rails51/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (3.2.0) + parallel_tests (2.32.0) parallel GEM @@ -65,7 +65,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.2) + parallel (1.19.1) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) diff --git a/spec/fixtures/rails52/Gemfile.lock b/spec/fixtures/rails52/Gemfile.lock index b43c95da..1044f004 100644 --- a/spec/fixtures/rails52/Gemfile.lock +++ b/spec/fixtures/rails52/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (3.2.0) + parallel_tests (2.32.0) parallel GEM @@ -72,7 +72,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.2) + parallel (1.19.1) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) From f3e2251217b1e27ab5be987c9ed08b54965446eb Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 12:40:50 -0400 Subject: [PATCH 05/15] add changelog and readme for CLI --- CHANGELOG.md | 4 ++++ Readme.md | 1 + 2 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1253599..72a524e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ - None +## 3.2.1 - 2020-09-13 + +- Added support for multiple isolated processes. + ## 3.2.0 - 2020-08-27 ### Breaking Changes diff --git a/Readme.md b/Readme.md index 29e99ff6..66c717bc 100644 --- a/Readme.md +++ b/Readme.md @@ -206,6 +206,7 @@ Options are: -m, --multiply-processes [FLOAT] use given number as a multiplier of processes to run -s, --single [PATTERN] Run all matching files in the same process -i, --isolate Do not run any other tests in the group used by --single(-s) + --isolate-n How many processes to reserve for isolated tests. Default is 1. --only-group INT[, INT] -e, --exec [COMMAND] execute this code parallel and with ENV['TEST_ENV_NUMBER'] -o, --test-options '[OPTIONS]' execute test commands with those options From 496ee0975c692c97f6278450b4129f179d461b8e Mon Sep 17 00:00:00 2001 From: Vikram B Kumar Date: Sun, 13 Sep 2020 15:55:23 -0400 Subject: [PATCH 06/15] Update lib/parallel_tests/cli.rb Co-authored-by: Michael Grosser --- lib/parallel_tests/cli.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index 228a9f82..fe5792d9 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -193,8 +193,8 @@ def parse_options!(argv) end opts.on("--isolate-n [PROCESSES]", - Integer, - "How many processes to reserve for isolated singles. By default it's one.") do |n| + Integer, + "Number of processes to use for isolated singles, default: 1") do |n| options[:isolated_count] = n end From 477759a2e26010f08b851cb2d952bcaa287478ec Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 17:03:32 -0400 Subject: [PATCH 07/15] address PR comments and add CLI specs --- lib/parallel_tests/cli.rb | 20 ++++++++-------- lib/parallel_tests/grouper.rb | 9 +++---- spec/fixtures/rails51/Gemfile.lock | 4 ++-- spec/fixtures/rails52/Gemfile.lock | 4 ++-- spec/parallel_tests/cli_spec.rb | 32 +++++++++++++++++++++++++ spec/parallel_tests/grouper_spec.rb | 2 +- spec/parallel_tests/test/runner_spec.rb | 2 +- 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index fe5792d9..eed6d5e1 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -190,12 +190,20 @@ def parse_options!(argv) "Do not run any other tests in the group used by --single(-s)") do |pattern| options[:isolate] = true + options[:isolate_count] = 1 end opts.on("--isolate-n [PROCESSES]", Integer, - "Number of processes to use for isolated singles, default: 1") do |n| - options[:isolated_count] = n + "Number of processes to use for isolated singles, default: 1. Do not set this without setting --isolate.") do |n| + + # isolate_count is dependent on isolate being set. + abort "Don't use isolate-n without isolate" unless options[:isolate] + if n >= ParallelTests.determine_number_of_processes(options[:count]) + abort 'Number of isolated processes must be less than total the number of processes' + end + + options[:isolate_count] = n end opts.on("--only-group INT[, INT]", Array) { |groups| options[:only_group] = groups.map(&:to_i) } @@ -261,14 +269,6 @@ def parse_options!(argv) raise "--group-by #{allowed.join(" or ")} is required for --only-group" end - if options[:isolate] - if options[:isolated_count].to_i > 0 - raise "--isolate-n must be less than n" if options[:isolated_count] >= options[:count] - else - options[:isolated_count] = 1 - end - end - options end diff --git a/lib/parallel_tests/grouper.rb b/lib/parallel_tests/grouper.rb index aed31033..a632c61c 100644 --- a/lib/parallel_tests/grouper.rb +++ b/lib/parallel_tests/grouper.rb @@ -21,16 +21,17 @@ def in_even_groups_by_size(items, num_groups, options= {}) single_process_patterns.any? { |pattern| item =~ pattern } end - isolated_count = options[:isolate] ? (options[:isolated_count] || 1) : 0 + if options[:isolate] + isolate_count = options[:isolate_count] || 1 - if isolated_count >= 1 # add all files that should run in a multiple isolated processes to their own groups - group_features_by_size(items_to_group(single_items), groups[0..(isolated_count - 1)]) + group_features_by_size(items_to_group(single_items), groups[0..(isolate_count - 1)]) # group the non-isolated by size - group_features_by_size(items_to_group(items), groups[isolated_count..-1]) + group_features_by_size(items_to_group(items), groups[isolate_count..-1]) else # add all files that should run in a single non-isolated process to first group single_items.each { |item, size| add_to_group(groups.first, item, size) } + # group all by size group_features_by_size(items_to_group(items), groups) end diff --git a/spec/fixtures/rails51/Gemfile.lock b/spec/fixtures/rails51/Gemfile.lock index 8733a076..c0502edd 100644 --- a/spec/fixtures/rails51/Gemfile.lock +++ b/spec/fixtures/rails51/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (2.32.0) + parallel_tests (3.2.0) parallel GEM @@ -65,7 +65,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.1) + parallel (1.19.2) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) diff --git a/spec/fixtures/rails52/Gemfile.lock b/spec/fixtures/rails52/Gemfile.lock index 1044f004..b43c95da 100644 --- a/spec/fixtures/rails52/Gemfile.lock +++ b/spec/fixtures/rails52/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (2.32.0) + parallel_tests (3.2.0) parallel GEM @@ -72,7 +72,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.1) + parallel (1.19.2) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) diff --git a/spec/parallel_tests/cli_spec.rb b/spec/parallel_tests/cli_spec.rb index 2eecd202..1ab9ac73 100644 --- a/spec/parallel_tests/cli_spec.rb +++ b/spec/parallel_tests/cli_spec.rb @@ -107,6 +107,38 @@ def call(*args) end end + context "single and isolate" do + it "single_process should be an array of patterns" do + expect(call(["test", "--single", '1'])).to eq(defaults.merge(single_process: [/1/])) + end + + it "single_process should be an array of patterns" do + expect(call(["test", "--single", '1', "--single", '2'])).to eq(defaults.merge(single_process: [/1/, /2/])) + end + + it "isolate should set isolate_count defaults" do + expect(call(["test", "--single", '1', "--isolate"])).to eq(defaults.merge(single_process: [/1/], isolate: true, isolate_count: 1)) + end + + it "isolate_n should set isolate_count" do + expect(call(["test", "-n", "3", "--single", '1', "--isolate", "--isolate-n", "2"])).to eq( + defaults.merge(count: 3, single_process: [/1/], isolate: true, isolate_count: 2) + ) + end + + it "isolate_n should not be set without isolate" do + expect(subject).to receive(:abort).with("Don't use isolate-n without isolate") + + call(["test", "-n", "3", "--single", '1', "--isolate-n", "2"]) + end + + it "isolate_n must be within limits" do + expect(subject).to receive(:abort).with("Number of isolated processes must be less than total the number of processes") + + call(["test", "-n", "3", "--single", '1', "--isolate", "--isolate-n", "3"]) + end + end + context "when the -- option separator is used" do it "interprets arguments as files/directories" do expect(call(%w(-- test))).to eq( files: %w(test)) diff --git a/spec/parallel_tests/grouper_spec.rb b/spec/parallel_tests/grouper_spec.rb index a022342f..da13e9f5 100644 --- a/spec/parallel_tests/grouper_spec.rb +++ b/spec/parallel_tests/grouper_spec.rb @@ -55,7 +55,7 @@ def call(num_groups, options={}) end it "groups single items into specified isolation groups" do - expect(call(3, :single_process => [/1|2|3|4/], :isolate => true, :isolated_count => 2)).to eq([["1", "4"], ["2", "3"], ["5"]]) + expect(call(3, :single_process => [/1|2|3|4/], :isolate => true, :isolate_count => 2)).to eq([["1", "4"], ["2", "3"], ["5"]]) end it "groups single items with others if there are too few" do diff --git a/spec/parallel_tests/test/runner_spec.rb b/spec/parallel_tests/test/runner_spec.rb index 5e9e5987..b7a0a4b1 100644 --- a/spec/parallel_tests/test/runner_spec.rb +++ b/spec/parallel_tests/test/runner_spec.rb @@ -151,7 +151,7 @@ def call(*args) skip if RUBY_PLATFORM == "java" expect(ParallelTests::Test::Runner).to receive(:runtimes). and_return({"aaa1" => 1, "aaa2" => 3, "aaa3" => 2, "bbb" => 3, "ccc" => 1, "ddd" => 2}) - result = call(["aaa1", "aaa2", "aaa3", "bbb", "ccc", "ddd", "eee"], 4, isolate: true, isolated_count: 2, single_process: [/^aaa/], group_by: :runtime) + result = call(["aaa1", "aaa2", "aaa3", "bbb", "ccc", "ddd", "eee"], 4, isolate: true, isolate_count: 2, single_process: [/^aaa/], group_by: :runtime) isolated_1, isolated_2, *groups = result expect(isolated_1).to eq(["aaa2"]) From a9ff592209be0fcddb115033b13752393ce18818 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 17:05:47 -0400 Subject: [PATCH 08/15] add to Unreleased in Changelog.md --- CHANGELOG.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72a524e0..a67fa413 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Added support for multiple isolated processes. + ### Breaking Changes - None @@ -14,10 +16,6 @@ - None -## 3.2.1 - 2020-09-13 - -- Added support for multiple isolated processes. - ## 3.2.0 - 2020-08-27 ### Breaking Changes From 04d811ce59835fed0c594917b7b5b598183108f7 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 17:16:08 -0400 Subject: [PATCH 09/15] address PR comments - 1. Do not set isolate_count. 2. Simply isolate validations and move them to the end --- lib/parallel_tests/cli.rb | 16 ++++++++-------- spec/parallel_tests/cli_spec.rb | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index eed6d5e1..ec621f16 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -190,19 +190,11 @@ def parse_options!(argv) "Do not run any other tests in the group used by --single(-s)") do |pattern| options[:isolate] = true - options[:isolate_count] = 1 end opts.on("--isolate-n [PROCESSES]", Integer, "Number of processes to use for isolated singles, default: 1. Do not set this without setting --isolate.") do |n| - - # isolate_count is dependent on isolate being set. - abort "Don't use isolate-n without isolate" unless options[:isolate] - if n >= ParallelTests.determine_number_of_processes(options[:count]) - abort 'Number of isolated processes must be less than total the number of processes' - end - options[:isolate_count] = n end @@ -269,6 +261,14 @@ def parse_options!(argv) raise "--group-by #{allowed.join(" or ")} is required for --only-group" end + if options[:isolate] + if options[:isolate_count] && options[:isolate_count] >= ParallelTests.determine_number_of_processes(options[:count]) + abort 'Number of isolated processes must be less than total the number of processes' + end + elsif options[:isolate_count] + abort "Don't use isolate-n without isolate" + end + options end diff --git a/spec/parallel_tests/cli_spec.rb b/spec/parallel_tests/cli_spec.rb index 1ab9ac73..eff0d9a4 100644 --- a/spec/parallel_tests/cli_spec.rb +++ b/spec/parallel_tests/cli_spec.rb @@ -117,7 +117,7 @@ def call(*args) end it "isolate should set isolate_count defaults" do - expect(call(["test", "--single", '1', "--isolate"])).to eq(defaults.merge(single_process: [/1/], isolate: true, isolate_count: 1)) + expect(call(["test", "--single", '1', "--isolate"])).to eq(defaults.merge(single_process: [/1/], isolate: true)) end it "isolate_n should set isolate_count" do From ce8bafe44fad1e36e8aba9260621f20667c572fa Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Sun, 13 Sep 2020 19:53:00 -0400 Subject: [PATCH 10/15] Address 'https://github.com/grosser/parallel_tests/pull/779#discussion_r487578612' --- lib/parallel_tests/cli.rb | 10 +++------- lib/parallel_tests/grouper.rb | 14 ++++++++++++-- spec/parallel_tests/cli_spec.rb | 14 ++++---------- spec/parallel_tests/grouper_spec.rb | 2 +- spec/parallel_tests/test/runner_spec.rb | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index ec621f16..fa82c77f 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -194,7 +194,7 @@ def parse_options!(argv) opts.on("--isolate-n [PROCESSES]", Integer, - "Number of processes to use for isolated singles, default: 1. Do not set this without setting --isolate.") do |n| + "Number of processes to use for isolated singles, default: 1. Setting any positive value turns on 'isolate'.") do |n| options[:isolate_count] = n end @@ -261,12 +261,8 @@ def parse_options!(argv) raise "--group-by #{allowed.join(" or ")} is required for --only-group" end - if options[:isolate] - if options[:isolate_count] && options[:isolate_count] >= ParallelTests.determine_number_of_processes(options[:count]) - abort 'Number of isolated processes must be less than total the number of processes' - end - elsif options[:isolate_count] - abort "Don't use isolate-n without isolate" + if options[:isolate_count] && options[:isolate_count] >= ParallelTests.determine_number_of_processes(options[:count]) + abort 'Number of isolated processes must be less than total the number of processes' end options diff --git a/lib/parallel_tests/grouper.rb b/lib/parallel_tests/grouper.rb index a632c61c..baa60684 100644 --- a/lib/parallel_tests/grouper.rb +++ b/lib/parallel_tests/grouper.rb @@ -21,9 +21,9 @@ def in_even_groups_by_size(items, num_groups, options= {}) single_process_patterns.any? { |pattern| item =~ pattern } end - if options[:isolate] - isolate_count = options[:isolate_count] || 1 + isolate_count = isolate_count(options) + if isolate_count >= 1 # add all files that should run in a multiple isolated processes to their own groups group_features_by_size(items_to_group(single_items), groups[0..(isolate_count - 1)]) # group the non-isolated by size @@ -41,6 +41,16 @@ def in_even_groups_by_size(items, num_groups, options= {}) private + def isolate_count(options) + if options[:isolate_count] && options[:isolate_count] > 1 + options[:isolate_count] + elsif options[:isolate] + 1 + else + 0 + end + end + def largest_first(files) files.sort_by{|_item, size| size }.reverse end diff --git a/spec/parallel_tests/cli_spec.rb b/spec/parallel_tests/cli_spec.rb index eff0d9a4..abc00d39 100644 --- a/spec/parallel_tests/cli_spec.rb +++ b/spec/parallel_tests/cli_spec.rb @@ -120,22 +120,16 @@ def call(*args) expect(call(["test", "--single", '1', "--isolate"])).to eq(defaults.merge(single_process: [/1/], isolate: true)) end - it "isolate_n should set isolate_count" do - expect(call(["test", "-n", "3", "--single", '1', "--isolate", "--isolate-n", "2"])).to eq( - defaults.merge(count: 3, single_process: [/1/], isolate: true, isolate_count: 2) + it "isolate_n should set isolate_count and turn on isolate" do + expect(call(["test", "-n", "3", "--single", '1', "--isolate-n", "2"])).to eq( + defaults.merge(count: 3, single_process: [/1/], isolate_count: 2) ) end - it "isolate_n should not be set without isolate" do - expect(subject).to receive(:abort).with("Don't use isolate-n without isolate") - - call(["test", "-n", "3", "--single", '1', "--isolate-n", "2"]) - end - it "isolate_n must be within limits" do expect(subject).to receive(:abort).with("Number of isolated processes must be less than total the number of processes") - call(["test", "-n", "3", "--single", '1', "--isolate", "--isolate-n", "3"]) + call(["test", "-n", "3", "--single", '1', "--isolate-n", "3"]) end end diff --git a/spec/parallel_tests/grouper_spec.rb b/spec/parallel_tests/grouper_spec.rb index da13e9f5..80b931f5 100644 --- a/spec/parallel_tests/grouper_spec.rb +++ b/spec/parallel_tests/grouper_spec.rb @@ -55,7 +55,7 @@ def call(num_groups, options={}) end it "groups single items into specified isolation groups" do - expect(call(3, :single_process => [/1|2|3|4/], :isolate => true, :isolate_count => 2)).to eq([["1", "4"], ["2", "3"], ["5"]]) + expect(call(3, :single_process => [/1|2|3|4/], :isolate_count => 2)).to eq([["1", "4"], ["2", "3"], ["5"]]) end it "groups single items with others if there are too few" do diff --git a/spec/parallel_tests/test/runner_spec.rb b/spec/parallel_tests/test/runner_spec.rb index b7a0a4b1..34b1778e 100644 --- a/spec/parallel_tests/test/runner_spec.rb +++ b/spec/parallel_tests/test/runner_spec.rb @@ -151,7 +151,7 @@ def call(*args) skip if RUBY_PLATFORM == "java" expect(ParallelTests::Test::Runner).to receive(:runtimes). and_return({"aaa1" => 1, "aaa2" => 3, "aaa3" => 2, "bbb" => 3, "ccc" => 1, "ddd" => 2}) - result = call(["aaa1", "aaa2", "aaa3", "bbb", "ccc", "ddd", "eee"], 4, isolate: true, isolate_count: 2, single_process: [/^aaa/], group_by: :runtime) + result = call(["aaa1", "aaa2", "aaa3", "bbb", "ccc", "ddd", "eee"], 4, isolate_count: 2, single_process: [/^aaa/], group_by: :runtime) isolated_1, isolated_2, *groups = result expect(isolated_1).to eq(["aaa2"]) From 00dd70139498a7f9ef3ef3974dbbf3e6c9d57201 Mon Sep 17 00:00:00 2001 From: Vikram B Kumar Date: Sun, 13 Sep 2020 22:56:48 -0400 Subject: [PATCH 11/15] Update lib/parallel_tests/cli.rb Co-authored-by: Michael Grosser --- lib/parallel_tests/cli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index fa82c77f..c146b830 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -194,7 +194,7 @@ def parse_options!(argv) opts.on("--isolate-n [PROCESSES]", Integer, - "Number of processes to use for isolated singles, default: 1. Setting any positive value turns on 'isolate'.") do |n| + "Use 'isolate' singles with number of processes, default: 1.") do |n| options[:isolate_count] = n end From 0826220208f737742f4191d102708fbc81f5b502 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Mon, 14 Sep 2020 23:07:59 -0400 Subject: [PATCH 12/15] address 'https://github.com/grosser/parallel_tests/pull/779#discussion_r488305819' --- lib/parallel_tests/cli.rb | 4 ---- lib/parallel_tests/grouper.rb | 4 ++++ spec/parallel_tests/cli_spec.rb | 6 ------ spec/parallel_tests/grouper_spec.rb | 7 +++++++ 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/parallel_tests/cli.rb b/lib/parallel_tests/cli.rb index c146b830..c9b0bca8 100644 --- a/lib/parallel_tests/cli.rb +++ b/lib/parallel_tests/cli.rb @@ -261,10 +261,6 @@ def parse_options!(argv) raise "--group-by #{allowed.join(" or ")} is required for --only-group" end - if options[:isolate_count] && options[:isolate_count] >= ParallelTests.determine_number_of_processes(options[:count]) - abort 'Number of isolated processes must be less than total the number of processes' - end - options end diff --git a/lib/parallel_tests/grouper.rb b/lib/parallel_tests/grouper.rb index baa60684..99177469 100644 --- a/lib/parallel_tests/grouper.rb +++ b/lib/parallel_tests/grouper.rb @@ -23,6 +23,10 @@ def in_even_groups_by_size(items, num_groups, options= {}) isolate_count = isolate_count(options) + if isolate_count >= num_groups + abort 'Number of isolated processes must be less than total the number of processes' + end + if isolate_count >= 1 # add all files that should run in a multiple isolated processes to their own groups group_features_by_size(items_to_group(single_items), groups[0..(isolate_count - 1)]) diff --git a/spec/parallel_tests/cli_spec.rb b/spec/parallel_tests/cli_spec.rb index abc00d39..7be6f4c8 100644 --- a/spec/parallel_tests/cli_spec.rb +++ b/spec/parallel_tests/cli_spec.rb @@ -125,12 +125,6 @@ def call(*args) defaults.merge(count: 3, single_process: [/1/], isolate_count: 2) ) end - - it "isolate_n must be within limits" do - expect(subject).to receive(:abort).with("Number of isolated processes must be less than total the number of processes") - - call(["test", "-n", "3", "--single", '1', "--isolate-n", "3"]) - end end context "when the -- option separator is used" do diff --git a/spec/parallel_tests/grouper_spec.rb b/spec/parallel_tests/grouper_spec.rb index 80b931f5..ff34df00 100644 --- a/spec/parallel_tests/grouper_spec.rb +++ b/spec/parallel_tests/grouper_spec.rb @@ -61,6 +61,13 @@ def call(num_groups, options={}) it "groups single items with others if there are too few" do expect(call(2, :single_process => [/1/])).to eq([["1", "3", "4"], ["2", "5"]]) end + + it "groups must abort when isolate_count is out of bounds" do + expect(subject).to receive(:abort).with("Number of isolated processes must be less than total the number of processes") + + call(3, :single_process => [/1/], :isolate_count => 3) + end + end describe '.by_scenarios' do From 645d9a71f6c7f9c8b762cffd202cea0e95c0cc62 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Mon, 14 Sep 2020 23:12:19 -0400 Subject: [PATCH 13/15] revert rails5 Gem updates --- spec/fixtures/rails51/Gemfile.lock | 4 ++-- spec/fixtures/rails52/Gemfile.lock | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/fixtures/rails51/Gemfile.lock b/spec/fixtures/rails51/Gemfile.lock index c0502edd..8733a076 100644 --- a/spec/fixtures/rails51/Gemfile.lock +++ b/spec/fixtures/rails51/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (3.2.0) + parallel_tests (2.32.0) parallel GEM @@ -65,7 +65,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.2) + parallel (1.19.1) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) diff --git a/spec/fixtures/rails52/Gemfile.lock b/spec/fixtures/rails52/Gemfile.lock index b43c95da..1044f004 100644 --- a/spec/fixtures/rails52/Gemfile.lock +++ b/spec/fixtures/rails52/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../../.. specs: - parallel_tests (3.2.0) + parallel_tests (2.32.0) parallel GEM @@ -72,7 +72,7 @@ GEM nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - parallel (1.19.2) + parallel (1.19.1) rack (2.0.5) rack-test (1.1.0) rack (>= 1.0, < 3) From b94d283008b9d139c44a49ce9c8e21092943d5a0 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Wed, 16 Sep 2020 13:55:44 -0400 Subject: [PATCH 14/15] consistent option documentation --- Readme.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Readme.md b/Readme.md index 66c717bc..9f49d054 100644 --- a/Readme.md +++ b/Readme.md @@ -205,8 +205,9 @@ Options are: default - runtime when runtime log is filled otherwise filesize -m, --multiply-processes [FLOAT] use given number as a multiplier of processes to run -s, --single [PATTERN] Run all matching files in the same process - -i, --isolate Do not run any other tests in the group used by --single(-s) - --isolate-n How many processes to reserve for isolated tests. Default is 1. + -i, --isolate Do not run any other tests in the group used by --single(-s). + Automatically turned on if --isolate-n is set above 0. + --isolate-n Number of processes for isolated groups. Default to 1 when --isolate is on. --only-group INT[, INT] -e, --exec [COMMAND] execute this code parallel and with ENV['TEST_ENV_NUMBER'] -o, --test-options '[OPTIONS]' execute test commands with those options From 58b4b71a7f50e4cb7dc35a1754126eff4170bc25 Mon Sep 17 00:00:00 2001 From: Vikram Kumar Date: Wed, 16 Sep 2020 15:08:02 -0400 Subject: [PATCH 15/15] don't abort, but raise from a methodf --- lib/parallel_tests/grouper.rb | 2 +- spec/parallel_tests/grouper_spec.rb | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/parallel_tests/grouper.rb b/lib/parallel_tests/grouper.rb index 99177469..de0d1bab 100644 --- a/lib/parallel_tests/grouper.rb +++ b/lib/parallel_tests/grouper.rb @@ -24,7 +24,7 @@ def in_even_groups_by_size(items, num_groups, options= {}) isolate_count = isolate_count(options) if isolate_count >= num_groups - abort 'Number of isolated processes must be less than total the number of processes' + raise 'Number of isolated processes must be less than total the number of processes' end if isolate_count >= 1 diff --git a/spec/parallel_tests/grouper_spec.rb b/spec/parallel_tests/grouper_spec.rb index ff34df00..ee571a36 100644 --- a/spec/parallel_tests/grouper_spec.rb +++ b/spec/parallel_tests/grouper_spec.rb @@ -63,9 +63,11 @@ def call(num_groups, options={}) end it "groups must abort when isolate_count is out of bounds" do - expect(subject).to receive(:abort).with("Number of isolated processes must be less than total the number of processes") - - call(3, :single_process => [/1/], :isolate_count => 3) + expect { + call(3, :single_process => [/1/], :isolate_count => 3) + }.to raise_error( + "Number of isolated processes must be less than total the number of processes" + ) end end