diff --git a/.circleci/config.yml b/.circleci/config.yml index bca1d783..c18e968d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -277,6 +277,16 @@ jobs: export KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true export KNAPSACK_PRO_SLOW_TEST_FILE_THRESHOLD=1 bundle exec rake knapsack_pro:queue:rspec + - run: + working_directory: ~/rails-app-with-knapsack_pro + command: | + # split by test examples AND a single CI node || + export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--queue--split--single-node" + export KNAPSACK_PRO_FIXED_QUEUE_SPLIT=true + export KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true + export KNAPSACK_PRO_CI_NODE_TOTAL=1 + export KNAPSACK_PRO_CI_NODE_INDEX=0 + bundle exec rake knapsack_pro:queue:rspec - run: working_directory: ~/rails-app-with-knapsack_pro command: | diff --git a/CHANGELOG.md b/CHANGELOG.md index acaadce6..729a17d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +### 7.12.1 + +* fix(RSpec split by examples): properly disable split by test examples on a single node to speed up tests + + https://github.com/KnapsackPro/knapsack_pro-ruby/pull/283 + +https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v7.12.0...v7.12.1 + ### 7.12.0 * Add `KNAPSACK_PRO_SLOW_TEST_FILE_THRESHOLD` to improve the RSpec split by examples feature with many skipped tests diff --git a/lib/knapsack_pro/base_allocator_builder.rb b/lib/knapsack_pro/base_allocator_builder.rb index ed4252be..aca3fe33 100644 --- a/lib/knapsack_pro/base_allocator_builder.rb +++ b/lib/knapsack_pro/base_allocator_builder.rb @@ -36,11 +36,6 @@ def fast_and_slow_test_files_to_run test_files_to_run = all_test_files_to_run if adapter_class.split_by_test_cases_enabled? - if KnapsackPro::Config::Env.ci_node_total < 2 - KnapsackPro.logger.warn('Skipping split of test files by test cases because you are running tests on a single CI node (no parallelism)') - return test_files_to_run - end - slow_test_files = get_slow_test_files return test_files_to_run if slow_test_files.empty? diff --git a/lib/knapsack_pro/config/env.rb b/lib/knapsack_pro/config/env.rb index 1c2838cd..01e5c4ee 100644 --- a/lib/knapsack_pro/config/env.rb +++ b/lib/knapsack_pro/config/env.rb @@ -188,12 +188,17 @@ def cucumber_queue_prefix ENV.fetch('KNAPSACK_PRO_CUCUMBER_QUEUE_PREFIX', 'bundle exec') end - def rspec_split_by_test_examples - ENV.fetch('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES', false) - end - def rspec_split_by_test_examples? - rspec_split_by_test_examples.to_s == 'true' + return @rspec_split_by_test_examples if defined?(@rspec_split_by_test_examples) + + split = ENV.fetch('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES', false).to_s == 'true' + + if split && ci_node_total < 2 + KnapsackPro.logger.debug('Skipping split of test files by test examples because you are running tests on a single CI node (no parallelism)') + @rspec_split_by_test_examples = false + else + @rspec_split_by_test_examples = split + end end def rspec_test_example_detector_prefix diff --git a/spec/knapsack_pro/base_allocator_builder_spec.rb b/spec/knapsack_pro/base_allocator_builder_spec.rb index f6abae25..af140ea1 100644 --- a/spec/knapsack_pro/base_allocator_builder_spec.rb +++ b/spec/knapsack_pro/base_allocator_builder_spec.rb @@ -134,57 +134,39 @@ before do expect(adapter_class).to receive(:split_by_test_cases_enabled?).and_return(true) - expect(KnapsackPro::Config::Env).to receive(:ci_node_total).and_return(node_total) test_file_pattern = double expect(KnapsackPro::TestFilePattern).to receive(:call).with(adapter_class).and_return(test_file_pattern) expect(KnapsackPro::TestFileFinder).to receive(:call).with(test_file_pattern).and_return(test_files_to_run) - end - - context 'when less than 2 CI nodes' do - let(:node_total) { 1 } - it 'returns test files without test cases' do - logger = instance_double(Logger) - expect(KnapsackPro).to receive(:logger).and_return(logger) - expect(logger).to receive(:warn).with('Skipping split of test files by test cases because you are running tests on a single CI node (no parallelism)') - expect(subject).to eq test_files_to_run - end + expect(allocator_builder).to receive(:get_slow_test_files).and_return(slow_test_files) end - context 'when at least 2 CI nodes' do - let(:node_total) { 2 } - - before do - expect(allocator_builder).to receive(:get_slow_test_files).and_return(slow_test_files) + context 'when slow test files are detected' do + let(:slow_test_files) do + [ + '1_spec.rb', + '2_spec.rb', + ] end - context 'when slow test files are detected' do - let(:slow_test_files) do - [ - '1_spec.rb', - '2_spec.rb', - ] - end - - it 'returns test files with test cases' do - test_file_cases = double - expect(adapter_class).to receive(:test_file_cases_for).with(slow_test_files).and_return(test_file_cases) + it 'returns test files with test cases' do + test_file_cases = double + expect(adapter_class).to receive(:test_file_cases_for).with(slow_test_files).and_return(test_file_cases) - test_files_with_test_cases = double - expect(KnapsackPro::TestFilesWithTestCasesComposer).to receive(:call).with(test_files_to_run, slow_test_files, test_file_cases).and_return(test_files_with_test_cases) + test_files_with_test_cases = double + expect(KnapsackPro::TestFilesWithTestCasesComposer).to receive(:call).with(test_files_to_run, slow_test_files, test_file_cases).and_return(test_files_with_test_cases) - expect(subject).to eq test_files_with_test_cases - end + expect(subject).to eq test_files_with_test_cases end + end - context 'when slow test files are not detected' do - let(:slow_test_files) { [] } + context 'when slow test files are not detected' do + let(:slow_test_files) { [] } - it 'returns test files without test cases' do - expect(subject).to eq test_files_to_run - end + it 'returns test files without test cases' do + expect(subject).to eq test_files_to_run end end end diff --git a/spec/knapsack_pro/config/env_spec.rb b/spec/knapsack_pro/config/env_spec.rb index b394a085..af4b21dc 100644 --- a/spec/knapsack_pro/config/env_spec.rb +++ b/spec/knapsack_pro/config/env_spec.rb @@ -1013,37 +1013,54 @@ end end - describe '.rspec_split_by_test_examples' do - subject { described_class.rspec_split_by_test_examples } + describe '.rspec_split_by_test_examples?' do + subject { described_class.rspec_split_by_test_examples? } - context 'when ENV exists' do - before { stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => true }) } - it { should eq true } + before do + described_class.remove_instance_variable(:@rspec_split_by_test_examples) + rescue NameError end - context "when ENV doesn't exist" do - before { stub_const("ENV", {}) } + context 'when KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true AND KNAPSACK_PRO_CI_NODE_TOTAL >= 2' do + before do + stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => 'true', 'KNAPSACK_PRO_CI_NODE_TOTAL' => '2' }) + expect(KnapsackPro).not_to receive(:logger) + end + + it { should be true } + end + + context 'when KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=false AND KNAPSACK_PRO_CI_NODE_TOTAL >= 2' do + before do + stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => 'false', 'KNAPSACK_PRO_CI_NODE_TOTAL' => '2' }) + expect(KnapsackPro).not_to receive(:logger) + end + it { should be false } end - end - describe '.rspec_split_by_test_examples?' do - subject { described_class.rspec_split_by_test_examples? } + context 'when KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true AND KNAPSACK_PRO_CI_NODE_TOTAL < 2' do + before { stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => 'true', 'KNAPSACK_PRO_CI_NODE_TOTAL' => '1' }) } - context 'when ENV exists' do - context 'when KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true' do - before { stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => 'true' }) } - it { should be true } - end + it { should be false } - context 'when KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=false' do - before { stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => 'false' }) } - it { should be false } + context 'when called twice' do + it 'logs a debug message only once' do + logger = instance_double(Logger) + expect(KnapsackPro).to receive(:logger).and_return(logger) + expect(logger).to receive(:debug).with('Skipping split of test files by test examples because you are running tests on a single CI node (no parallelism)') + + 2.times { described_class.rspec_split_by_test_examples? } + end end end context "when ENV doesn't exist" do - before { stub_const("ENV", {}) } + before do + stub_const("ENV", {}) + expect(KnapsackPro).not_to receive(:logger) + end + it { should be false } end end