From ac21f1c909e2876bc7a7ce23768b8cc3a1f78520 Mon Sep 17 00:00:00 2001 From: Alexander Jeurissen Date: Sun, 21 Mar 2021 12:45:34 +0100 Subject: [PATCH 1/5] Add selective test running support --- lib/knapsack/allocator_builder.rb | 5 +++++ lib/knapsack/distributors/base_distributor.rb | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/knapsack/allocator_builder.rb b/lib/knapsack/allocator_builder.rb index 94901a3..e28fe28 100644 --- a/lib/knapsack/allocator_builder.rb +++ b/lib/knapsack/allocator_builder.rb @@ -9,6 +9,7 @@ def allocator Knapsack::Allocator.new({ report: Knapsack.report.open, test_file_pattern: test_file_pattern, + test_file_list_source_file: test_file_list_source_file, ci_node_total: Knapsack::Config::Env.ci_node_total, ci_node_index: Knapsack::Config::Env.ci_node_index }) @@ -33,5 +34,9 @@ def report_path def test_file_pattern Knapsack::Config::Env.test_file_pattern || @adapter_class::TEST_DIR_PATTERN end + + def test_file_list_source_file + Knapsack::Config::test_file_list_source_file + end end end diff --git a/lib/knapsack/distributors/base_distributor.rb b/lib/knapsack/distributors/base_distributor.rb index d1562ba..9440245 100644 --- a/lib/knapsack/distributors/base_distributor.rb +++ b/lib/knapsack/distributors/base_distributor.rb @@ -1,11 +1,12 @@ module Knapsack module Distributors class BaseDistributor - attr_reader :report, :node_tests, :test_file_pattern + attr_reader :report, :node_tests, :test_file_pattern, :test_file_list_source_file def initialize(args={}) @report = args[:report] || raise('Missing report') @test_file_pattern = args[:test_file_pattern] || raise('Missing test_file_pattern') + @test_file_list_source_file = args[:test_file_list_source_file] @ci_node_total = args[:ci_node_total] || raise('Missing ci_node_total') @ci_node_index = args[:ci_node_index] || raise('Missing ci_node_index') @@ -37,7 +38,19 @@ def assign_test_files_to_node end def all_tests - @all_tests ||= Dir.glob(test_file_pattern).uniq.sort + @all_tests ||= test_files + end + + # NOTE: Use the test_file_pattern by default + # support specifying a list_file to use similar to KnapsackPro + # ref: KnapsackPro/knapsack_pro-ruby/commit/7d7b8db8be524f2f30d7d80d3a6444dad9f85b1b + def test_files + return Dir.glob(test_file_pattern).uniq.sort if test_file_list_source_file.nil? + + File.read(test_file_list_source_file) + .split(/\n/) + .uniq + .sort end protected From ab5865a773e7f11fc2f8368947700e44dd3a0ed0 Mon Sep 17 00:00:00 2001 From: Alexander Jeurissen Date: Sun, 21 Mar 2021 13:10:20 +0100 Subject: [PATCH 2/5] Add specs --- lib/knapsack/allocator_builder.rb | 2 +- lib/knapsack/config/env.rb | 4 ++++ spec/fixtures/test_file_list_source_file.txt | 5 +++++ spec/knapsack/allocator_builder_spec.rb | 21 +++++++++++++++++++ spec/knapsack/config/env_spec.rb | 14 +++++++++++++ .../distributors/leftover_distributor_spec.rb | 17 +++++++++++++++ 6 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/test_file_list_source_file.txt diff --git a/lib/knapsack/allocator_builder.rb b/lib/knapsack/allocator_builder.rb index e28fe28..e7238a8 100644 --- a/lib/knapsack/allocator_builder.rb +++ b/lib/knapsack/allocator_builder.rb @@ -36,7 +36,7 @@ def test_file_pattern end def test_file_list_source_file - Knapsack::Config::test_file_list_source_file + Knapsack::Config::Env.test_file_list_source_file end end end diff --git a/lib/knapsack/config/env.rb b/lib/knapsack/config/env.rb index 1e3fd7b..67da29a 100644 --- a/lib/knapsack/config/env.rb +++ b/lib/knapsack/config/env.rb @@ -18,6 +18,10 @@ def test_file_pattern ENV['KNAPSACK_TEST_FILE_PATTERN'] end + def test_file_list_source_file + ENV['KNAPSACK_TEST_FILE_LIST_SOURCE_FILE'] + end + def test_dir ENV['KNAPSACK_TEST_DIR'] end diff --git a/spec/fixtures/test_file_list_source_file.txt b/spec/fixtures/test_file_list_source_file.txt new file mode 100644 index 0000000..e827c5a --- /dev/null +++ b/spec/fixtures/test_file_list_source_file.txt @@ -0,0 +1,5 @@ +./spec/test1_spec.rb +spec/test2_spec.rb[1] +./spec/test3_spec.rb[1:2:3:4] +./spec/test4_spec.rb:4 +./spec/test4_spec.rb:5 diff --git a/spec/knapsack/allocator_builder_spec.rb b/spec/knapsack/allocator_builder_spec.rb index 200505e..5c55a6f 100644 --- a/spec/knapsack/allocator_builder_spec.rb +++ b/spec/knapsack/allocator_builder_spec.rb @@ -12,6 +12,7 @@ let(:env_ci_node_index) { double } let(:env_report_path) { nil } let(:env_test_file_pattern) { nil } + let(:env_test_file_list_source_file) { nil } describe '#allocator' do subject { allocator_builder.allocator } @@ -19,6 +20,7 @@ before do expect(Knapsack::Config::Env).to receive(:report_path).and_return(env_report_path) expect(Knapsack::Config::Env).to receive(:test_file_pattern).and_return(env_test_file_pattern) + expect(Knapsack::Config::Env).to receive(:test_file_list_source_file).and_return(env_test_file_list_source_file) expect(Knapsack::Config::Env).to receive(:ci_node_total).and_return(env_ci_node_total) expect(Knapsack::Config::Env).to receive(:ci_node_index).and_return(env_ci_node_index) @@ -36,6 +38,7 @@ { report: report, test_file_pattern: adapter_test_file_pattern, + test_file_list_source_file: env_test_file_list_source_file, ci_node_total: env_ci_node_total, ci_node_index: env_ci_node_index } @@ -51,6 +54,7 @@ { report: report, test_file_pattern: adapter_test_file_pattern, + test_file_list_source_file: env_test_file_list_source_file, ci_node_total: env_ci_node_total, ci_node_index: env_ci_node_index } @@ -66,6 +70,23 @@ { report: report, test_file_pattern: env_test_file_pattern, + test_file_list_source_file: env_test_file_list_source_file, + ci_node_total: env_ci_node_total, + ci_node_index: env_ci_node_index + } + end + + it { should eql allocator } + end + + context 'when ENV test_file_list_source_file has value' do + let(:env_test_file_list_source_file) { 'knapsack_custom_file_list.txt' } + let(:report_config) { { report_path: adapter_report_path } } + let(:allocator_args) do + { + report: report, + test_file_pattern: adapter_test_file_pattern, + test_file_list_source_file: env_test_file_list_source_file, ci_node_total: env_ci_node_total, ci_node_index: env_ci_node_index } diff --git a/spec/knapsack/config/env_spec.rb b/spec/knapsack/config/env_spec.rb index 669f57b..118fee3 100644 --- a/spec/knapsack/config/env_spec.rb +++ b/spec/knapsack/config/env_spec.rb @@ -122,6 +122,20 @@ end end + describe '.test_file_list_source_file' do + subject { described_class.test_file_list_source_file } + + context 'when ENV exists' do + let(:test_file_list_source_file) { 'spec/fixtures/test_file_list_source_file.txt' } + before { stub_const("ENV", { 'KNAPSACK_TEST_FILE_LIST_SOURCE_FILE' => test_file_list_source_file }) } + it { should eql test_file_list_source_file } + end + + context "when ENV doesn't exist" do + it { should be_nil } + end + end + describe '.test_dir' do subject { described_class.test_dir } diff --git a/spec/knapsack/distributors/leftover_distributor_spec.rb b/spec/knapsack/distributors/leftover_distributor_spec.rb index 2c74e4c..51f36a2 100644 --- a/spec/knapsack/distributors/leftover_distributor_spec.rb +++ b/spec/knapsack/distributors/leftover_distributor_spec.rb @@ -8,10 +8,12 @@ } end let(:test_file_pattern) { 'spec/**{,/*/**}/*_spec.rb' } + let(:test_file_list_source_file) { nil } let(:default_args) do { report: report, test_file_pattern: test_file_pattern, + test_file_list_source_file: test_file_list_source_file, ci_node_total: '1', ci_node_index: '0' } @@ -52,6 +54,21 @@ end end + context 'when given test_file_list_source_file' do + context 'spec/fixtures/test_file_list_source_file.txt' do + let(:test_file_list_source_file) { 'spec/fixtures/test_file_list_source_file.txt' } + it { should_not be_empty } + it { should include './spec/test3_spec.rb[1:2:3:4]' } + it { should include './spec/test4_spec.rb:5' } + it { should_not include 'spec/knapsack/tracker_spec.rb' } + it { should_not include 'spec/knapsack/adapters/rspec_adapter_spec.rb' } + + it 'has no duplicated test file paths' do + expect(subject.size).to eq subject.uniq.size + end + end + end + context 'when fake test_file_pattern' do let(:test_file_pattern) { 'fake_pattern' } it { should be_empty } From e46af314d3c0339f013b3145aede856584247470 Mon Sep 17 00:00:00 2001 From: Alexander Jeurissen Date: Sun, 21 Mar 2021 21:55:38 +0100 Subject: [PATCH 3/5] Fix bug where empty node_tests executes complete test_suite --- lib/knapsack/runners/cucumber_runner.rb | 4 ++++ lib/knapsack/runners/minitest_runner.rb | 4 ++++ lib/knapsack/runners/rspec_runner.rb | 4 ++++ lib/knapsack/runners/spinach_runner.rb | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/lib/knapsack/runners/cucumber_runner.rb b/lib/knapsack/runners/cucumber_runner.rb index 4b87b59..3fc06e1 100644 --- a/lib/knapsack/runners/cucumber_runner.rb +++ b/lib/knapsack/runners/cucumber_runner.rb @@ -12,6 +12,10 @@ def self.run(args) Knapsack.logger.info allocator.leftover_node_tests Knapsack.logger.info + # NOTE: return if there are no specs to execute for this node. + # This can occurr if test_file_list_source_file is used with less then CI_NODES specs + return Knapsack.logger.warn('No specs to execute') if allocator.stringify_node_tests.empty? + cmd = %Q[bundle exec cucumber #{args} --require #{allocator.test_dir} -- #{allocator.stringify_node_tests}] system(cmd) diff --git a/lib/knapsack/runners/minitest_runner.rb b/lib/knapsack/runners/minitest_runner.rb index bc30654..b614099 100644 --- a/lib/knapsack/runners/minitest_runner.rb +++ b/lib/knapsack/runners/minitest_runner.rb @@ -12,6 +12,10 @@ def self.run(args) Knapsack.logger.info allocator.leftover_node_tests Knapsack.logger.info + # NOTE: return if there are no specs to execute for this node. + # This can occurr if test_file_list_source_file is used with less then CI_NODES specs + return Knapsack.logger.warn('No specs to execute') if allocator.stringify_node_tests.empty? + task_name = 'knapsack:minitest_run' if Rake::Task.task_defined?(task_name) diff --git a/lib/knapsack/runners/rspec_runner.rb b/lib/knapsack/runners/rspec_runner.rb index 19f6fb6..bff1635 100644 --- a/lib/knapsack/runners/rspec_runner.rb +++ b/lib/knapsack/runners/rspec_runner.rb @@ -12,6 +12,10 @@ def self.run(args) Knapsack.logger.info allocator.leftover_node_tests Knapsack.logger.info + # NOTE: return if there are no specs to execute for this node. + # This can occurr if test_file_list_source_file is used with less then CI_NODES specs + return Knapsack.logger.warn('No specs to execute') if allocator.stringify_node_tests.empty? + cmd = %Q[bundle exec rspec #{args} --default-path #{allocator.test_dir} -- #{allocator.stringify_node_tests}] exec(cmd) diff --git a/lib/knapsack/runners/spinach_runner.rb b/lib/knapsack/runners/spinach_runner.rb index ceed286..bb66218 100644 --- a/lib/knapsack/runners/spinach_runner.rb +++ b/lib/knapsack/runners/spinach_runner.rb @@ -12,6 +12,10 @@ def self.run(args) Knapsack.logger.info allocator.leftover_node_tests Knapsack.logger.info + # NOTE: return if there are no specs to execute for this node. + # This can occurr if test_file_list_source_file is used with less then CI_NODES specs + return Knapsack.logger.warn('No specs to execute') if allocator.stringify_node_tests.empty? + cmd = %Q[bundle exec spinach #{args} --features_path #{allocator.test_dir} -- #{allocator.stringify_node_tests}] system(cmd) From 82f1c63bd0cf34dce686c810ea6f3d00cdc2fb1e Mon Sep 17 00:00:00 2001 From: Alexander Jeurissen Date: Fri, 26 Mar 2021 19:19:38 +0100 Subject: [PATCH 4/5] Also use default behavior if test_file_list_source_file is empty --- lib/knapsack/distributors/base_distributor.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/knapsack/distributors/base_distributor.rb b/lib/knapsack/distributors/base_distributor.rb index 9440245..5ec3563 100644 --- a/lib/knapsack/distributors/base_distributor.rb +++ b/lib/knapsack/distributors/base_distributor.rb @@ -45,12 +45,17 @@ def all_tests # support specifying a list_file to use similar to KnapsackPro # ref: KnapsackPro/knapsack_pro-ruby/commit/7d7b8db8be524f2f30d7d80d3a6444dad9f85b1b def test_files - return Dir.glob(test_file_pattern).uniq.sort if test_file_list_source_file.nil? + default = Dir.glob(test_file_pattern).uniq.sort - File.read(test_file_list_source_file) + return default if test_file_list_source_file.nil? + + test_file_list = File.read(test_file_list_source_file) .split(/\n/) .uniq .sort + + return default if test_file_list.empty? + return test_file_list end protected From 7c47f46f51bb06f2867dd9e681f3fbaefcd4e302 Mon Sep 17 00:00:00 2001 From: Alexander Jeurissen Date: Tue, 20 Apr 2021 18:48:00 +0200 Subject: [PATCH 5/5] Add puts, so we print out valuable knapsack info (even with log disabled) --- lib/knapsack/runners/rspec_runner.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/knapsack/runners/rspec_runner.rb b/lib/knapsack/runners/rspec_runner.rb index bff1635..eb1d2b7 100644 --- a/lib/knapsack/runners/rspec_runner.rb +++ b/lib/knapsack/runners/rspec_runner.rb @@ -5,12 +5,19 @@ def self.run(args) allocator = Knapsack::AllocatorBuilder.new(Knapsack::Adapters::RSpecAdapter).allocator Knapsack.logger.info + puts '' Knapsack.logger.info 'Report specs:' + puts 'Report specs:' Knapsack.logger.info allocator.report_node_tests + puts allocator.report_node_tests Knapsack.logger.info + puts '' Knapsack.logger.info 'Leftover specs:' + puts 'Leftover specs:' Knapsack.logger.info allocator.leftover_node_tests + puts allocator.leftover_node_tests Knapsack.logger.info + puts # NOTE: return if there are no specs to execute for this node. # This can occurr if test_file_list_source_file is used with less then CI_NODES specs