diff --git a/Changelog.md b/Changelog.md index f7fe77900f..fe3008a184 100644 --- a/Changelog.md +++ b/Changelog.md @@ -19,6 +19,8 @@ Bug Fixes: * 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) diff --git a/lib/rspec/core/configuration.rb b/lib/rspec/core/configuration.rb index 39111fb107..4d2ea329d1 100644 --- a/lib/rspec/core/configuration.rb +++ b/lib/rspec/core/configuration.rb @@ -1855,9 +1855,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. diff --git a/spec/rspec/core/configuration_spec.rb b/spec/rspec/core/configuration_spec.rb index 77e165b23e..975f607748 100644 --- a/spec/rspec/core/configuration_spec.rb +++ b/spec/rspec/core/configuration_spec.rb @@ -1819,6 +1819,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|