Fail fast when we can't support --only-failures due to lack of config.

Author: myronmarston

features/command_line/only_failures.feature

@@ -102,3 +102,8 @@ Feature: Only Failures
 
     When I run `rspec --next-failure`
     Then the output should contain "All examples were filtered out"
+
+  Scenario: Clear error given when using `--only-failures` without configuring `example_status_persistence_file_path`
+    Given I have not configured `example_status_persistence_file_path`
+     When I run `rspec --only-failures`
+     Then it should fail with "To use `--only-failures`, you must first set `config.example_status_persistence_file_path`."

features/step_definitions/additional_cli_steps.rb

@@ -138,3 +138,15 @@
     File.open(file_name, "w") { |f| f.write(fixed) }
   end
 end
+
+Then(/^it should fail with "(.*?)"$/) do |snippet|
+  assert_failing_with(snippet)
+end
+
+Given(/^I have not configured `example_status_persistence_file_path`$/) do
+  in_current_dir do
+    return unless File.exist?("spec/spec_helper.rb")
+    return unless File.read("spec/spec_helper.rb").include?("example_status_persistence_file_path")
+    File.open("spec/spec_helper.rb", "w") { |f| f.write("") }
+  end
+end

lib/rspec/core/configuration.rb

@@ -176,6 +176,11 @@ def example_status_persistence_file_path=(value)
       define_reader :only_failures
       alias_method :only_failures?, :only_failures
 
+      # @private
+      def only_failures_but_not_configured?
+        only_failures? && !example_status_persistence_file_path
+      end
+
       # @macro add_setting
       # Clean up and exit after the first failure (default: `false`).
       add_setting :fail_fast

lib/rspec/core/formatters/base_text_formatter.rb

@@ -68,6 +68,7 @@ def close(_notification)
 
           output.puts
 
+          output.flush
           output.close unless output == $stdout
         end
       end

lib/rspec/core/reporter.rb

@@ -154,7 +154,7 @@ def finish
                                                                    @pending_examples, @load_time)
       notify :seed, Notifications::SeedNotification.new(@configuration.seed, seed_used?)
     ensure
-      notify :close, Notifications::NullNotification
+      close
     end
 
     # @private
@@ -170,8 +170,19 @@ def notify(event, notification)
       end
     end
 
+    # @private
+    def abort_with(msg, exit_status)
+      message(msg)
+      close
+      exit!(exit_status)
+    end
+
   private
 
+    def close
+      notify :close, Notifications::NullNotification
+    end
+
     def mute_profile_output?
       # Don't print out profiled info if there are failures and `--fail-fast` is
       # used, it just clutters the output.

lib/rspec/core/world.rb

@@ -112,6 +112,7 @@ def reporter
       #
       # Notify reporter of filters.
       def announce_filters
+        fail_if_config_and_cli_options_invalid
         filter_announcements = []
 
         announce_inclusion_filter filter_announcements
@@ -175,6 +176,16 @@ def announce_exclusion_filter(announcements)
       def declaration_line_numbers
         @declaration_line_numbers ||= FlatMap.flat_map(example_groups, &:declaration_line_numbers)
       end
+
+      def fail_if_config_and_cli_options_invalid
+        return unless @configuration.only_failures_but_not_configured?
+
+        reporter.abort_with(
+          "\nTo use `--only-failures`, you must first set " \
+          "`config.example_status_persistence_file_path`.",
+          1 # exit code
+        )
+      end
     end
   end
 end

spec/rspec/core/formatters/base_text_formatter_spec.rb

@@ -5,13 +5,21 @@
   include FormatterSupport
 
   context "when closing the formatter", :isolated_directory => true do
+    let(:output_to_close) { File.new("./output_to_close", "w") }
+    let(:formatter) { described_class.new(output_to_close) }
+
     it 'does not close an already closed output stream' do
-      output_to_close = File.new("./output_to_close", "w")
-      formatter = described_class.new(output_to_close)
       output_to_close.close
 
       expect { formatter.close(RSpec::Core::Notifications::NullNotification) }.not_to raise_error
     end
+
+    it "flushes output before closing the stream so buffered bytes are not lost if we exit right away" do
+      expect(output_to_close).to receive(:flush).ordered
+      expect(output_to_close).to receive(:close).ordered
+
+      formatter.close(RSpec::Core::Notifications::NullNotification)
+    end
   end
 
   describe "#dump_summary" do

spec/rspec/core/reporter_spec.rb

@@ -206,6 +206,29 @@ module RSpec::Core
       end
     end
 
+    describe "#abort_with" do
+      before { allow(reporter).to receive(:exit!) }
+
+      it "publishes the message and notifies :close" do
+        listener = double("Listener")
+        reporter.register_listener(listener, :message, :close)
+        stream   = StringIO.new
+
+        allow(listener).to receive(:message) { |n| stream << n.message }
+        allow(listener).to receive(:close) { stream.close }
+
+        reporter.register_listener(listener)
+        reporter.abort_with("Booom!", 1)
+
+        expect(stream).to have_attributes(:string => "Booom!").and be_closed
+      end
+
+      it "exits with the provided exit code" do
+        reporter.abort_with("msg", 13)
+        expect(reporter).to have_received(:exit!).with(13)
+      end
+    end
+
     describe "timing" do
       before do
         config.start_time = start_time

spec/rspec/core/world_spec.rb

@@ -109,9 +109,49 @@ module RSpec::Core
     end
 
     describe "#announce_filters" do
-      let(:reporter) { double('reporter').as_null_object }
+      let(:reporter) { instance_spy(Reporter) }
       before { allow(world).to receive(:reporter) { reporter } }
 
+      context "when --only-failures is passed" do
+        before { configuration.force(:only_failures => true) }
+
+        context "and `example_status_persistence_file_path` is not configured" do
+          it 'aborts with a message explaining the config option must be set first' do
+            configuration.example_status_persistence_file_path = nil
+            world.announce_filters
+            expect(reporter).to have_received(:abort_with).with(/example_status_persistence_file_path/, 1)
+          end
+        end
+
+        context "and `example_status_persistence_file_path` is configured" do
+          it 'does not abort' do
+            configuration.example_status_persistence_file_path = "foo.txt"
+            world.announce_filters
+            expect(reporter).not_to have_received(:abort_with)
+          end
+        end
+      end
+
+      context "when --only-failures is not passed" do
+        before { expect(configuration.only_failures?).not_to eq true }
+
+        context "and `example_status_persistence_file_path` is not configured" do
+          it 'does not abort' do
+            configuration.example_status_persistence_file_path = nil
+            world.announce_filters
+            expect(reporter).not_to have_received(:abort_with)
+          end
+        end
+
+        context "and `example_status_persistence_file_path` is configured" do
+          it 'does not abort' do
+            configuration.example_status_persistence_file_path = "foo.txt"
+            world.announce_filters
+            expect(reporter).not_to have_received(:abort_with)
+          end
+        end
+      end
+
       context "with no examples" do
         before { allow(world).to receive(:example_count) { 0 } }