Merge branch 'master' into add-isolated-env-for-rake

Author: JonRowe

Changelog.md

@@ -11,16 +11,23 @@ Enhancements:
   outside of examples. (@obromios, #2601)
 * Add a minimalist quick fix style formatter, only outputs failures as
   `file:line:message`. (Romain Tartière, #2614)
+* Convert string number values to integer when used for `RSpec::Configuration#fail_fast`
+  (Viktor Fonic, #2634)
+* Issue warning when invalid values are used for `RSpec::Configuration#fail_fast`
+  (Viktor Fonic, #2634)
 * Add support for running the Rake task in a clean environment.
   (Jon Rowe, #2632)
 
+
 Bug Fixes:
 
 * Handle RSpec description(s) with japanese chars in CP932 encoded files.
   (Benoit Tigeot, #2575)
 * When defining `let` methods that overwrite an existing method, prevent
   a warning being issued by removing the old definition. (Jon Rowe, #2593)
 * Prevent warning on Ruby 2.6.0-rc1 (Keiji Yoshimi, #2582)
+* Fix `config.define_derived_metadata` so that it supports cascades.
+  (Myron Marston, #2630).
 
 ### 3.8.0 / 2018-08-04
 [Full Changelog](http://github.com/rspec/rspec-core/compare/v3.7.1...v3.8.0)

lib/rspec/core/configuration.rb

@@ -202,10 +202,33 @@ def only_failures_but_not_configured?
         only_failures? && !example_status_persistence_file_path
       end
 
-      # @macro add_setting
+      # @macro define_reader
       # If specified, indicates the number of failures required before cleaning
-      # up and exit (default: `nil`).
-      add_setting :fail_fast
+      # up and exit (default: `nil`). Can also be `true` to fail and exit on first
+      # failure
+      define_reader :fail_fast
+
+      # @see fail_fast
+      def fail_fast=(value)
+        case value
+        when true, 'true'
+          @fail_fast = true
+        when false, 'false', 0
+          @fail_fast = false
+        when nil
+          @fail_fast = nil
+        else
+          @fail_fast = value.to_i
+
+          if value.to_i == 0
+            # TODO: in RSpec 4, consider raising an error here.
+            RSpec.warning "Cannot set `RSpec.configuration.fail_fast`" \
+              " to `#{value.inspect}`. Only `true`, `false`, `nil` and integers" \
+              " are valid values."
+            @fail_fast = true
+          end
+        end
+      end
 
       # @macro add_setting
       # Prints the formatter output of your suite without running any
@@ -1855,9 +1878,28 @@ def when_first_matching_example_defined(*filters)
 
       # @private
       def apply_derived_metadata_to(metadata)
-        @derived_metadata_blocks.items_for(metadata).each do |block|
-          block.call(metadata)
+        already_run_blocks = Set.new
+
+        # We loop and attempt to re-apply metadata blocks to support cascades
+        # (e.g. where a derived bit of metadata triggers the application of
+        # another piece of derived metadata, etc)
+        #
+        # We limit our looping to 200 times as a way to detect infinitely recursing derived metadata blocks.
+        # It's hard to imagine a valid use case for a derived metadata cascade greater than 200 iterations.
+        200.times do
+          return if @derived_metadata_blocks.items_for(metadata).all? do |block|
+            already_run_blocks.include?(block).tap do |skip_block|
+              block.call(metadata) unless skip_block
+              already_run_blocks << block
+            end
+          end
         end
+
+        # If we got here, then `@derived_metadata_blocks.items_for(metadata).all?` never returned
+        # `true` above and we treat this as an attempt to recurse infinitely. It's better to fail
+        # with a clear # error than hang indefinitely, which is what would happen if we didn't limit
+        # the looping above.
+        raise SystemStackError, "Attempted to recursively derive metadata indefinitely."
       end
 
       # Defines a `before` hook. See {Hooks#before} for full docs.

spec/rspec/core/configuration_spec.rb

@@ -41,6 +41,87 @@ module RSpec::Core
       end
     end
 
+    describe "#fail_fast=" do
+      context 'when true' do
+        it 'is set to true' do
+          config.fail_fast = true
+          expect(config.fail_fast).to eq true
+        end
+      end
+
+      context "when 'true'" do
+        it 'is set to true' do
+          config.fail_fast = 'true'
+          expect(config.fail_fast).to eq true
+        end
+      end
+
+      context "when false" do
+        it 'is set to false' do
+          config.fail_fast = false
+          expect(config.fail_fast).to eq false
+        end
+      end
+
+      context "when 'false'" do
+        it 'is set to false' do
+          config.fail_fast = 'false'
+          expect(config.fail_fast).to eq false
+        end
+      end
+
+      context "when 0" do
+        it 'is set to false' do
+          config.fail_fast = 0
+          expect(config.fail_fast).to eq false
+        end
+      end
+
+      context "when integer number" do
+        it 'is set to number' do
+          config.fail_fast = 5
+          expect(config.fail_fast).to eq 5
+        end
+      end
+
+      context "when floating point number" do
+        it 'is set to integer number' do
+          config.fail_fast = 5.9
+          expect(config.fail_fast).to eq 5
+        end
+      end
+
+      context "when string represeting an integer number" do
+        it 'is set to number' do
+          config.fail_fast = '5'
+          expect(config.fail_fast).to eq 5
+        end
+      end
+
+      context "when nil" do
+        it 'is nil' do
+          config.fail_fast = nil
+          expect(config.fail_fast).to eq nil
+        end
+      end
+
+      context "when unrecognized value" do
+        before do
+          allow(RSpec).to receive(:warning)
+        end
+
+        it 'prints warning' do
+          config.fail_fast = 'yes'
+          expect(RSpec).to have_received(:warning).with(/Cannot set `RSpec.configuration.fail_fast`/i)
+        end
+
+        it 'is set to true' do
+          config.fail_fast = 'yes'
+          expect(config.fail_fast).to eq true
+        end
+      end
+    end
+
     describe 'fail_if_no_examples' do
       it 'defaults to false' do
         expect(RSpec::Core::Configuration.new.fail_if_no_examples).to be(false)
@@ -1819,6 +1900,42 @@ def exclude?(line)
         expect(group.metadata).to include(:b1_desc => "bar (block 1)", :b2_desc => "bar (block 1) (block 2)")
       end
 
+      it 'supports cascades of derived metadata, but avoids re-running derived metadata blocks that have already been applied' do
+        RSpec.configure do |c|
+          c.define_derived_metadata(:foo1) { |m| m[:foo2] = (m[:foo2] || 0) + 1 }
+          c.define_derived_metadata(:foo2) { |m| m[:foo3] = (m[:foo3] || 0) + 1 }
+          c.define_derived_metadata(:foo3) { |m| m[:foo1] += 1 }
+        end
+
+        group = RSpec.describe("bar", :foo1 => 0)
+        expect(group.metadata).to include(:foo1 => 1, :foo2 => 1, :foo3 => 1)
+
+        ex = RSpec.describe("My group").example("foo", :foo1 => 0)
+        expect(ex.metadata).to include(:foo1 => 1, :foo2 => 1, :foo3 => 1)
+      end
+
+      it 'does not allow a derived metadata cascade to recurse infinitely' do
+        RSpec.configure do |c|
+          counter = 1
+          derive_next_metadata = lambda do |outer_meta|
+            tag = :"foo#{counter += 1}"
+            outer_meta[tag] = true
+
+            c.define_derived_metadata(tag) do |inner_meta|
+              derive_next_metadata.call(inner_meta)
+            end
+          end
+
+          c.define_derived_metadata(:foo1) do |meta|
+            derive_next_metadata.call(meta)
+          end
+        end
+
+        expect {
+          RSpec.describe("group", :foo1)
+        }.to raise_error(SystemStackError)
+      end
+
       it "derives metadata before the group or example blocks are eval'd so their logic can depend on the derived metadata" do
         RSpec.configure do |c|
           c.define_derived_metadata(:foo) do |metadata|