diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f432028..882f9e24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Change Log +### 3.4.1 + +* Improve the RSpec Queue Mode runner log output (add seed) + + https://github.com/KnapsackPro/knapsack_pro-ruby/pull/178 + +https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v3.4.0...v3.4.1 + ### 3.4.0 * Update documentation and code because the encryption feature does not work with the RSpec split by examples feature diff --git a/lib/knapsack_pro/adapters/rspec_adapter.rb b/lib/knapsack_pro/adapters/rspec_adapter.rb index e624003a..29afeba7 100644 --- a/lib/knapsack_pro/adapters/rspec_adapter.rb +++ b/lib/knapsack_pro/adapters/rspec_adapter.rb @@ -12,16 +12,15 @@ def self.ensure_no_tag_option_when_rspec_split_by_test_examples_enabled!(cli_arg end def self.has_tag_option?(cli_args) - # use start_with? because user can define tag option in a few ways: - # -t mytag - # -tmytag - # --tag mytag - # --tag=mytag - cli_args.any? { |arg| arg.start_with?('-t') || arg.start_with?('--tag') } + !!parsed_options(cli_args)&.[](:inclusion_filter) end def self.has_format_option?(cli_args) - cli_args.any? { |arg| arg.start_with?('-f') || arg.start_with?('--format') } + !!parsed_options(cli_args)&.[](:formatters) + end + + def self.order_option(cli_args) + parsed_options(cli_args)&.[](:order) end def self.test_path(example) @@ -107,6 +106,12 @@ def bind_before_queue_hook def self.rspec_configuration ::RSpec.configuration end + + def self.parsed_options(cli_args) + ::RSpec::Core::Parser.parse(cli_args) + rescue SystemExit + nil + end end # This is added to provide backwards compatibility diff --git a/lib/knapsack_pro/runners/queue/rspec_runner.rb b/lib/knapsack_pro/runners/queue/rspec_runner.rb index b7715df7..e91e40c6 100644 --- a/lib/knapsack_pro/runners/queue/rspec_runner.rb +++ b/lib/knapsack_pro/runners/queue/rspec_runner.rb @@ -2,6 +2,8 @@ module KnapsackPro module Runners module Queue class RSpecRunner < BaseRunner + @@used_seed = nil + def self.run(args) require 'rspec/core' require_relative '../../formatters/rspec_queue_summary_formatter' @@ -11,7 +13,6 @@ def self.run(args) ENV['KNAPSACK_PRO_QUEUE_RECORDING_ENABLED'] = 'true' ENV['KNAPSACK_PRO_QUEUE_ID'] = KnapsackPro::Config::EnvGenerator.set_queue_id - adapter_class = KnapsackPro::Adapters::RSpecAdapter KnapsackPro::Config::Env.set_test_runner_adapter(adapter_class) runner = new(adapter_class) @@ -61,6 +62,8 @@ def self.run_tests(accumulator) KnapsackPro::Formatters::RSpecQueueSummaryFormatter.print_summary KnapsackPro::Formatters::RSpecQueueProfileFormatterExtension.print_summary + args += ['--seed', @@used_seed] if @@used_seed + log_rspec_command(args, all_test_file_paths, :end_of_queue) end @@ -82,12 +85,15 @@ def self.run_tests(accumulator) all_test_file_paths += test_file_paths cli_args = args + test_file_paths - log_rspec_command(args, test_file_paths, :subset_queue) - options = ::RSpec::Core::ConfigurationOptions.new(cli_args) - exit_code = ::RSpec::Core::Runner.new(options).run($stderr, $stdout) + rspec_runner = ::RSpec::Core::Runner.new(options) + + exit_code = rspec_runner.run($stderr, $stdout) exitstatus = exit_code if exit_code != 0 + printable_args = args_with_seed_option_added_when_viable(args, rspec_runner) + log_rspec_command(printable_args, test_file_paths, :subset_queue) + rspec_clear_examples KnapsackPro::Hooks::Queue.call_after_subset_queue @@ -107,19 +113,22 @@ def self.run_tests(accumulator) private + def self.adapter_class + KnapsackPro::Adapters::RSpecAdapter + end + def self.log_rspec_command(cli_args, test_file_paths, type) case type when :subset_queue - KnapsackPro.logger.info("To retry the last batch of tests fetched from the API Queue, please run the following command on your machine. (If you use the `-- order random` option, remember to add correct `--seed 123` that you can find at the end of the RSpec output.)") + KnapsackPro.logger.info("To retry the last batch of tests fetched from the API Queue, please run the following command on your machine:") when :end_of_queue KnapsackPro.logger.info("To retry all the tests assigned to this CI node, please run the following command on your machine:") end - stringify_cli_args = cli_args.join(' ') - stringify_cli_args.slice!("--format #{KnapsackPro::Formatters::RSpecQueueSummaryFormatter}") + stringified_cli_args = cli_args.join(' ').sub(" --format #{KnapsackPro::Formatters::RSpecQueueSummaryFormatter}", '') KnapsackPro.logger.info( - "bundle exec rspec #{stringify_cli_args} " + + "bundle exec rspec #{stringified_cli_args} " + KnapsackPro::TestFilePresenter.stringify_paths(test_file_paths) ) end @@ -151,6 +160,24 @@ def self.rspec_clear_examples ::RSpec.configuration.reset_filters end end + + def self.args_with_seed_option_added_when_viable(args, rspec_runner) + order_option = adapter_class.order_option(args) + + if order_option + # Don't add the seed option for order other than random, e.g. `defined` + return args unless order_option.include?('rand') + # Don't add the seed option if the seed is already set in args, e.g. `rand:12345` + return args if order_option.to_s.split(':')[1] + end + + # Don't add the seed option if the seed was not used (i.e. a different order is being used, e.g. `defined`) + return args unless rspec_runner.configuration.seed_used? + + @@used_seed = rspec_runner.configuration.seed.to_s + + args + ['--seed', @@used_seed] + end end end end diff --git a/lib/knapsack_pro/version.rb b/lib/knapsack_pro/version.rb index fa066f74..c1b65f18 100644 --- a/lib/knapsack_pro/version.rb +++ b/lib/knapsack_pro/version.rb @@ -1,3 +1,3 @@ module KnapsackPro - VERSION = '3.4.0' + VERSION = '3.4.1' end diff --git a/spec/knapsack_pro/adapters/rspec_adapter_spec.rb b/spec/knapsack_pro/adapters/rspec_adapter_spec.rb index a4f3730c..86ba31f5 100644 --- a/spec/knapsack_pro/adapters/rspec_adapter_spec.rb +++ b/spec/knapsack_pro/adapters/rspec_adapter_spec.rb @@ -110,6 +110,58 @@ end end + describe '.order_option' do + subject { described_class.order_option(cli_args) } + + context "when order is 'defined'" do + let(:cli_args) { ['--order', 'defined'] } + + it { expect(subject).to eq 'defined' } + end + + context "when order is 'recently-modified'" do + let(:cli_args) { ['--order', 'recently-modified'] } + + it { expect(subject).to eq 'recently-modified' } + end + + context "when order is 'rand'" do + let(:cli_args) { ['--order', 'rand'] } + + it { expect(subject).to eq 'rand' } + + context 'with the seed' do + let(:cli_args) { ['--order', 'rand:123456'] } + + it { expect(subject).to eq 'rand:123456' } + end + end + + context "when order is 'random'" do + let(:cli_args) { ['--order', 'random'] } + + it { expect(subject).to eq 'random' } + + context 'with the seed' do + let(:cli_args) { ['--order', 'random:123456'] } + + it { expect(subject).to eq 'random:123456' } + end + end + + context 'when some custom order is specified' do + let(:cli_args) { ['--order', 'some-custom-order'] } + + it { expect(subject).to eq 'some-custom-order' } + end + + context "when the seed is given with the --seed command" do + let(:cli_args) { ['--seed', '123456'] } + + it { expect(subject).to eq 'rand:123456' } + end + end + describe '.test_path' do let(:example_group) do { diff --git a/spec/knapsack_pro/runners/queue/rspec_runner_spec.rb b/spec/knapsack_pro/runners/queue/rspec_runner_spec.rb index f30899a0..18658cdb 100644 --- a/spec/knapsack_pro/runners/queue/rspec_runner_spec.rb +++ b/spec/knapsack_pro/runners/queue/rspec_runner_spec.rb @@ -177,7 +177,7 @@ describe '.run_tests' do let(:runner) { instance_double(described_class) } let(:can_initialize_queue) { double(:can_initialize_queue) } - let(:args) { ['--example-arg', 'example-value', '--default-path', 'fake-test-dir'] } + let(:args) { ['--no-color', '--default-path', 'fake-test-dir'] } let(:exitstatus) { double } let(:all_test_file_paths) { [] } let(:accumulator) do @@ -198,6 +198,8 @@ context 'when test files exist' do let(:test_file_paths) { ['a_spec.rb', 'b_spec.rb'] } + let(:logger) { double } + let(:rspec_seed) { 7771 } before do subset_queue_id = 'fake-subset-queue-id' @@ -212,7 +214,7 @@ options = double expect(RSpec::Core::ConfigurationOptions).to receive(:new).with([ - '--example-arg', 'example-value', + '--no-color', '--default-path', 'fake-test-dir', 'a_spec.rb', 'b_spec.rb', ]).and_return(options) @@ -226,6 +228,16 @@ expect(KnapsackPro::Hooks::Queue).to receive(:call_after_subset_queue) expect(KnapsackPro::Report).to receive(:save_subset_queue_to_file) + + configuration = double + expect(rspec_core_runner).to receive(:configuration).twice.and_return(configuration) + expect(configuration).to receive(:seed_used?).and_return(true) + expect(configuration).to receive(:seed).and_return(rspec_seed) + + expect(KnapsackPro).to receive(:logger).twice.and_return(logger) + expect(logger).to receive(:info) + .with("To retry the last batch of tests fetched from the API Queue, please run the following command on your machine:") + expect(logger).to receive(:info).with(/#{args.join(' ')} --seed #{rspec_seed}/) end context 'when exit code is zero' do @@ -264,8 +276,13 @@ context 'when all_test_file_paths exist' do let(:all_test_file_paths) { ['a_spec.rb'] } + let(:logger) { double } + + before do + described_class.class_variable_set(:@@used_seed, used_seed) + + expect(KnapsackPro).to receive(:logger).twice.and_return(logger) - it do expect(KnapsackPro::Adapters::RSpecAdapter).to receive(:verify_bind_method_called) expect(KnapsackPro::Formatters::RSpecQueueSummaryFormatter).to receive(:print_summary) @@ -274,10 +291,33 @@ expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue) expect(KnapsackPro::Report).to receive(:save_node_queue_to_api) - expect(subject).to eq({ - status: :completed, - exitstatus: exitstatus, - }) + expect(logger).to receive(:info) + .with('To retry all the tests assigned to this CI node, please run the following command on your machine:') + expect(logger).to receive(:info).with(logged_rspec_command_matcher) + end + + context 'when @@used_seed has been set' do + let(:used_seed) { '8333' } + let(:logged_rspec_command_matcher) { /#{args.join(' ')} --seed #{used_seed} \"a_spec.rb"/ } + + it do + expect(subject).to eq({ + status: :completed, + exitstatus: exitstatus, + }) + end + end + + context 'when @@used_seed has not been set' do + let(:used_seed) { nil } + let(:logged_rspec_command_matcher) { /#{args.join(' ')} \"a_spec.rb"/ } + + it do + expect(subject).to eq({ + status: :completed, + exitstatus: exitstatus, + }) + end end end @@ -287,6 +327,7 @@ it do expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue) expect(KnapsackPro::Report).to receive(:save_node_queue_to_api) + expect(KnapsackPro).to_not receive(:logger) expect(subject).to eq({ status: :completed,