From 0709dcb4c535940501b2fd3eec3880431a34c74f Mon Sep 17 00:00:00 2001 From: Dana Sherson Date: Thu, 13 Aug 2020 14:54:12 +1000 Subject: [PATCH] Add error-exit-code to differentiate from failures it can be helpful to know if RSpec fails because of an example or if it errors out because it couldn't load a spec file, or there was an issue in the a before(:suite) hook or etc i've named the setting error-exit-code, and tried to add it to all the relevant places, but i don't know rspec codebase well so there was some guessing. in particular i'm not sure what bisect should do. i have it fall back to the failure-exit-code before defaulting to 1 so there's no changes if people don't opt in to the setting. the specific use of this is our CI automatically retries failures using the persistence file, and assumes if they pass on that retry they were flaky. however, the persistence file isn't written to when there's an error outside of examples, so this could mean falsely passing builds. by checking for a different exit code, and then not running the retry we can avoid this issue. --- .../configuration/error_exit_code.feature | 52 +++++++++++++++++++ .../configuration/failure_exit_code.feature | 26 ++++++++++ lib/rspec/core/configuration.rb | 6 +++ lib/rspec/core/drb.rb | 7 +++ lib/rspec/core/invocations.rb | 2 +- lib/rspec/core/option_parser.rb | 5 ++ lib/rspec/core/runner.rb | 16 ++++-- .../integration/spec_file_load_errors_spec.rb | 8 +-- spec/integration/suite_hooks_errors_spec.rb | 6 ++- spec/rspec/core/configuration_options_spec.rb | 12 +++++ spec/rspec/core/configuration_spec.rb | 22 ++++++++ spec/rspec/core/runner_spec.rb | 46 +++++++++++++++- 12 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 features/configuration/error_exit_code.feature diff --git a/features/configuration/error_exit_code.feature b/features/configuration/error_exit_code.feature new file mode 100644 index 0000000000..f64bcdef90 --- /dev/null +++ b/features/configuration/error_exit_code.feature @@ -0,0 +1,52 @@ +Feature: error exit code + + Use the `error_exit_code` option to set a custom exit code when RSpec fails outside an example. + + ```ruby + RSpec.configure { |c| c.error_exit_code = 42 } + ``` + + Background: + Given a file named "spec/spec_helper.rb" with: + """ruby + RSpec.configure { |c| c.error_exit_code = 42 } + """ + + Scenario: A erroring spec with the default exit code + Given a file named "spec/typo_spec.rb" with: + """ruby + RSpec.escribe "something" do # intentional typo + it "works" do + true + end + end + """ + When I run `rspec spec/typo_spec.rb` + Then the exit status should be 1 + + Scenario: A erroring spec with a custom exit code + Given a file named "spec/typo_spec.rb" with: + """ruby + require 'spec_helper' + RSpec.escribe "something" do # intentional typo + it "works" do + true + end + end + """ + When I run `rspec spec/typo_spec.rb` + And the exit status should be 42 + + + Scenario: Success running specs spec with a custom error exit code defined + Given a file named "spec/example_spec.rb" with: + """ruby + require 'spec_helper' + RSpec.describe "something" do + it "works" do + true + end + end + """ + When I run `rspec spec/example_spec.rb` + Then the exit status should be 0 diff --git a/features/configuration/failure_exit_code.feature b/features/configuration/failure_exit_code.feature index a61fcdd049..d71f203576 100644 --- a/features/configuration/failure_exit_code.feature +++ b/features/configuration/failure_exit_code.feature @@ -37,6 +37,32 @@ Feature: failure exit code When I run `rspec spec/example_spec.rb` Then the exit status should be 42 + Scenario: An error running specs spec with a custom exit code + Given a file named "spec/typo_spec.rb" with: + """ruby + require 'spec_helper' + RSpec.escribe "something" do # intentional typo + it "works" do + true + end + end + """ + When I run `rspec spec/typo_spec.rb` + Then the exit status should be 42 + + Scenario: Success running specs spec with a custom exit code defined + Given a file named "spec/example_spec.rb" with: + """ruby + require 'spec_helper' + RSpec.describe "something" do + it "works" do + true + end + end + """ + When I run `rspec spec/example_spec.rb` + Then the exit status should be 0 + Scenario: Exit with the default exit code when an `at_exit` hook is added upstream Given a file named "exit_at_spec.rb" with: """ruby diff --git a/lib/rspec/core/configuration.rb b/lib/rspec/core/configuration.rb index 120a5656f6..bb38a8de4f 100644 --- a/lib/rspec/core/configuration.rb +++ b/lib/rspec/core/configuration.rb @@ -242,6 +242,11 @@ def fail_fast=(value) # @return [Integer] add_setting :failure_exit_code + # @macro add_setting + # The exit code to return if there are any errors outside examples (default: failure_exit_code) + # @return [Integer] + add_setting :error_exit_code + # @macro add_setting # Whether or not to fail when there are no RSpec examples (default: false). # @return [Boolean] @@ -523,6 +528,7 @@ def initialize @pattern = '**{,/*/**}/*_spec.rb' @exclude_pattern = '' @failure_exit_code = 1 + @error_exit_code = nil # so it can be overridden by failure exit code @fail_if_no_examples = false @spec_files_loaded = false diff --git a/lib/rspec/core/drb.rb b/lib/rspec/core/drb.rb index c0e59291c9..e44db97c3b 100644 --- a/lib/rspec/core/drb.rb +++ b/lib/rspec/core/drb.rb @@ -51,6 +51,7 @@ def options argv << "--order" << @submitted_options[:order] if @submitted_options[:order] add_failure_exit_code(argv) + add_error_exit_code(argv) add_full_description(argv) add_filter(argv, :inclusion, @filter_manager.inclusions) add_filter(argv, :exclusion, @filter_manager.exclusions) @@ -67,6 +68,12 @@ def add_failure_exit_code(argv) argv << "--failure-exit-code" << @submitted_options[:failure_exit_code].to_s end + def add_error_exit_code(argv) + return unless @submitted_options[:error_exit_code] + + argv << "--error-exit-code" << @submitted_options[:error_exit_code].to_s + end + def add_full_description(argv) return unless @submitted_options[:full_description] diff --git a/lib/rspec/core/invocations.rb b/lib/rspec/core/invocations.rb index 7e5049f686..4719085b36 100644 --- a/lib/rspec/core/invocations.rb +++ b/lib/rspec/core/invocations.rb @@ -37,7 +37,7 @@ def call(options, err, out) runner, options.args, formatter ) - success ? 0 : runner.configuration.failure_exit_code + runner.exit_code(success) end private diff --git a/lib/rspec/core/option_parser.rb b/lib/rspec/core/option_parser.rb index d63ef18b3f..c962374510 100644 --- a/lib/rspec/core/option_parser.rb +++ b/lib/rspec/core/option_parser.rb @@ -95,6 +95,11 @@ def parser(options) options[:failure_exit_code] = code end + parser.on('--error-exit-code CODE', Integer, + 'Override the exit code used when there are errors loading or running specs outside of examples.') do |code| + options[:error_exit_code] = code + end + parser.on('-X', '--[no-]drb', 'Run examples via DRb.') do |use_drb| options[:drb] = use_drb options[:runner] = RSpec::Core::Invocations::DRbWithFallback.new if use_drb diff --git a/lib/rspec/core/runner.rb b/lib/rspec/core/runner.rb index 862511a293..caf9c871af 100644 --- a/lib/rspec/core/runner.rb +++ b/lib/rspec/core/runner.rb @@ -84,7 +84,7 @@ def initialize(options, configuration=RSpec.configuration, world=RSpec.world) # @param out [IO] output stream def run(err, out) setup(err, out) - return @configuration.reporter.exit_early(@configuration.failure_exit_code) if RSpec.world.wants_to_quit + return @configuration.reporter.exit_early(exit_code) if RSpec.world.wants_to_quit run_specs(@world.ordered_example_groups).tap do persist_example_statuses @@ -112,7 +112,7 @@ def setup(err, out) # failed. def run_specs(example_groups) examples_count = @world.example_count(example_groups) - success = @configuration.reporter.report(examples_count) do |reporter| + examples_passed = @configuration.reporter.report(examples_count) do |reporter| @configuration.with_suite_hooks do if examples_count == 0 && @configuration.fail_if_no_examples return @configuration.failure_exit_code @@ -120,9 +120,9 @@ def run_specs(example_groups) example_groups.map { |g| g.run(reporter) }.all? end - end && !@world.non_example_failure + end - success ? 0 : @configuration.failure_exit_code + exit_code(examples_passed) end # @private @@ -186,6 +186,14 @@ def self.handle_interrupt end end + # @private + def exit_code(examples_passed=false) + return @configuration.error_exit_code || @configuration.failure_exit_code if @world.non_example_failure + return @configuration.failure_exit_code unless examples_passed + + 0 + end + private def persist_example_statuses diff --git a/spec/integration/spec_file_load_errors_spec.rb b/spec/integration/spec_file_load_errors_spec.rb index 2e017504ee..db897b398d 100644 --- a/spec/integration/spec_file_load_errors_spec.rb +++ b/spec/integration/spec_file_load_errors_spec.rb @@ -6,6 +6,7 @@ include FormatterSupport let(:failure_exit_code) { rand(97) + 2 } # 2..99 + let(:error_exit_code) { failure_exit_code + 1 } # 3..100 if RSpec::Support::Ruby.jruby_9000? let(:spec_line_suffix) { ":in `
'" } @@ -24,6 +25,7 @@ c.filter_gems_from_backtrace "gems/aruba" c.backtrace_exclusion_patterns << %r{/rspec-core/spec/} << %r{rspec_with_simplecov} c.failure_exit_code = failure_exit_code + c.error_exit_code = error_exit_code end end @@ -31,7 +33,7 @@ write_file_formatted "helper_with_error.rb", "raise 'boom'" run_command "--require ./helper_with_error" - expect(last_cmd_exit_status).to eq(failure_exit_code) + expect(last_cmd_exit_status).to eq(error_exit_code) output = normalize_durations(last_cmd_stdout) expect(output).to eq unindent(<<-EOS) @@ -60,7 +62,7 @@ " run_command "--require ./helper_with_error 1_spec.rb" - expect(last_cmd_exit_status).to eq(failure_exit_code) + expect(last_cmd_exit_status).to eq(error_exit_code) output = normalize_durations(last_cmd_stdout) expect(output).to eq unindent(<<-EOS) @@ -109,7 +111,7 @@ " run_command "1_spec.rb 2_spec.rb 3_spec.rb" - expect(last_cmd_exit_status).to eq(failure_exit_code) + expect(last_cmd_exit_status).to eq(error_exit_code) output = normalize_durations(last_cmd_stdout) expect(output).to eq unindent(<<-EOS) diff --git a/spec/integration/suite_hooks_errors_spec.rb b/spec/integration/suite_hooks_errors_spec.rb index b4c711cd62..b4a862d2fe 100644 --- a/spec/integration/suite_hooks_errors_spec.rb +++ b/spec/integration/suite_hooks_errors_spec.rb @@ -6,6 +6,7 @@ include FormatterSupport let(:failure_exit_code) { rand(97) + 2 } # 2..99 + let(:error_exit_code) { failure_exit_code + 2 } # 4..101 if RSpec::Support::Ruby.jruby_9000? let(:spec_line_suffix) { ":in `block in (root)'" } @@ -24,6 +25,7 @@ c.filter_gems_from_backtrace "gems/aruba" c.backtrace_exclusion_patterns << %r{/rspec-core/spec/} << %r{rspec_with_simplecov} c.failure_exit_code = failure_exit_code + c.error_exit_code = error_exit_code end end @@ -41,7 +43,7 @@ def run_spec_expecting_non_zero(before_or_after) " run_command "the_spec.rb" - expect(last_cmd_exit_status).to eq(failure_exit_code) + expect(last_cmd_exit_status).to eq(error_exit_code) normalize_durations(last_cmd_stdout) end @@ -96,7 +98,7 @@ def run_spec_expecting_non_zero(before_or_after) " run_command "the_spec.rb" - expect(last_cmd_exit_status).to eq(failure_exit_code) + expect(last_cmd_exit_status).to eq(error_exit_code) output = normalize_durations(last_cmd_stdout) expect(output).to eq unindent(<<-EOS) diff --git a/spec/rspec/core/configuration_options_spec.rb b/spec/rspec/core/configuration_options_spec.rb index ab7ee587d2..b077cb37f3 100644 --- a/spec/rspec/core/configuration_options_spec.rb +++ b/spec/rspec/core/configuration_options_spec.rb @@ -321,6 +321,18 @@ end end + describe "--error-exit-code" do + it "sets :error_exit_code" do + expect(parse_options('--error-exit-code', '0')).to include(:error_exit_code => 0) + expect(parse_options('--error-exit-code', '1')).to include(:error_exit_code => 1) + expect(parse_options('--error-exit-code', '2')).to include(:error_exit_code => 2) + end + + it "overrides previous :error_exit_code" do + expect(parse_options('--error-exit-code', '2', '--error-exit-code', '3')).to include(:error_exit_code => 3) + end + end + describe "--dry-run" do it "defaults to nil" do expect(parse_options[:dry_run]).to be(nil) diff --git a/spec/rspec/core/configuration_spec.rb b/spec/rspec/core/configuration_spec.rb index 3f2430091a..6c1692a100 100644 --- a/spec/rspec/core/configuration_spec.rb +++ b/spec/rspec/core/configuration_spec.rb @@ -2862,6 +2862,28 @@ def emulate_not_configured_expectation_framework end end + describe '#failure_exit_code' do + it 'defaults to 1' do + expect(config.failure_exit_code).to eq 1 + end + + it 'is configurable' do + config.failure_exit_code = 2 + expect(config.failure_exit_code).to eq 2 + end + end + + describe '#error_exit_code' do + it 'defaults to nil' do + expect(config.error_exit_code).to eq nil + end + + it 'is configurable' do + config.error_exit_code = 2 + expect(config.error_exit_code).to eq 2 + end + end + describe "#shared_context_metadata_behavior" do it "defaults to :trigger_inclusion for backwards compatibility" do expect(config.shared_context_metadata_behavior).to eq :trigger_inclusion diff --git a/spec/rspec/core/runner_spec.rb b/spec/rspec/core/runner_spec.rb index 0567453642..4b0804e472 100644 --- a/spec/rspec/core/runner_spec.rb +++ b/spec/rspec/core/runner_spec.rb @@ -232,6 +232,51 @@ def interrupt end end + describe '#exit_code' do + let(:world) { World.new } + let(:config) { Configuration.new } + let(:runner) { Runner.new({}, config, world) } + + it 'defaults to 1' do + expect(runner.exit_code).to eq 1 + end + + it 'is failure_exit_code by default' do + config.failure_exit_code = 2 + expect(runner.exit_code).to eq 2 + end + + it 'is failure_exit_code when world is errored by default' do + world.non_example_failure = true + config.failure_exit_code = 2 + expect(runner.exit_code).to eq 2 + end + + it 'is error_exit_code when world is errored by and both are defined' do + world.non_example_failure = true + config.failure_exit_code = 2 + config.error_exit_code = 3 + expect(runner.exit_code).to eq 3 + end + + it 'is error_exit_code when world is errored by and failure exit code is not defined' do + world.non_example_failure = true + config.error_exit_code = 3 + expect(runner.exit_code).to eq 3 + end + + it 'can be given success' do + config.error_exit_code = 3 + expect(runner.exit_code(true)).to eq 0 + end + + it 'can be given success, but non_example_failure=true will still cause an error code' do + world.non_example_failure = true + config.error_exit_code = 3 + expect(runner.exit_code(true)).to eq 3 + end + end + describe ".invoke" do let(:runner) { RSpec::Core::Runner } @@ -287,7 +332,6 @@ def interrupt expect(process_proxy).to have_received(:run).with(err, out) end end - end context "when run" do