Skip to content
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

Enable Split By Test Example by default in RSpec #288

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
18 changes: 16 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -293,20 +293,34 @@ jobs:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# turnip ||
mv .rspec .rspec.off
cp .rspec.turnip .rspec
export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--queue--turnip"
export KNAPSACK_PRO_FIXED_QUEUE_SPLIT=true
export KNAPSACK_PRO_TEST_DIR=turnip
export KNAPSACK_PRO_TEST_FILE_PATTERN="turnip/**/*.feature"
bundle exec rake "knapsack_pro:queue:rspec[-r turnip/rspec]"
export KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN="turnip/acceptance/foo.feature"
bundle exec rake knapsack_pro:queue:rspec
RSPEC_EXIT_CODE=$?
rm .rspec
mv .rspec.off .rspec
exit $RSPEC_EXIT_CODE
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# turnip retry ||
mv .rspec .rspec.off
cp .rspec.turnip .rspec
export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--queue--turnip"
export KNAPSACK_PRO_FIXED_QUEUE_SPLIT=true
export KNAPSACK_PRO_TEST_DIR=turnip
export KNAPSACK_PRO_TEST_FILE_PATTERN="turnip/**/*.feature"
bundle exec rake "knapsack_pro:queue:rspec[-r turnip/rspec]"
export KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN="turnip/acceptance/foo.feature"
bundle exec rake knapsack_pro:queue:rspec
RSPEC_EXIT_CODE=$?
rm .rspec
mv .rspec.off .rspec
exit $RSPEC_EXIT_CODE

e2e-regular-minitest:
parallelism: 2
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changelog

### 8.0.0

