From 48350a9449d01efb77cce3b5b359f9a3a0b393a0 Mon Sep 17 00:00:00 2001 From: Alex Speller <1217+alexspeller@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:44:58 +0000 Subject: [PATCH 1/5] Cucumber: add support for Split by Test Examples Split slow feature files by scenario across parallel CI nodes, mirroring the existing RSpec Split by Test Examples feature. Scenarios are addressed as file:line test example paths (e.g., features/a.feature:12), which Cucumber natively supports on the command line. Scenario Outline example rows are split individually because the JSON formatter expands them to one element per row. Opt in with KNAPSACK_PRO_CUCUMBER_SPLIT_BY_TEST_EXAMPLES=true. Requires Cucumber >= 4.0 (verified against Cucumber 4.1, 7.1, and 11.1). - KnapsackPro::Adapters::CucumberAdapter implements split_by_test_cases_enabled?, calculate_slow_id_paths, and the id-path helpers used by KnapsackPro::TestSuite, so both Regular Mode and Queue Mode pick up the split through the existing generic flow. - The rake task knapsack_pro:cucumber_test_example_detector runs cucumber --dry-run --format json on the slow test files (fetched from the API build distribution, like RSpec) to enumerate scenario paths. The dry run respects the user's cucumber args (e.g. --tags), which are parsed with Shellwords to keep quoted tag expressions intact. - The Around hook records time per scenario path when the scenario's file was split (Tracker#scheduled_test_path?), and per file otherwise. - KnapsackPro::TestCaseMergers::CucumberMerger merges file:line entries fetched from the API back into whole-file times before determining slow test files, mirroring the RSpec merger. --- CHANGELOG.md | 2 + lib/knapsack_pro.rb | 2 + lib/knapsack_pro/adapters/cucumber_adapter.rb | 71 ++++++- lib/knapsack_pro/config/env.rb | 22 +++ lib/knapsack_pro/runners/cucumber_runner.rb | 1 + .../runners/queue/cucumber_runner.rb | 1 + .../cucumber_test_example_detector.rb | 82 ++++++++ .../test_case_mergers/cucumber_merger.rb | 34 ++++ lib/knapsack_pro/tracker.rb | 8 + lib/tasks/cucumber.rake | 15 ++ .../adapters/cucumber_adapter_spec.rb | 178 ++++++++++++++++++ spec/knapsack_pro/config/env_spec.rb | 57 ++++++ .../runners/cucumber_runner_spec.rb | 1 + .../runners/queue/cucumber_runner_spec.rb | 1 + .../cucumber_test_example_detector_spec.rb | 172 +++++++++++++++++ .../test_case_mergers/cucumber_merger_spec.rb | 99 ++++++++++ spec/knapsack_pro/tracker_spec.rb | 28 +++ 17 files changed, 773 insertions(+), 1 deletion(-) create mode 100644 lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb create mode 100644 lib/knapsack_pro/test_case_mergers/cucumber_merger.rb create mode 100644 spec/knapsack_pro/test_case_detectors/cucumber_test_example_detector_spec.rb create mode 100644 spec/knapsack_pro/test_case_mergers/cucumber_merger_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 59728345..a5c1dfa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ### Unreleased +* Cucumber: Add support for [Split by Test Examples](https://docs.knapsackpro.com/ruby/split-by-test-examples/) so slow feature files can be split by scenario (e.g., `features/a.feature:12`) across parallel CI nodes. Opt in with `KNAPSACK_PRO_CUCUMBER_SPLIT_BY_TEST_EXAMPLES=true` (requires Cucumber >= 4.0). + ### 10.0.1 * Add support for [File Paths Encryption](https://docs.knapsackpro.com/ruby/encryption/) to [Retry only Failures](https://docs.knapsackpro.com/ruby/retry-only-failures/). diff --git a/lib/knapsack_pro.rb b/lib/knapsack_pro.rb index 4820cea3..424285e0 100644 --- a/lib/knapsack_pro.rb +++ b/lib/knapsack_pro.rb @@ -58,6 +58,7 @@ require_relative 'knapsack_pro/mask_string' require_relative 'knapsack_pro/test_suite' require_relative 'knapsack_pro/test_case_mergers/rspec_merger' +require_relative 'knapsack_pro/test_case_mergers/cucumber_merger' require_relative 'knapsack_pro/build_distribution_fetcher' require_relative 'knapsack_pro/slow_test_file_determiner' require_relative 'knapsack_pro/base_allocator_builder' @@ -76,6 +77,7 @@ require_relative 'knapsack_pro/runners/queue/cucumber_runner' require_relative 'knapsack_pro/runners/queue/minitest_runner' require_relative 'knapsack_pro/test_case_detectors/rspec_test_example_detector' +require_relative 'knapsack_pro/test_case_detectors/cucumber_test_example_detector' require_relative 'knapsack_pro/crypto/encryptor' require_relative 'knapsack_pro/crypto/branch_encryptor' require_relative 'knapsack_pro/crypto/decryptor' diff --git a/lib/knapsack_pro/adapters/cucumber_adapter.rb b/lib/knapsack_pro/adapters/cucumber_adapter.rb index 8f05791e..9596b582 100644 --- a/lib/knapsack_pro/adapters/cucumber_adapter.rb +++ b/lib/knapsack_pro/adapters/cucumber_adapter.rb @@ -4,6 +4,75 @@ module KnapsackPro module Adapters class CucumberAdapter < BaseAdapter TEST_DIR_PATTERN = 'features/**{,/*/**}/*.feature' + # Matches a test example path like features/a.feature:12 + REGEX = /\A(.*?)(?::(\d+))?\z/.freeze + + def self.split_by_test_cases_enabled? + return false unless KnapsackPro::Config::Env.cucumber_split_by_test_examples? + + require 'cucumber/platform' + unless ::Cucumber::VERSION.to_i >= 4 + raise "Cucumber >= 4.0 is required to split test files by test examples. Learn more: #{KnapsackPro::Urls::SPLIT_BY_TEST_EXAMPLES}" + end + + true + end + + def self.calculate_slow_id_paths + # Shell out not to pollute the Cucumber state + cmd = [ + 'RACK_ENV=test', + 'RAILS_ENV=test', + KnapsackPro::Config::Env.cucumber_test_example_detector_prefix, + 'rake knapsack_pro:cucumber_test_example_detector', + ].join(' ') + raise "Failed to calculate Split by Test Examples: #{cmd}" unless Kernel.system(cmd) + + KnapsackPro::TestCaseDetectors::CucumberTestExampleDetector.new.slow_id_paths! + end + + def self.parse_file_path(path) + file, _id = path.match(REGEX).captures + file + end + + def self.id_path?(path) + _file, id = path.match(REGEX).captures + !id.nil? + end + + def self.concat_test_files(test_files, id_paths) + paths = concat_paths(test_files, id_paths) + KnapsackPro::TestFilePresenter.test_files(paths) + end + + def self.concat_paths(test_files, id_paths) + paths = KnapsackPro::TestFilePresenter.paths(test_files) + file_paths = id_paths.map { |id_path| parse_file_path(id_path) } + paths + id_paths - file_paths + end + + def self.remove_formatters(cli_args) + formatter_options = ['-f', '--format', '-o', '--out'] + cli_args.dup.each_with_index do |arg, index| + if formatter_options.include?(arg) + cli_args[index] = nil + cli_args[index + 1] = nil + end + end + cli_args.compact + end + + # When the test file was split by test examples, track the time execution + # per test example path (e.g., features/a.feature:12). + # Otherwise, track the time execution per test file path. + def self.tracked_test_path(object) + file = test_path(object) + return file unless split_by_test_cases_enabled? + + id_path = "#{file}:#{object.location.line}" + KnapsackPro.tracker.scheduled_test_path?(id_path) ? id_path : file + end def self.test_path(object) if ::Cucumber::VERSION.to_i >= 2 @@ -32,7 +101,7 @@ def self.test_path(object) def bind_time_tracker Around do |object, block| - KnapsackPro.tracker.current_test_path = KnapsackPro::Adapters::CucumberAdapter.test_path(object) + KnapsackPro.tracker.current_test_path = KnapsackPro::Adapters::CucumberAdapter.tracked_test_path(object) KnapsackPro.tracker.start_timer block.call KnapsackPro.tracker.stop_timer diff --git a/lib/knapsack_pro/config/env.rb b/lib/knapsack_pro/config/env.rb index d03ba0af..e44eb99c 100644 --- a/lib/knapsack_pro/config/env.rb +++ b/lib/knapsack_pro/config/env.rb @@ -202,6 +202,28 @@ def rspec_test_example_detector_prefix ENV.fetch('KNAPSACK_PRO_RSPEC_TEST_EXAMPLE_DETECTOR_PREFIX', 'bundle exec') end + def cucumber_split_by_test_examples? + return @cucumber_split_by_test_examples if defined?(@cucumber_split_by_test_examples) + + split = ENV['KNAPSACK_PRO_CUCUMBER_SPLIT_BY_TEST_EXAMPLES'].to_s == 'true' + + if split && test_files_encrypted? + 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 (@cucumber_split_by_test_examples = false) + end + + if split && ci_node_total < 2 + KnapsackPro.logger.debug('Skipping split by test examples because tests are running on a single CI node (no parallelism)') + @cucumber_split_by_test_examples = false + else + @cucumber_split_by_test_examples = split + end + end + + def cucumber_test_example_detector_prefix + ENV.fetch('KNAPSACK_PRO_CUCUMBER_TEST_EXAMPLE_DETECTOR_PREFIX', 'bundle exec') + end + def test_suite_token env_name = 'KNAPSACK_PRO_TEST_SUITE_TOKEN' ENV[env_name] || raise("Missing environment variable #{env_name}. You should set environment variable like #{env_name}_RSPEC (note there is suffix _RSPEC at the end). knapsack_pro gem will set #{env_name} based on #{env_name}_RSPEC value. If you use other test runner than RSpec then use proper suffix.") diff --git a/lib/knapsack_pro/runners/cucumber_runner.rb b/lib/knapsack_pro/runners/cucumber_runner.rb index af09dd00..faf63b90 100644 --- a/lib/knapsack_pro/runners/cucumber_runner.rb +++ b/lib/knapsack_pro/runners/cucumber_runner.rb @@ -6,6 +6,7 @@ class CucumberRunner < BaseRunner def self.run(args) ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_cucumber ENV['KNAPSACK_PRO_REGULAR_MODE_ENABLED'] = 'true' + ENV['KNAPSACK_PRO_CUCUMBER_OPTIONS'] = args.to_s adapter_class = KnapsackPro::Adapters::CucumberAdapter KnapsackPro::Config::Env.set_test_runner_adapter(adapter_class) diff --git a/lib/knapsack_pro/runners/queue/cucumber_runner.rb b/lib/knapsack_pro/runners/queue/cucumber_runner.rb index 5ce62ab2..53d57765 100644 --- a/lib/knapsack_pro/runners/queue/cucumber_runner.rb +++ b/lib/knapsack_pro/runners/queue/cucumber_runner.rb @@ -10,6 +10,7 @@ def self.run(args) ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_cucumber ENV['KNAPSACK_PRO_QUEUE_MODE_ENABLED'] = 'true' ENV['KNAPSACK_PRO_QUEUE_ID'] = KnapsackPro::Config::EnvGenerator.set_queue_id + ENV['KNAPSACK_PRO_CUCUMBER_OPTIONS'] = args.to_s adapter_class = KnapsackPro::Adapters::CucumberAdapter KnapsackPro::Config::Env.set_test_runner_adapter(adapter_class) diff --git a/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb b/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb new file mode 100644 index 00000000..32548a43 --- /dev/null +++ b/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'shellwords' + +module KnapsackPro + module TestCaseDetectors + class CucumberTestExampleDetector + def dry_run_to_file(cucumber_args) + KnapsackPro::Config::TempFiles.ensure_temp_directory_exists! + FileUtils.mkdir_p(File.dirname(report_path)) + File.delete(report_path) if File.exist?(report_path) + + slow_test_files = fetch_slow_file_paths + return File.write(report_path, [].to_json) if slow_test_files.empty? + + KnapsackPro.logger.info("Calculating Split by Test Examples. Analyzing #{slow_test_files.size} slow test files.") + # Shellwords to respect quoted args like --tags "not @slow" + args = Shellwords.split(cucumber_args || '') + cli_args_without_formatters = KnapsackPro::Adapters::CucumberAdapter.remove_formatters(args) + cli_args = cli_args_without_formatters + [ + '--dry-run', + '--format', 'json', + '--out', report_path, + '--require', test_dir, + ] + KnapsackPro::TestFilePresenter.paths(slow_test_files) + exit_code = dry_run(cli_args) + return if exit_code.zero? + + command = (['bundle exec cucumber'] + cli_args).join(' ') + KnapsackPro.logger.error("Failed to calculate Split by Test Examples: #{command}") + exit exit_code + end + + def slow_id_paths! + raise "No report found at #{report_path}" unless File.exist?(report_path) + + JSON.parse(File.read(report_path)) + .flat_map { |feature| feature.fetch('elements', []).map { |element| [feature, element] } } + .select { |_feature, element| element['type'] == 'scenario' } + .map { |feature, element| TestFileCleaner.clean("#{feature.fetch('uri')}:#{element.fetch('line')}") } + end + + private + + def dry_run(cli_args) + require 'cucumber' + + ::Cucumber::Cli::Main.new(cli_args).execute! + 0 + rescue SystemExit => e + e.status + end + + def report_path + "#{KnapsackPro::Config::TempFiles::TEMP_DIRECTORY_PATH}/test_case_detectors/cucumber/cucumber_dry_run_json_report_node_#{KnapsackPro::Config::Env.ci_node_index}.json" + end + + def fetch_slow_file_paths + if KnapsackPro::Config::Env.slow_test_file_pattern + return KnapsackPro::TestFileFinder.slow_test_files_by_pattern(adapter_class) + end + + if KnapsackPro::Config::Env.test_files_encrypted? + raise "Split by test cases is not possible when you have enabled test file names encryption ( #{KnapsackPro::Urls::ENCRYPTION} ). You need to disable encryption with KNAPSACK_PRO_TEST_FILES_ENCRYPTED=false in order to use split by test cases #{KnapsackPro::Urls::SPLIT_BY_TEST_EXAMPLES}" + end + + build_distribution = KnapsackPro::BuildDistributionFetcher.new.call + merged_test_files_from_api = KnapsackPro::TestCaseMergers::CucumberMerger.new(build_distribution.test_files).call + test_files_existing_on_disk = KnapsackPro::TestFileFinder.select_test_files_that_can_be_run(adapter_class, merged_test_files_from_api) + KnapsackPro::SlowTestFileDeterminer.call(test_files_existing_on_disk) + end + + def adapter_class + KnapsackPro::Adapters::CucumberAdapter + end + + def test_dir + KnapsackPro::Config::Env.test_dir || KnapsackPro::TestFilePattern.test_dir(adapter_class) + end + end + end +end diff --git a/lib/knapsack_pro/test_case_mergers/cucumber_merger.rb b/lib/knapsack_pro/test_case_mergers/cucumber_merger.rb new file mode 100644 index 00000000..1df16771 --- /dev/null +++ b/lib/knapsack_pro/test_case_mergers/cucumber_merger.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module KnapsackPro + module TestCaseMergers + class CucumberMerger + def initialize(test_files) + @test_files = test_files + end + + def call + file_paths = {} + id_paths = {} + + @test_files.each do |test_file| + raw_path = test_file.fetch('path') + file_path = KnapsackPro::Adapters::CucumberAdapter.parse_file_path(raw_path) + + if KnapsackPro::Adapters::CucumberAdapter.id_path?(raw_path) + id_paths[file_path] ||= 0.0 + id_paths[file_path] += test_file.fetch('time_execution') + else + file_paths[file_path] = test_file.fetch('time_execution') # may be nil + end + end + + file_paths + .merge(id_paths) { |_, v1, v2| [v1, v2].compact.max } + .map do |file_path, time_execution| + { 'path' => file_path, 'time_execution' => time_execution } + end + end + end + end +end diff --git a/lib/knapsack_pro/tracker.rb b/lib/knapsack_pro/tracker.rb index 53916392..4f10cb08 100644 --- a/lib/knapsack_pro/tracker.rb +++ b/lib/knapsack_pro/tracker.rb @@ -69,6 +69,14 @@ def set_prerun_tests(test_file_paths) @prerun_tests_loaded = true end + def scheduled_test_path?(test_file_path) + # When the test files are not loaded in the memory then load them from the disk. + # Useful for the Regular Mode when the memory is not shared between tracker instances. + load_prerun_tests unless prerun_tests_loaded + + @test_files_with_time.key?(KnapsackPro::TestFileCleaner.clean(test_file_path)) + end + def to_a # When the test files are not loaded in the memory then load them from the disk. # Useful for the Regular Mode when the memory is not shared between tracker instances. diff --git a/lib/tasks/cucumber.rake b/lib/tasks/cucumber.rake index b335d25f..b92c4697 100644 --- a/lib/tasks/cucumber.rake +++ b/lib/tasks/cucumber.rake @@ -6,4 +6,19 @@ namespace :knapsack_pro do task :cucumber, [:cucumber_args] do |_, args| KnapsackPro::Runners::CucumberRunner.run(args[:cucumber_args]) end + + # private + task :cucumber_test_example_detector do + key = 'KNAPSACK_PRO_CUCUMBER_OPTIONS' + raise "The internal #{key} environment variable is unset. Ensure it is not overridden accidentally. Otherwise, please report this as a bug: #{KnapsackPro::Urls::SUPPORT}" if ENV[key].nil? + + # The adapter bound in the support code (e.g., features/support/env.rb) must not + # register time tracking hooks or save a report to the API during the dry run. + ENV.delete('KNAPSACK_PRO_REGULAR_MODE_ENABLED') + ENV.delete('KNAPSACK_PRO_QUEUE_MODE_ENABLED') + + KnapsackPro::TestCaseDetectors::CucumberTestExampleDetector + .new + .dry_run_to_file(ENV[key]) + end end diff --git a/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb b/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb index 4b3103e0..b57b6f15 100644 --- a/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb +++ b/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb @@ -90,6 +90,184 @@ end end + describe '.split_by_test_cases_enabled?' do + subject { described_class.split_by_test_cases_enabled? } + + before do + expect(KnapsackPro::Config::Env).to receive(:cucumber_split_by_test_examples?).and_return(cucumber_split_by_test_examples_enabled) + end + + context 'when the Cucumber split by test examples is enabled' do + let(:cucumber_split_by_test_examples_enabled) { true } + + before { stub_const('Cucumber::VERSION', '4.1.0') } + + it { expect(subject).to be true } + + context 'when the Cucumber version is < 4.0' do + before { stub_const('Cucumber::VERSION', '3.2.0') } + + it do + expect { subject }.to raise_error RuntimeError, 'Cucumber >= 4.0 is required to split test files by test examples. Learn more: https://knapsackpro.com/perma/ruby/split-by-test-examples' + end + end + end + + context 'when the Cucumber split by test examples is disabled' do + let(:cucumber_split_by_test_examples_enabled) { false } + + it { expect(subject).to be false } + end + end + + describe '.calculate_slow_id_paths' do + subject { described_class.calculate_slow_id_paths } + + before do + cmd = 'RACK_ENV=test RAILS_ENV=test bundle exec rake knapsack_pro:cucumber_test_example_detector' + expect(Kernel).to receive(:system).with(cmd).and_return(cmd_result) + end + + context 'when the rake task to detect Cucumber test examples succeeded' do + let(:cmd_result) { true } + + it 'returns test example paths for slow test files' do + cucumber_test_example_detector = instance_double(KnapsackPro::TestCaseDetectors::CucumberTestExampleDetector) + expect(KnapsackPro::TestCaseDetectors::CucumberTestExampleDetector).to receive(:new).and_return(cucumber_test_example_detector) + + slow_id_paths = double + expect(cucumber_test_example_detector).to receive(:slow_id_paths!).and_return(slow_id_paths) + + expect(subject).to eq slow_id_paths + end + end + + context 'when the rake task to detect Cucumber test examples failed' do + let(:cmd_result) { false } + + it do + expect { subject }.to raise_error(RuntimeError, 'Failed to calculate Split by Test Examples: RACK_ENV=test RAILS_ENV=test bundle exec rake knapsack_pro:cucumber_test_example_detector') + end + end + end + + describe '.parse_file_path' do + subject { described_class.parse_file_path(path) } + + context 'when the path is a test example path' do + let(:path) { 'features/a.feature:12' } + + it { is_expected.to eq 'features/a.feature' } + end + + context 'when the path is a test file path' do + let(:path) { 'features/a.feature' } + + it { is_expected.to eq 'features/a.feature' } + end + end + + describe '.id_path?' do + subject { described_class.id_path?(path) } + + context 'when the path is a test example path' do + let(:path) { 'features/a.feature:12' } + + it { is_expected.to be true } + end + + context 'when the path is a test file path' do + let(:path) { 'features/a.feature' } + + it { is_expected.to be false } + end + end + + describe '.concat_test_files' do + let(:test_files) do + [ + { 'path' => 'features/a.feature' }, + { 'path' => 'features/b.feature' }, + { 'path' => 'features/c.feature' }, + { 'path' => 'features/slow_1.feature' }, + { 'path' => 'features/slow_2.feature' }, + ] + end + + let(:id_paths) do + [ + 'features/slow_1.feature:3', + 'features/slow_1.feature:12', + 'features/slow_2.feature:5', + 'features/slow_2.feature:14', + 'features/slow_2.feature:15', + ] + end + + subject { described_class.concat_test_files(test_files, id_paths) } + + it 'concats by replacing test_files with the associated id_paths' do + expect(subject).to eq([ + { 'path' => 'features/a.feature' }, + { 'path' => 'features/b.feature' }, + { 'path' => 'features/c.feature' }, + { 'path' => 'features/slow_1.feature:3' }, + { 'path' => 'features/slow_1.feature:12' }, + { 'path' => 'features/slow_2.feature:5' }, + { 'path' => 'features/slow_2.feature:14' }, + { 'path' => 'features/slow_2.feature:15' }, + ]) + end + end + + describe '.remove_formatters' do + subject { described_class.remove_formatters(cli_args) } + + context 'when CLI args include formatters' do + let(:cli_args) { ['--tags', 'not @slow', '-f', 'pretty', '-o', '/tmp/pretty.txt', '--format', 'json', '--out', '/tmp/file.json', '--strict'] } + + it 'removes formatters and the related output file options' do + expect(subject).to eq ['--tags', 'not @slow', '--strict'] + end + end + end + + describe '.tracked_test_path' do + let(:file) { 'features/a.feature' } + let(:test_case) { double(location: double(file: file, line: 12)) } + + subject { described_class.tracked_test_path(test_case) } + + before { stub_const('Cucumber::VERSION', '4.1.0') } + + context 'when the split by test examples is disabled' do + before do + expect(described_class).to receive(:split_by_test_cases_enabled?).and_return(false) + end + + it { is_expected.to eq file } + end + + context 'when the split by test examples is enabled' do + before do + expect(described_class).to receive(:split_by_test_cases_enabled?).and_return(true) + expect(KnapsackPro.tracker).to receive(:scheduled_test_path?).with('features/a.feature:12').and_return(scheduled) + end + + context 'when the test example path is scheduled on this CI node (the test file was split by test examples)' do + let(:scheduled) { true } + + it { is_expected.to eq 'features/a.feature:12' } + end + + context 'when the test example path is not scheduled on this CI node (the test file was not split)' do + let(:scheduled) { false } + + it { is_expected.to eq file } + end + end + end + describe 'bind methods' do describe '#bind_time_tracker' do let(:file) { 'features/a.feature' } diff --git a/spec/knapsack_pro/config/env_spec.rb b/spec/knapsack_pro/config/env_spec.rb index fabddada..59af569e 100644 --- a/spec/knapsack_pro/config/env_spec.rb +++ b/spec/knapsack_pro/config/env_spec.rb @@ -960,6 +960,63 @@ end end + describe '.cucumber_split_by_test_examples?' do + subject { described_class.cucumber_split_by_test_examples? } + + before do + described_class.remove_instance_variable(:@cucumber_split_by_test_examples) if described_class.instance_variable_defined?(:@cucumber_split_by_test_examples) + end + after do + described_class.remove_instance_variable(:@cucumber_split_by_test_examples) + end + + [ + ['false', '2', nil, false], + ['true', '2', nil, true], + [nil, '2', nil, false], + ['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], + ['false', '2', 'true', false], + ['true', '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"], + [nil, '2', 'true', false], + ['false', '1', 'true', false], + ['true', '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"], + [nil, '1', 'true', false], + ].each do |sbte, node_total, encrypted, expected, log_level, log_message| + context "KNAPSACK_PRO_CUCUMBER_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_CUCUMBER_SPLIT_BY_TEST_EXAMPLES' => sbte, 'KNAPSACK_PRO_CI_NODE_TOTAL' => node_total, 'KNAPSACK_PRO_TEST_FILES_ENCRYPTED' => encrypted }.compact) + + 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 + + it do + expect(described_class.cucumber_split_by_test_examples?).to eq(expected) + expect(described_class.cucumber_split_by_test_examples?).to eq(expected) + end + end + end + end + + describe '.cucumber_test_example_detector_prefix' do + subject { described_class.cucumber_test_example_detector_prefix } + + context 'when ENV exists' do + before { stub_const("ENV", { 'KNAPSACK_PRO_CUCUMBER_TEST_EXAMPLE_DETECTOR_PREFIX' => '' }) } + it { should eq '' } + end + + context "when ENV doesn't exist" do + before { stub_const("ENV", {}) } + it { should eq 'bundle exec' } + end + end + describe '.slow_test_file_threshold' do subject { described_class.slow_test_file_threshold } diff --git a/spec/knapsack_pro/runners/cucumber_runner_spec.rb b/spec/knapsack_pro/runners/cucumber_runner_spec.rb index 386038bd..3888e2d9 100644 --- a/spec/knapsack_pro/runners/cucumber_runner_spec.rb +++ b/spec/knapsack_pro/runners/cucumber_runner_spec.rb @@ -17,6 +17,7 @@ expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_TEST_SUITE_TOKEN', test_suite_token_cucumber) expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_REGULAR_MODE_ENABLED', 'true') + expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_CUCUMBER_OPTIONS', args.to_s) expect(KnapsackPro::Config::Env).to receive(:set_test_runner_adapter).with(KnapsackPro::Adapters::CucumberAdapter) diff --git a/spec/knapsack_pro/runners/queue/cucumber_runner_spec.rb b/spec/knapsack_pro/runners/queue/cucumber_runner_spec.rb index 8da9c306..8dc90901 100644 --- a/spec/knapsack_pro/runners/queue/cucumber_runner_spec.rb +++ b/spec/knapsack_pro/runners/queue/cucumber_runner_spec.rb @@ -18,6 +18,7 @@ expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_TEST_SUITE_TOKEN', test_suite_token_cucumber) expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_QUEUE_MODE_ENABLED', 'true') expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_QUEUE_ID', queue_id) + expect(ENV).to receive(:[]=).with('KNAPSACK_PRO_CUCUMBER_OPTIONS', args.to_s) expect(KnapsackPro::Config::Env).to receive(:set_test_runner_adapter).with(KnapsackPro::Adapters::CucumberAdapter) diff --git a/spec/knapsack_pro/test_case_detectors/cucumber_test_example_detector_spec.rb b/spec/knapsack_pro/test_case_detectors/cucumber_test_example_detector_spec.rb new file mode 100644 index 00000000..cff8bcbd --- /dev/null +++ b/spec/knapsack_pro/test_case_detectors/cucumber_test_example_detector_spec.rb @@ -0,0 +1,172 @@ +require 'cucumber' + +describe KnapsackPro::TestCaseDetectors::CucumberTestExampleDetector do + let(:report_dir) { '.knapsack_pro/test_case_detectors/cucumber' } + let(:report_path) { '.knapsack_pro/test_case_detectors/cucumber/cucumber_dry_run_json_report_node_0.json' } + let(:cucumber_test_example_detector) { described_class.new } + + around(:each) do |example| + KnapsackPro.reset_logger! + $stdout = StringIO.new + $stderr = StringIO.new + KnapsackPro.stdout = $stdout + example.run + KnapsackPro.stdout = STDOUT + $stdout = STDOUT + $stderr = STDERR + KnapsackPro.reset_logger! + end + + describe '#dry_run_to_file' do + subject { cucumber_test_example_detector.dry_run_to_file(cucumber_args) } + + before do + expect(KnapsackPro::Config::TempFiles).to receive(:ensure_temp_directory_exists!) + + expect(FileUtils).to receive(:mkdir_p).with(report_dir) + + allow(File).to receive(:exist?) + expect(File).to receive(:exist?).at_least(:once).with(report_path).and_return(true) + expect(File).to receive(:delete).with(report_path) + + expect(cucumber_test_example_detector).to receive(:fetch_slow_file_paths).and_return(test_file_entities) + end + + context 'when there are no slow test files' do + let(:cucumber_args) { '' } + let(:test_file_entities) { [] } + + before do + expect(File).to receive(:write).with(report_path, [].to_json) + end + + it do + subject + end + end + + context 'when slow test files exist' do + let(:test_file_entities) do + [ + { 'path' => 'features/a.feature' }, + { 'path' => 'features/b.feature' }, + ] + end + let(:cucumber_cli_main) { double } + + before do + test_dir = 'features' + expect(KnapsackPro::Config::Env).to receive(:test_dir).and_return(nil) + expect(KnapsackPro::TestFilePattern).to receive(:test_dir).with(KnapsackPro::Adapters::CucumberAdapter).and_return(test_dir) + + expect(Cucumber::Cli::Main).to receive(:new).with(expected_args + [ + '--dry-run', + '--format', 'json', + '--out', report_path, + '--require', test_dir, + 'features/a.feature', 'features/b.feature', + ]).and_return(cucumber_cli_main) + end + + context 'when Cucumber::Cli::Main exits with the status 0' do + let(:cucumber_args) { '' } + let(:expected_args) { [] } + + before do + expect(cucumber_cli_main).to receive(:execute!).and_raise(SystemExit.new(0)) + end + + it do + subject + end + end + + context 'when Cucumber::Cli::Main exits with the status 1' do + let(:cucumber_args) { '' } + let(:expected_args) { [] } + + before do + expect(cucumber_cli_main).to receive(:execute!).and_raise(SystemExit.new(1)) + end + + it do + expect { subject }.to raise_error(SystemExit) { |error| expect(error.status).to eq 1 } + end + end + + context 'when Cucumber CLI args are present including format options' do + let(:cucumber_args) { '--tags "not @slow" --format pretty --out /tmp/pretty.txt --strict' } + let(:expected_args) { ['--tags', 'not @slow', '--strict'] } + + before do + expect(cucumber_cli_main).to receive(:execute!).and_raise(SystemExit.new(0)) + end + + it 'removes formatter options from the args used for the dry run' do + subject + end + end + end + end + + describe '#slow_id_paths!' do + subject { cucumber_test_example_detector.slow_id_paths! } + + context 'when the report exists' do + before do + expect(File).to receive(:exist?).with(report_path).and_return(true) + expect(File).to receive(:read).with(report_path).and_return(json_report) + end + + context 'when the report has scenarios' do + let(:json_report) do + [ + { + 'uri' => 'features/a.feature', + 'elements' => [ + { 'type' => 'background', 'line' => 2 }, + { 'type' => 'scenario', 'line' => 5 }, + { 'type' => 'scenario', 'line' => 9 }, + # scenario outline rows are expanded into separate elements + { 'type' => 'scenario', 'line' => 17 }, + { 'type' => 'scenario', 'line' => 18 }, + ], + }, + { + 'uri' => './features/b.feature', + 'elements' => [ + { 'type' => 'scenario', 'line' => 3 }, + ], + }, + ].to_json + end + + it 'returns test example paths for scenarios (skipping backgrounds) with cleaned file paths' do + expect(subject).to eq([ + 'features/a.feature:5', + 'features/a.feature:9', + 'features/a.feature:17', + 'features/a.feature:18', + 'features/b.feature:3', + ]) + end + end + + context 'when the report is empty' do + let(:json_report) { [].to_json } + + it { should eq [] } + end + end + + context 'when the report does not exist' do + before do + expect(File).to receive(:exist?).with(report_path).and_return(false) + end + + it do + expect { subject }.to raise_error(RuntimeError, "No report found at #{report_path}") + end + end + end +end diff --git a/spec/knapsack_pro/test_case_mergers/cucumber_merger_spec.rb b/spec/knapsack_pro/test_case_mergers/cucumber_merger_spec.rb new file mode 100644 index 00000000..f3e72926 --- /dev/null +++ b/spec/knapsack_pro/test_case_mergers/cucumber_merger_spec.rb @@ -0,0 +1,99 @@ +describe KnapsackPro::TestCaseMergers::CucumberMerger do + describe '#call' do + subject { KnapsackPro::TestCaseMergers::CucumberMerger.new(test_files).call } + + context 'when test files are regular file paths (not test example paths)' do + let(:test_files) do + [ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + { 'path' => 'features/b.feature', 'time_execution' => 2.2 }, + ] + end + + it 'returns the test files unchanged' do + expect(subject).to eq([ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + { 'path' => 'features/b.feature', 'time_execution' => 2.2 }, + ]) + end + end + + context 'when test files have test example paths' do + let(:test_files) do + [ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + # test example paths + { 'path' => 'features/test_case.feature:3', 'time_execution' => 2.2 }, + { 'path' => 'features/test_case.feature:12', 'time_execution' => 0.8 }, + ] + end + + it 'merges the test example paths and sums their execution times' do + expect(subject).to eq([ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + { 'path' => 'features/test_case.feature', 'time_execution' => 3.0 }, + ]) + end + end + + context 'when test files have test example paths and the full test file path exists simultaneously' do + context 'when the full test file path has a higher execution time than the sum of test example paths' do + let(:test_files) do + [ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + # full test file path exists alongside test example paths + { 'path' => 'features/test_case.feature', 'time_execution' => 3.1 }, + # test example paths + { 'path' => 'features/test_case.feature:3', 'time_execution' => 2.2 }, + { 'path' => 'features/test_case.feature:12', 'time_execution' => 0.8 }, + ] + end + + it 'returns the full test file path execution time' do + expect(subject).to eq([ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + { 'path' => 'features/test_case.feature', 'time_execution' => 3.1 }, + ]) + end + end + + context 'when the full test file path has a lower execution time than the sum of test example paths' do + let(:test_files) do + [ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + # full test file path exists alongside test example paths + { 'path' => 'features/test_case.feature', 'time_execution' => 2.9 }, + # test example paths + { 'path' => 'features/test_case.feature:3', 'time_execution' => 2.2 }, + { 'path' => 'features/test_case.feature:12', 'time_execution' => 0.8 }, + ] + end + + it "returns the sum of test example paths' execution times" do + expect(subject).to eq([ + { 'path' => 'features/a.feature', 'time_execution' => 1.1 }, + { 'path' => 'features/test_case.feature', 'time_execution' => 3.0 }, + ]) + end + end + + context 'with a file path with nil time_execution' do + let(:test_files) do + [ + # full test file path with unknown execution time + { 'path' => 'features/test_case.feature', 'time_execution' => nil }, + # test example paths + { 'path' => 'features/test_case.feature:3', 'time_execution' => 2.2 }, + { 'path' => 'features/test_case.feature:12', 'time_execution' => 0.8 }, + ] + end + + it "returns the sum of test example paths' execution times" do + expect(subject).to eq([ + { 'path' => 'features/test_case.feature', 'time_execution' => 3.0 }, + ]) + end + end + end + end +end diff --git a/spec/knapsack_pro/tracker_spec.rb b/spec/knapsack_pro/tracker_spec.rb index 3925b6a5..b8c49ed8 100644 --- a/spec/knapsack_pro/tracker_spec.rb +++ b/spec/knapsack_pro/tracker_spec.rb @@ -155,6 +155,34 @@ end end + describe '#scheduled_test_path?' do + let(:test_paths) { ['features/a.feature', 'features/slow.feature:12'] } + + before do + tracker.set_prerun_tests(test_paths) + end + + it 'returns true for scheduled test file paths and test example paths' do + expect(tracker.scheduled_test_path?('features/a.feature')).to be true + expect(tracker.scheduled_test_path?('features/slow.feature:12')).to be true + expect(tracker.scheduled_test_path?('./features/slow.feature:12')).to be true + end + + it 'returns false for paths that are not scheduled' do + expect(tracker.scheduled_test_path?('features/slow.feature')).to be false + expect(tracker.scheduled_test_path?('features/slow.feature:5')).to be false + expect(tracker.scheduled_test_path?('features/b.feature')).to be false + end + + it '2nd tracker instance loads prerun tests from the disk' do + tracker2 = described_class.send(:new) + expect(tracker2.prerun_tests_loaded).to be false + expect(tracker2.scheduled_test_path?('features/slow.feature:12')).to be true + expect(tracker2.scheduled_test_path?('features/slow.feature')).to be false + expect(tracker2.prerun_tests_loaded).to be true + end + end + describe '#reset!' do let(:test_file_path) { 'a_spec.rb' } From bb8e507231ae72ddbd3d6954331bcc8bf6153d58 Mon Sep 17 00:00:00 2001 From: Alex Speller <1217+alexspeller@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:10:58 +0000 Subject: [PATCH 2/5] Cucumber & Queue Mode: experimental preload mode (fork per batch) Queue Mode for Cucumber shells out a brand new cucumber process for every batch pulled from the Queue, paying the full application boot cost (support code, e.g. a Rails app) each time. With Split by Test Examples enabled, batch counts grow substantially and that boot cost dominates the build time. With KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD=true the runner boots Cucumber once per CI node via a dry run that loads the support code, then executes each batch in a forked child process that reuses the preloaded Cucumber::Runtime through the public Cucumber::Cli::Main#execute!(existing_runtime) seam (the old spork/DRb integration point, still present in Cucumber 4 through 11). Details: - Cucumber::Runtime memoizes reports, formatters, and parsed features against the first configuration it ran with; the child resets those instance variables so they are rebuilt against the batch's configuration and event bus (otherwise results are published to the parent's event bus and the child exits 0 even on failures). The loaded support code is preserved. - The parent process loads the support code, so the adapter's at_exit hooks (save subset queue report, after-subset-queue hook) are skipped in the parent via KNAPSACK_PRO_CUCUMBER_PRELOAD_PARENT_PID; forked children run them as before, preserving per-batch reporting. - Children exit via Kernel.exit so at_exit reporting runs; the batch exit status is propagated to the runner loop unchanged. - POSIX only: raises a clear error when Process.fork is unavailable. Verified against Cucumber 9.2.1 and 11.1.1 with a simulated queue: single boot, correct per-scenario time tracking through forks, correct non-zero exit status propagation from failing batches. --- CHANGELOG.md | 1 + lib/knapsack_pro.rb | 1 + lib/knapsack_pro/adapters/cucumber_adapter.rb | 14 +++- lib/knapsack_pro/config/env.rb | 4 + .../cucumber/runtime_preloader.rb | 78 +++++++++++++++++++ .../runners/queue/cucumber_runner.rb | 50 ++++++++++++ .../cucumber_test_example_detector.rb | 3 +- .../adapters/cucumber_adapter_spec.rb | 19 +++++ spec/knapsack_pro/config/env_spec.rb | 14 ++++ .../cucumber/runtime_preloader_spec.rb | 19 +++++ 10 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 lib/knapsack_pro/cucumber/runtime_preloader.rb create mode 100644 spec/knapsack_pro/cucumber/runtime_preloader_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c1dfa8..8f612f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Unreleased * Cucumber: Add support for [Split by Test Examples](https://docs.knapsackpro.com/ruby/split-by-test-examples/) so slow feature files can be split by scenario (e.g., `features/a.feature:12`) across parallel CI nodes. Opt in with `KNAPSACK_PRO_CUCUMBER_SPLIT_BY_TEST_EXAMPLES=true` (requires Cucumber >= 4.0). +* Cucumber & Queue Mode: Add an experimental preload mode (`KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD=true`) that boots Cucumber (and the app loaded by the support code) once, then runs each batch of tests from the Queue in a forked child process instead of shelling out a brand new `cucumber` process per batch. This removes the per-batch application boot cost, which is significant for Rails apps and grows with the number of batches (e.g., when Split by Test Examples is enabled). POSIX only (requires `Process.fork`). ### 10.0.1 diff --git a/lib/knapsack_pro.rb b/lib/knapsack_pro.rb index 424285e0..c44f31cf 100644 --- a/lib/knapsack_pro.rb +++ b/lib/knapsack_pro.rb @@ -78,6 +78,7 @@ require_relative 'knapsack_pro/runners/queue/minitest_runner' require_relative 'knapsack_pro/test_case_detectors/rspec_test_example_detector' require_relative 'knapsack_pro/test_case_detectors/cucumber_test_example_detector' +require_relative 'knapsack_pro/cucumber/runtime_preloader' require_relative 'knapsack_pro/crypto/encryptor' require_relative 'knapsack_pro/crypto/branch_encryptor' require_relative 'knapsack_pro/crypto/decryptor' diff --git a/lib/knapsack_pro/adapters/cucumber_adapter.rb b/lib/knapsack_pro/adapters/cucumber_adapter.rb index 9596b582..f3044ce5 100644 --- a/lib/knapsack_pro/adapters/cucumber_adapter.rb +++ b/lib/knapsack_pro/adapters/cucumber_adapter.rb @@ -136,11 +136,21 @@ def bind_before_queue_hook def bind_after_queue_hook ::Kernel.at_exit do - KnapsackPro::Hooks::Queue.call_after_subset_queue - KnapsackPro::Report.save_subset_queue_to_file + # In the preload mode ( KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD ) the parent + # process loads the support code but never executes tests; only the forked + # child processes that ran a batch of tests should save a report. + unless KnapsackPro::Adapters::CucumberAdapter.preload_parent_process? + KnapsackPro::Hooks::Queue.call_after_subset_queue + KnapsackPro::Report.save_subset_queue_to_file + end end end + def self.preload_parent_process? + !!ENV['KNAPSACK_PRO_CUCUMBER_PRELOAD_PARENT_PID'] && + ENV['KNAPSACK_PRO_CUCUMBER_PRELOAD_PARENT_PID'] == Process.pid.to_s + end + private def Around(*tag_expressions, &proc) diff --git a/lib/knapsack_pro/config/env.rb b/lib/knapsack_pro/config/env.rb index e44eb99c..ad6d11c6 100644 --- a/lib/knapsack_pro/config/env.rb +++ b/lib/knapsack_pro/config/env.rb @@ -224,6 +224,10 @@ def cucumber_test_example_detector_prefix ENV.fetch('KNAPSACK_PRO_CUCUMBER_TEST_EXAMPLE_DETECTOR_PREFIX', 'bundle exec') end + def cucumber_queue_preload_enabled? + ENV['KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD'] == 'true' + end + def test_suite_token env_name = 'KNAPSACK_PRO_TEST_SUITE_TOKEN' ENV[env_name] || raise("Missing environment variable #{env_name}. You should set environment variable like #{env_name}_RSPEC (note there is suffix _RSPEC at the end). knapsack_pro gem will set #{env_name} based on #{env_name}_RSPEC value. If you use other test runner than RSpec then use proper suffix.") diff --git a/lib/knapsack_pro/cucumber/runtime_preloader.rb b/lib/knapsack_pro/cucumber/runtime_preloader.rb new file mode 100644 index 00000000..9e70cb54 --- /dev/null +++ b/lib/knapsack_pro/cucumber/runtime_preloader.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'shellwords' + +module KnapsackPro + module Cucumber + class RuntimePreloader + # Cucumber::Runtime memoizes these based on the configuration it was + # first run with. When the runtime is reused for another run via + # Cucumber::Cli::Main#execute!(existing_runtime), they must be reset so + # that reports, formatters, and parsed feature files are rebuilt against + # the new configuration and its event bus. Without this, test results + # are published to the previous run's event bus: formatters print + # nothing and the process exits 0 even when scenarios fail. + RUNTIME_MEMOIZED_IVARS = %i[ + @features + @filespecs + @report + @summary_report + @fail_fast_report + @publish_banner_printer + @formatters + ].freeze + + # Boots Cucumber once (a dry run loads the support code, e.g. + # features/support/env.rb which usually boots a Rails app) and returns + # the runtime so batches can be executed in forked child processes + # without paying the boot cost again. + def self.preload(test_dir, args) + unless Process.respond_to?(:fork) + raise "KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD requires an operating system that supports Process.fork (POSIX). Please disable the preload mode." + end + + require 'cucumber' + + # The parent process must not run the adapter's at_exit hooks + # (saving a subset queue report to disk / calling queue hooks); + # only the forked children that actually execute tests should. + # See lib/knapsack_pro/adapters/cucumber_adapter.rb + ENV['KNAPSACK_PRO_CUCUMBER_PRELOAD_PARENT_PID'] = Process.pid.to_s + + KnapsackPro.logger.info('Preloading Cucumber (loading support code once; batches will run in forked processes).') + + cli_args_without_formatters = KnapsackPro::Adapters::CucumberAdapter.remove_formatters(Shellwords.split(args || '')) + cli_args = cli_args_without_formatters + [ + '--dry-run', + '--format', 'progress', + '--out', File::NULL, + '--require', test_dir, + ] + + # Cucumber's option parser mutates the args array, so build the debug command upfront. + command = (['bundle exec cucumber'] + cli_args).join(' ') + + runtime = ::Cucumber::Runtime.new + exit_code = + begin + ::Cucumber::Cli::Main.new(cli_args).execute!(runtime) + 0 + rescue SystemExit => e + e.status + end + + unless exit_code.zero? + raise "Failed to preload Cucumber (exit code #{exit_code}). To reproduce the problem, run: #{command}" + end + + runtime + end + + def self.reset_runtime_memoization(runtime) + RUNTIME_MEMOIZED_IVARS.each do |ivar| + runtime.remove_instance_variable(ivar) if runtime.instance_variable_defined?(ivar) + end + end + end + end +end diff --git a/lib/knapsack_pro/runners/queue/cucumber_runner.rb b/lib/knapsack_pro/runners/queue/cucumber_runner.rb index 53d57765..9d2d9cc8 100644 --- a/lib/knapsack_pro/runners/queue/cucumber_runner.rb +++ b/lib/knapsack_pro/runners/queue/cucumber_runner.rb @@ -92,6 +92,10 @@ def self.run_tests(accumulator) private def self.cucumber_run(runner, test_file_paths, args) + if KnapsackPro::Config::Env.cucumber_queue_preload_enabled? + return preloaded_cucumber_run(runner, test_file_paths, args) + end + stringify_test_file_paths = KnapsackPro::TestFilePresenter.stringify_paths(test_file_paths) cmd = [ @@ -115,6 +119,52 @@ def self.cucumber_run(runner, test_file_paths, args) child_status.exitstatus end + + # Boots Cucumber (including the support code, e.g. a Rails app loaded by + # features/support/env.rb) once in this process, then runs each batch of + # tests in a forked child process that reuses the preloaded runtime. + # This avoids paying the application boot cost for every batch pulled + # from the Queue. Requires an OS that supports Process.fork (POSIX). + def self.preloaded_cucumber_run(runner, test_file_paths, args) + runtime = preloaded_cucumber_runtime(runner, args) + + child_pid = Kernel.fork do + KnapsackPro::Cucumber::RuntimePreloader.reset_runtime_memoization(runtime) + + cli_args = Shellwords.split(args || '') + ['--require', runner.test_dir] + test_file_paths + exit_code = + begin + ::Cucumber::Cli::Main.new(cli_args).execute!(runtime) + 0 + rescue SystemExit => e + e.status + end + # Kernel.exit (not exit!) so that at_exit hooks run in the child: + # KnapsackPro::Hooks::Queue.call_after_subset_queue + # KnapsackPro::Report.save_subset_queue_to_file + # which are registered in lib/knapsack_pro/adapters/cucumber_adapter.rb + Kernel.exit(exit_code) + end + _pid, status = Process.waitpid2(child_pid) + + # it must be set here so when we run the next batch we won't run again: + # KnapsackPro::Hooks::Queue.call_before_queue + # which is defined in lib/knapsack_pro/adapters/cucumber_adapter.rb + ENV['KNAPSACK_PRO_BEFORE_QUEUE_HOOK_CALLED'] = 'true' + + unless status.exited? + raise "Cucumber process execution failed. It's likely that your CI server has exceeded"\ + " its available memory. Please try changing CI config or retrying the CI build.\n"\ + "Failed test files: #{test_file_paths.inspect}\n"\ + "Process status: #{status.inspect}" + end + + status.exitstatus + end + + def self.preloaded_cucumber_runtime(runner, args) + @preloaded_cucumber_runtime ||= KnapsackPro::Cucumber::RuntimePreloader.preload(runner.test_dir, args) + end end end end diff --git a/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb b/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb index 32548a43..4ad766e5 100644 --- a/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb +++ b/lib/knapsack_pro/test_case_detectors/cucumber_test_example_detector.rb @@ -23,10 +23,11 @@ def dry_run_to_file(cucumber_args) '--out', report_path, '--require', test_dir, ] + KnapsackPro::TestFilePresenter.paths(slow_test_files) + # Cucumber's option parser mutates the args array, so build the debug command upfront. + command = (['bundle exec cucumber'] + cli_args).join(' ') exit_code = dry_run(cli_args) return if exit_code.zero? - command = (['bundle exec cucumber'] + cli_args).join(' ') KnapsackPro.logger.error("Failed to calculate Split by Test Examples: #{command}") exit exit_code end diff --git a/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb b/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb index b57b6f15..c2dc48cd 100644 --- a/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb +++ b/spec/knapsack_pro/adapters/cucumber_adapter_spec.rb @@ -268,6 +268,25 @@ end end + describe '.preload_parent_process?' do + subject { described_class.preload_parent_process? } + + context 'when the preload parent PID matches the current process' do + before { stub_const("ENV", { 'KNAPSACK_PRO_CUCUMBER_PRELOAD_PARENT_PID' => Process.pid.to_s }) } + it { should be true } + end + + context 'when the preload parent PID is a different process (a forked child)' do + before { stub_const("ENV", { 'KNAPSACK_PRO_CUCUMBER_PRELOAD_PARENT_PID' => '0' }) } + it { should be false } + end + + context 'when the preload mode is not used' do + before { stub_const("ENV", {}) } + it { should be false } + end + end + describe 'bind methods' do describe '#bind_time_tracker' do let(:file) { 'features/a.feature' } diff --git a/spec/knapsack_pro/config/env_spec.rb b/spec/knapsack_pro/config/env_spec.rb index 59af569e..ce0c1ced 100644 --- a/spec/knapsack_pro/config/env_spec.rb +++ b/spec/knapsack_pro/config/env_spec.rb @@ -1003,6 +1003,20 @@ end end + describe '.cucumber_queue_preload_enabled?' do + subject { described_class.cucumber_queue_preload_enabled? } + + context 'when ENV exists' do + before { stub_const("ENV", { 'KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD' => 'true' }) } + it { should be true } + end + + context "when ENV doesn't exist" do + before { stub_const("ENV", {}) } + it { should be false } + end + end + describe '.cucumber_test_example_detector_prefix' do subject { described_class.cucumber_test_example_detector_prefix } diff --git a/spec/knapsack_pro/cucumber/runtime_preloader_spec.rb b/spec/knapsack_pro/cucumber/runtime_preloader_spec.rb new file mode 100644 index 00000000..7b7c6f26 --- /dev/null +++ b/spec/knapsack_pro/cucumber/runtime_preloader_spec.rb @@ -0,0 +1,19 @@ +describe KnapsackPro::Cucumber::RuntimePreloader do + describe '.reset_runtime_memoization' do + it 'removes the memoized instance variables so a reused runtime rebuilds reports, formatters, and parsed features against the new configuration' do + runtime = Object.new + runtime.instance_variable_set(:@features, double) + runtime.instance_variable_set(:@report, double) + runtime.instance_variable_set(:@summary_report, double) + runtime.instance_variable_set(:@support_code, support_code = double) + + described_class.reset_runtime_memoization(runtime) + + expect(runtime.instance_variable_defined?(:@features)).to be false + expect(runtime.instance_variable_defined?(:@report)).to be false + expect(runtime.instance_variable_defined?(:@summary_report)).to be false + # the loaded support code (e.g. the booted Rails app) must be preserved + expect(runtime.instance_variable_get(:@support_code)).to eq support_code + end + end +end From 137b3cf2464db1362182d6bf2544bface5a28ea1 Mon Sep 17 00:00:00 2001 From: Alex Speller <1217+alexspeller@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:38:10 +0000 Subject: [PATCH 3/5] Fix preload dry run conflicting with profile formatters writing to /dev/null Cucumber expands cucumber.yml profiles into the CLI args before validating that no two formatters write to the same stream. When a user profile contains e.g. --format SomeFormatter --out /dev/null, the preload dry run's own --out /dev/null triggered: All but one formatter must use --out, only one can print to each stream Write the preload dry run progress output to a unique file under .knapsack_pro instead. --- lib/knapsack_pro/cucumber/runtime_preloader.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/knapsack_pro/cucumber/runtime_preloader.rb b/lib/knapsack_pro/cucumber/runtime_preloader.rb index 9e70cb54..53f5f046 100644 --- a/lib/knapsack_pro/cucumber/runtime_preloader.rb +++ b/lib/knapsack_pro/cucumber/runtime_preloader.rb @@ -44,8 +44,11 @@ def self.preload(test_dir, args) cli_args_without_formatters = KnapsackPro::Adapters::CucumberAdapter.remove_formatters(Shellwords.split(args || '')) cli_args = cli_args_without_formatters + [ '--dry-run', + # A unique --out path: pointing two formatters at the same stream is an + # error in Cucumber, and a profile from cucumber.yml may already use + # --out /dev/null (profiles are expanded before the stream conflict check). '--format', 'progress', - '--out', File::NULL, + '--out', dry_run_progress_report_path, '--require', test_dir, ] @@ -68,6 +71,13 @@ def self.preload(test_dir, args) runtime end + def self.dry_run_progress_report_path + KnapsackPro::Config::TempFiles.ensure_temp_directory_exists! + dir = "#{KnapsackPro::Config::TempFiles::TEMP_DIRECTORY_PATH}/cucumber_queue_preload" + FileUtils.mkdir_p(dir) + "#{dir}/dry_run_progress_node_#{KnapsackPro::Config::Env.ci_node_index}.txt" + end + def self.reset_runtime_memoization(runtime) RUNTIME_MEMOIZED_IVARS.each do |ivar| runtime.remove_instance_variable(ivar) if runtime.instance_variable_defined?(ivar) From 5f99f10f69a366be758ee84284371581f8ed5049 Mon Sep 17 00:00:00 2001 From: Alex Speller <1217+alexspeller@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:11:56 +0000 Subject: [PATCH 4/5] Preload mode: fork hygiene and fast preload Real-world trial findings: - The preload dry run covered the whole test suite, which can take minutes (step matching runs for every step even in a dry run; ~0.6s per scenario on a large Rails app). The dry run only exists to load the support code, so limit it to a single sample test file path (the first path of the first batch). - Database connections opened while booting the app were inherited by every forked child; concurrent use of an inherited socket corrupts the connection and hangs or fails queries (observed as Capybara rendering error pages instead of the app). Clear ActiveRecord connection pools in the parent after preloading so the parent and each child lazily check out fresh connections. - Add a KnapsackPro::Hooks::Queue.after_preload_fork hook, called in each forked child right after the fork, so apps can re-establish resources that do not survive a fork (e.g. SemanticLogger.reopen for log appender threads, or reconnecting Redis clients). --- .../cucumber/runtime_preloader.rb | 22 ++++++++++++++++++- lib/knapsack_pro/hooks/queue.rb | 21 +++++++++++++++++- .../runners/queue/cucumber_runner.rb | 7 +++--- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lib/knapsack_pro/cucumber/runtime_preloader.rb b/lib/knapsack_pro/cucumber/runtime_preloader.rb index 53f5f046..854e172f 100644 --- a/lib/knapsack_pro/cucumber/runtime_preloader.rb +++ b/lib/knapsack_pro/cucumber/runtime_preloader.rb @@ -26,7 +26,11 @@ class RuntimePreloader # features/support/env.rb which usually boots a Rails app) and returns # the runtime so batches can be executed in forked child processes # without paying the boot cost again. - def self.preload(test_dir, args) + # + # sample_test_file_path limits the dry run to a single test file: the + # dry run only exists to load the support code, and dry-running a whole + # test suite can take minutes (e.g., step matching on every step). + def self.preload(test_dir, args, sample_test_file_path) unless Process.respond_to?(:fork) raise "KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD requires an operating system that supports Process.fork (POSIX). Please disable the preload mode." end @@ -50,6 +54,7 @@ def self.preload(test_dir, args) '--format', 'progress', '--out', dry_run_progress_report_path, '--require', test_dir, + sample_test_file_path, ] # Cucumber's option parser mutates the args array, so build the debug command upfront. @@ -68,9 +73,24 @@ def self.preload(test_dir, args) raise "Failed to preload Cucumber (exit code #{exit_code}). To reproduce the problem, run: #{command}" end + disconnect_active_record_in_parent! + runtime end + # Database connections opened while booting the app must not be shared + # with the forked children (concurrent use of an inherited socket + # corrupts the connection protocol and hangs or fails queries). Clearing + # the pools here means both the parent and each forked child lazily + # check out fresh connections when they first need one. + def self.disconnect_active_record_in_parent! + return unless defined?(::ActiveRecord::Base) + + ::ActiveRecord::Base.connection_handler.clear_all_connections! + rescue StandardError => e + KnapsackPro.logger.warn("Could not clear ActiveRecord connections after preloading Cucumber: #{e.class}: #{e.message}") + end + def self.dry_run_progress_report_path KnapsackPro::Config::TempFiles.ensure_temp_directory_exists! dir = "#{KnapsackPro::Config::TempFiles::TEMP_DIRECTORY_PATH}/cucumber_queue_preload" diff --git a/lib/knapsack_pro/hooks/queue.rb b/lib/knapsack_pro/hooks/queue.rb index d44c097d..d18f9e62 100644 --- a/lib/knapsack_pro/hooks/queue.rb +++ b/lib/knapsack_pro/hooks/queue.rb @@ -7,7 +7,8 @@ class << self attr_reader :before_queue_store, :before_subset_queue_store, :after_subset_queue_store, - :after_queue_store + :after_queue_store, + :after_preload_fork_store def reset_before_queue @before_queue_store = nil @@ -25,6 +26,10 @@ def reset_after_queue @after_queue_store = nil end + def reset_after_preload_fork + @after_preload_fork_store = nil + end + def before_queue(&block) @before_queue_store ||= [] @before_queue_store << block @@ -45,6 +50,20 @@ def after_queue(&block) @after_queue_store << block end + # Called in a forked child process right after the fork when the Cucumber + # preload mode ( KNAPSACK_PRO_CUCUMBER_QUEUE_PRELOAD ) is enabled. + # Use it to re-establish resources that do not survive a fork, + # e.g., reopen log appenders or reconnect clients holding sockets. + def after_preload_fork(&block) + @after_preload_fork_store ||= [] + @after_preload_fork_store << block + end + + def call_after_preload_fork + return unless after_preload_fork_store + after_preload_fork_store.each(&:call) + end + def call_before_queue return unless before_queue_store before_queue_store.each do |block| diff --git a/lib/knapsack_pro/runners/queue/cucumber_runner.rb b/lib/knapsack_pro/runners/queue/cucumber_runner.rb index 9d2d9cc8..be1ec481 100644 --- a/lib/knapsack_pro/runners/queue/cucumber_runner.rb +++ b/lib/knapsack_pro/runners/queue/cucumber_runner.rb @@ -126,9 +126,10 @@ def self.cucumber_run(runner, test_file_paths, args) # This avoids paying the application boot cost for every batch pulled # from the Queue. Requires an OS that supports Process.fork (POSIX). def self.preloaded_cucumber_run(runner, test_file_paths, args) - runtime = preloaded_cucumber_runtime(runner, args) + runtime = preloaded_cucumber_runtime(runner, args, test_file_paths.first) child_pid = Kernel.fork do + KnapsackPro::Hooks::Queue.call_after_preload_fork KnapsackPro::Cucumber::RuntimePreloader.reset_runtime_memoization(runtime) cli_args = Shellwords.split(args || '') + ['--require', runner.test_dir] + test_file_paths @@ -162,8 +163,8 @@ def self.preloaded_cucumber_run(runner, test_file_paths, args) status.exitstatus end - def self.preloaded_cucumber_runtime(runner, args) - @preloaded_cucumber_runtime ||= KnapsackPro::Cucumber::RuntimePreloader.preload(runner.test_dir, args) + def self.preloaded_cucumber_runtime(runner, args, sample_test_file_path) + @preloaded_cucumber_runtime ||= KnapsackPro::Cucumber::RuntimePreloader.preload(runner.test_dir, args, sample_test_file_path) end end end From 8d38b227cfff442838bad7e34d6178661725864e Mon Sep 17 00:00:00 2001 From: Alex Speller <1217+alexspeller@users.noreply.github.com> Date: Fri, 31 Jul 2026 03:03:06 +0000 Subject: [PATCH 5/5] Preload mode: keep the parent process inert (no Capybara server, no profiles) Second round of real-world trial fixes, found by reproducing the failure locally with per-thread SQL instrumentation: - The preload dry run instantiated formatters from cucumber.yml profiles. A formatter touching Capybara (e.g. one that captures screenshots) boots the Capybara server - and a browser - in the parent process. Forked children then inherit Capybara's session pool and app->port registry, find the parent's server responsive, and send their requests to a server running in the WRONG PROCESS: it cannot see the child's open database transactions (observed as every scenario failing at login because the test-created tenant was invisible), and the parent's lingering browser/server kept the CI step's output pipes open, hanging the job until timeout. The preload dry run now strips -p/--profile and runs with --no-profile: it only exists to load the support code. - Belt and braces for the same class of bug: forked children now clear Capybara's session pool and Capybara::Server.ports before running, so a child always boots its own server even if something did boot one in the parent. - Clear ActiveRecord connection leases (not just pools) in the parent after preloading: leases are keyed by thread identity, which survives a fork, so a child's first checkout could return the parent's connection object and reconnect it in place (undefined behavior on a forked libpq handle). --- .../cucumber/runtime_preloader.rb | 59 ++++++++++++++++++- .../runners/queue/cucumber_runner.rb | 2 +- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/knapsack_pro/cucumber/runtime_preloader.rb b/lib/knapsack_pro/cucumber/runtime_preloader.rb index 854e172f..4c607ab0 100644 --- a/lib/knapsack_pro/cucumber/runtime_preloader.rb +++ b/lib/knapsack_pro/cucumber/runtime_preloader.rb @@ -46,7 +46,14 @@ def self.preload(test_dir, args, sample_test_file_path) KnapsackPro.logger.info('Preloading Cucumber (loading support code once; batches will run in forked processes).') cli_args_without_formatters = KnapsackPro::Adapters::CucumberAdapter.remove_formatters(Shellwords.split(args || '')) - cli_args = cli_args_without_formatters + [ + # The preload dry run only exists to load the support code. Profiles from + # cucumber.yml can register formatters, and a formatter that touches + # Capybara would boot the Capybara server (and a browser) in this parent + # process; forked children would then reuse the parent's server via + # Capybara's inherited session/port registries and run their requests in + # the wrong process. + cli_args = remove_profiles(cli_args_without_formatters) + [ + '--no-profile', '--dry-run', # A unique --out path: pointing two formatters at the same stream is an # error in Cucumber, and a profile from cucumber.yml may already use @@ -86,7 +93,20 @@ def self.preload(test_dir, args, sample_test_file_path) def self.disconnect_active_record_in_parent! return unless defined?(::ActiveRecord::Base) - ::ActiveRecord::Base.connection_handler.clear_all_connections! + handler = ::ActiveRecord::Base.connection_handler + + # Connection leases are keyed by thread identity, which survives a fork. + # Without releasing the lease acquired while booting the app, a forked + # child's first `pool.checkout`/`pin_connection!` returns the parent's + # connection object and reconnects it in place (undefined behavior on a + # forked libpq handle; observed as a segfault in the pg gem). + if handler.respond_to?(:each_connection_pool) + handler.each_connection_pool do |pool| + pool.release_connection if pool.respond_to?(:release_connection) + end + end + + handler.clear_all_connections! rescue StandardError => e KnapsackPro.logger.warn("Could not clear ActiveRecord connections after preloading Cucumber: #{e.class}: #{e.message}") end @@ -98,11 +118,46 @@ def self.dry_run_progress_report_path "#{dir}/dry_run_progress_node_#{KnapsackPro::Config::Env.ci_node_index}.txt" end + def self.remove_profiles(cli_args) + profile_options = ['-p', '--profile'] + cli_args.dup.each_with_index do |arg, index| + if profile_options.include?(arg) + cli_args[index] = nil + cli_args[index + 1] = nil + elsif arg == '--no-profile' || arg == '-P' + cli_args[index] = nil + end + end + cli_args.compact + end + + # Called in the forked child process before running a batch of tests. + def self.reset_forked_child_state(runtime) + reset_runtime_memoization(runtime) + reset_capybara! + end + def self.reset_runtime_memoization(runtime) RUNTIME_MEMOIZED_IVARS.each do |ivar| runtime.remove_instance_variable(ivar) if runtime.instance_variable_defined?(ivar) end end + + # Capybara's session registry and app->port registry are inherited by the + # fork. If a Capybara server was booted in the parent process, a child + # reusing these registries would find the parent's server responsive and + # send its requests to the wrong process (which cannot see, e.g., the + # child's open database transactions). Clearing them makes each child + # boot its own server on first use. + def self.reset_capybara! + return unless defined?(::Capybara) + + ::Capybara.send(:session_pool).clear if ::Capybara.respond_to?(:session_pool, true) + + if defined?(::Capybara::Server) && ::Capybara::Server.respond_to?(:ports) + ::Capybara::Server.ports.clear + end + end end end end diff --git a/lib/knapsack_pro/runners/queue/cucumber_runner.rb b/lib/knapsack_pro/runners/queue/cucumber_runner.rb index be1ec481..e501f78f 100644 --- a/lib/knapsack_pro/runners/queue/cucumber_runner.rb +++ b/lib/knapsack_pro/runners/queue/cucumber_runner.rb @@ -130,7 +130,7 @@ def self.preloaded_cucumber_run(runner, test_file_paths, args) child_pid = Kernel.fork do KnapsackPro::Hooks::Queue.call_after_preload_fork - KnapsackPro::Cucumber::RuntimePreloader.reset_runtime_memoization(runtime) + KnapsackPro::Cucumber::RuntimePreloader.reset_forked_child_state(runtime) cli_args = Shellwords.split(args || '') + ['--require', runner.test_dir] + test_file_paths exit_code =