* Enable [`KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES`](https://docs.knapsackpro.com/ruby/split-by-test-examples/) by default
* This should improve the speed of your builds, but you can disable it with [`KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=false`](https://docs.knapsackpro.com/ruby/reference/#knapsack_pro_rspec_split_by_test_examples-rspec)

https://github.com/KnapsackPro/knapsack_pro-ruby/pull/288

https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v7.14.1...v8.0.0

### 7.14.1

* Improve execution time tracking for RSpec individual test examples
Expand Down
10 changes: 5 additions & 5 deletions knapsack_pro.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ Gem::Specification.new do |spec|
spec.add_dependency 'rake', '>= 0'

spec.add_development_dependency 'bundler', '>= 1.6'
spec.add_development_dependency 'cucumber', '>= 0'
spec.add_development_dependency 'minitest', '>= 5.0.0'
spec.add_development_dependency 'ostruct', '>= 0.6.0'
spec.add_development_dependency 'pry', '~> 0'
spec.add_development_dependency 'rspec', '~> 3.0'
spec.add_development_dependency 'rspec-its', '~> 1.3'
spec.add_development_dependency 'cucumber', '>= 0'
spec.add_development_dependency 'spinach', '>= 0.8'
spec.add_development_dependency 'minitest', '>= 5.0.0'
spec.add_development_dependency 'test-unit', '>= 3.0.0'
spec.add_development_dependency 'pry', '~> 0'
spec.add_development_dependency 'timecop', '>= 0.9.9'
spec.add_development_dependency 'vcr', '>= 6.0'
spec.add_development_dependency 'webmock', '>= 3.13'
spec.add_development_dependency 'timecop', '>= 0.9.9'
spec.add_development_dependency 'ostruct', '>= 0.6.0'
end
10 changes: 5 additions & 5 deletions lib/knapsack_pro/base_allocator_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ def test_dir
KnapsackPro::Config::Env.test_dir || TestFilePattern.test_dir(adapter_class)
end

# in fallback mode we always want to run the whole test files
# (not split by test cases) to guarantee that each test will be executed
# at least once across parallel CI nodes
# In Fallback Mode, we always want to run whole test files (not split by
# test cases) to guarantee that each test will be executed at least once
# across parallel CI nodes.
def fallback_mode_test_files
all_test_files_to_run
end

# detect test files present on the disk that should be run
# this may include some fast test files + slow test files split by test cases
# Detect test files present on the disk that should be run.
# This may include fast test files + slow test files split by test cases.
def fast_and_slow_test_files_to_run
test_files_to_run = all_test_files_to_run

Expand Down
11 changes: 9 additions & 2 deletions lib/knapsack_pro/config/env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,17 @@ def cucumber_queue_prefix
def rspec_split_by_test_examples?
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'
env = ENV['KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES']

if test_files_encrypted? && env.nil?
KnapsackPro.logger.warn("Skipping split by test examples because test file names encryption is enabled:\n#{KnapsackPro::Urls::ENCRYPTION}\n#{KnapsackPro::Urls::SPLIT_BY_TEST_EXAMPLES}")
return (@rspec_split_by_test_examples = false)
end

split = (env || true).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)')
KnapsackPro.logger.debug('Skipping split by test examples because tests are running on a single CI node (no parallelism)')
@rspec_split_by_test_examples = false
else
@rspec_split_by_test_examples = split
Expand Down
15 changes: 0 additions & 15 deletions spec/integration/runners/queue/rspec_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1961,16 +1961,13 @@ def when_first_matching_example_defined(type:)

context 'when the RSpec split by test examples is enabled' do
before do
ENV['KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES'] = 'true'

# remember to stub Queue API batches to include test examples (example: a_spec.rb[1:1])
# for the following slow test files
ENV['KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN'] = "#{SPEC_DIRECTORY}/a_spec.rb"

ENV['KNAPSACK_PRO_CI_NODE_TOTAL'] = '2'
end
after do
ENV.delete('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES')
ENV.delete('KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN')
ENV.delete('KNAPSACK_PRO_CI_NODE_TOTAL')
end
Expand Down Expand Up @@ -2060,16 +2057,13 @@ def when_first_matching_example_defined(type:)

context 'when the RSpec split by test examples is enabled AND --tag is set' do
before do
ENV['KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES'] = 'true'

# remember to stub Queue API batches to include test examples (example: a_spec.rb[1:1])
# for the following slow test files
ENV['KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN'] = "#{SPEC_DIRECTORY}/a_spec.rb"

ENV['KNAPSACK_PRO_CI_NODE_TOTAL'] = '2'
end
after do
ENV.delete('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES')
ENV.delete('KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN')
ENV.delete('KNAPSACK_PRO_CI_NODE_TOTAL')
end
Expand Down Expand Up @@ -2141,16 +2135,13 @@ def when_first_matching_example_defined(type:)
let(:json_file) { "#{SPEC_DIRECTORY}/rspec.json" }

before do
ENV['KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES'] = 'true'

# remember to stub Queue API batches to include test examples (example: a_spec.rb[1:1])
# for the following slow test files
ENV['KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN'] = "#{SPEC_DIRECTORY}/a_spec.rb"

ENV['KNAPSACK_PRO_CI_NODE_TOTAL'] = '2'
end
after do
ENV.delete('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES')
ENV.delete('KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN')
ENV.delete('KNAPSACK_PRO_CI_NODE_TOTAL')
end
Expand Down Expand Up @@ -2243,16 +2234,13 @@ def when_first_matching_example_defined(type:)
let(:xml_file) { "#{SPEC_DIRECTORY}/rspec.xml" }

before do
ENV['KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES'] = 'true'

# remember to stub Queue API batches to include test examples (example: a_spec.rb[1:1])
# for the following slow test files
ENV['KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN'] = "#{SPEC_DIRECTORY}/a_spec.rb"

ENV['KNAPSACK_PRO_CI_NODE_TOTAL'] = '2'
end
after do
ENV.delete('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES')
ENV.delete('KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN')
ENV.delete('KNAPSACK_PRO_CI_NODE_TOTAL')
end
Expand Down Expand Up @@ -2343,16 +2331,13 @@ def when_first_matching_example_defined(type:)
let(:coverage_file) { "#{coverage_dir}/index.html" }

before do
ENV['KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES'] = 'true'

# remember to stub Queue API batches to include test examples (example: a_spec.rb[1:1])
# for the following slow test files
ENV['KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN'] = "#{SPEC_DIRECTORY}/a_spec.rb"

ENV['KNAPSACK_PRO_CI_NODE_TOTAL'] = '2'
end
after do
ENV.delete('KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES')
ENV.delete('KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN')
ENV.delete('KNAPSACK_PRO_CI_NODE_TOTAL')
end
Expand Down
63 changes: 26 additions & 37 deletions spec/knapsack_pro/config/env_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -977,47 +977,36 @@
described_class.remove_instance_variable(:@rspec_split_by_test_examples)
end

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

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' }) }

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)')
[
['false', '2', nil, false],
['true', '2', nil, true],
[nil, '2', nil, true],
['false', '1', nil, false],
['true', '1', nil, false, :debug, 'Skipping split by test examples because tests are running on a single CI node (no parallelism)'],
[nil, '1', nil, false, :debug, 'Skipping split by test examples because tests are running on a single CI node (no parallelism)'],
['false', '2', 'true', false],
['true', '2', 'true', true],
[nil, '2', 'true', false, :warn, "Skipping split by test examples because test file names encryption is enabled:\nhttps://knapsackpro.com/perma/ruby/encryption\nhttps://knapsackpro.com/perma/ruby/split-by-test-examples"],
['false', '1', 'true', false],
['true', '1', 'true', false, :debug, 'Skipping split by test examples because tests are running on a single CI node (no parallelism)'],
[nil, '1', 'true', false, :warn, "Skipping split by test examples because test file names encryption is enabled:\nhttps://knapsackpro.com/perma/ruby/encryption\nhttps://knapsackpro.com/perma/ruby/split-by-test-examples"],
].each do |sbte, node_total, encrypted, expected, log_level, log_message|
context "KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=#{sbte.inspect} AND KNAPSACK_PRO_CI_NODE_TOTAL=#{node_total.inspect} AND KNAPSACK_PRO_TEST_FILES_ENCRYPTED=#{encrypted.inspect}" do
before do
stub_const("ENV", { 'KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES' => sbte, 'KNAPSACK_PRO_CI_NODE_TOTAL' => node_total, 'KNAPSACK_PRO_TEST_FILES_ENCRYPTED' => encrypted }.compact)

2.times { described_class.rspec_split_by_test_examples? }
if log_level && log_message
logger = instance_double(Logger)
expect(KnapsackPro).to receive(:logger).and_return(logger)
expect(logger).to receive(log_level).once.with(log_message)
end
end
end
end

context "when ENV doesn't exist" do
before do
stub_const("ENV", {})
expect(KnapsackPro).not_to receive(:logger)
it do
expect(described_class.rspec_split_by_test_examples?).to eq(expected)
expect(described_class.rspec_split_by_test_examples?).to eq(expected)
end
end

it { should be false }
end
end

Expand Down