config.include works unpredictably when supplying more than one metadata to filter

[timdiggins]

Subject of the issue

When supplying more than one metadata to filter for config.before or config.include, I would expect them both to:

• behave the same (config.before and config.include behave differently)
• require all the filters (config.before works like this, but config.include seems to just requires any of the metadata to be present)
• have a determinate order of running - the order they appear in the configuration

Or have I missed some docs where it says don't use more than one metadata to filter on?

Your environment

• Ruby version: 3.0
• rspec-core version: v3.13.0

Steps to reproduce

inline ruby

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.13.0" # Activate the gem and version you are reporting the issue against.
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

module TestTypeBase
  def sample_method
    "TestTypeBase"
  end

  attr_reader :test_state
  def store_test_state state
    @test_state ||= []
    @test_state << state
  end
end

module TestTypeSystem
  def sample_method
    "TestTypeSystem"
  end
end

module TestJs
  def sample_method
    "TestJs"
  end
end

module BothSpecified
  def sample_method
    "BothSpecified"
  end
end

RSpec.configure do |config|
  config.include TestTypeBase
  config.include TestTypeSystem, test_type: :system
  config.include TestJs, test_js: true
  config.include BothSpecified, test_type: :system, test_js: true
  config.before(:each) do
    store_test_state("before (all)")
  end
  config.before(:each, test_type: :system) do
    store_test_state("before test_type=system")
  end
  config.before(:each, test_js: true) do
    store_test_state("before test_js=true")
  end
  config.before(:each, test_type: :system, test_js: true) do
    store_test_state("before test_type=system&test_js=true")
  end
end

RSpec.describe "test support" do
  it "with no metadata supplied" do
    aggregate_failures do
      expect(test_state).to include("before (all)")
      expect(is_a?(TestTypeBase)).to be_truthy
      expect(test_state).not_to include("before test_type=system")
      expect(is_a?(TestTypeSystem)).to be_falsey
      expect(test_state).not_to include("before test_js=true")
      expect(is_a?(TestJs)).to be_falsey
      expect(test_state).not_to include("before test_type=system&test_js=true")
      expect(is_a?(BothSpecified)).to be_falsey
      expect(sample_method).to eq("TestTypeBase")
    end
  end
  it "with test_type system", test_type: :system do
    aggregate_failures do
      expect(test_state).to include("before (all)")
      expect(is_a?(TestTypeBase)).to be_truthy
      expect(test_state).to include("before test_type=system")
      expect(is_a?(TestTypeSystem)).to be_truthy
      expect(test_state).not_to include("before test_js=true")
      expect(is_a?(TestJs)).to be_falsey
      expect(test_state).not_to include("before test_type=system&test_js=true") # succeeds
      expect(is_a?(BothSpecified)).to be_falsey # fails
      expect(sample_method).to eq("TestTypeSystem")
    end
  end
  it "with test_js", test_js: true do
    aggregate_failures do
      expect(test_state).to include("before (all)")
      expect(is_a?(TestTypeBase)).to be_truthy
      expect(test_state).not_to include("before test_type=system")
      expect(is_a?(TestTypeSystem)).to be_falsey
      expect(test_state).to include("before test_js=true")
      expect(is_a?(TestJs)).to be_truthy
      expect(test_state).not_to include("before test_type=system&test_js=true") # succeeds
      expect(is_a?(BothSpecified)).to be_falsey # fails
      expect(sample_method).to eq("TestJs")
    end
  end
  describe "with test_type: :system at group level", test_type: :system do
    it "with test_js: true", test_js: true do
      aggregate_failures do
        expect(test_state).to include("before (all)")
        expect(is_a?(TestTypeBase)).to be_truthy
        expect(test_state).to include("before test_type=system")
        expect(is_a?(TestTypeSystem)).to be_truthy
        expect(test_state).to include("before test_js=true")
        expect(is_a?(TestJs)).to be_truthy
        expect(test_state).to include("before test_type=system&test_js=true")
        expect(is_a?(BothSpecified)).to be_truthy
        expect(sample_method).to eq("BothSpecified") # fails - compare below
      end
    end
  end
  describe "with nothing at group level" do
    it "with test_type: :system and test_js: true", test_type: :system, test_js: true do
      aggregate_failures do
        expect(test_state).to include("before (all)")
        expect(is_a?(TestTypeBase)).to be_truthy
        expect(test_state).to include("before test_type=system")
        expect(is_a?(TestTypeSystem)).to be_truthy
        expect(test_state).to include("before test_js=true")
        expect(is_a?(TestJs)).to be_truthy
        expect(test_state).to include("before test_type=system&test_js=true")
        expect(is_a?(BothSpecified)).to be_truthy
        expect(sample_method).to eq("BothSpecified") # succeeds - compare above
      end
    end
  end
end

Expected behavior

No failures.

Actual behavior

3 failures

output of running above code

ruby rspec_issue.rb                                                                                         ─╯
Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Ruby version is: 3.0.6
.FFF.

Failures:

  1) test support with test_type system
     Got 2 failures from failure aggregation block.
     # rspec_issue.rb:83:in `block (2 levels) in <main>'

     1.1) Failure/Error: expect(is_a?(BothSpecified)).to be_falsey # fails

            expected: falsey value
                 got: true
          # rspec_issue.rb:91:in `block (3 levels) in <main>'

     1.2) Failure/Error: expect(sample_method).to eq("TestTypeSystem")

            expected: "TestTypeSystem"
                 got: "BothSpecified"

            (compared using ==)
          # rspec_issue.rb:92:in `block (3 levels) in <main>'

  2) test support with test_js
     Got 2 failures from failure aggregation block.
     # rspec_issue.rb:96:in `block (2 levels) in <main>'

     2.1) Failure/Error: expect(is_a?(BothSpecified)).to be_falsey # fails

            expected: falsey value
                 got: true
          # rspec_issue.rb:104:in `block (3 levels) in <main>'

     2.2) Failure/Error: expect(sample_method).to eq("TestJs")

            expected: "TestJs"
                 got: "BothSpecified"

            (compared using ==)
          # rspec_issue.rb:105:in `block (3 levels) in <main>'

  3) test support with test_type: :system at group level with test_js: true
     Failure/Error: expect(sample_method).to eq("BothSpecified") # fails - compare below

       expected: "BothSpecified"
            got: "TestJs"

       (compared using ==)
     # rspec_issue.rb:119:in `block (4 levels) in <main>'
     # rspec_issue.rb:110:in `block (3 levels) in <main>'

Finished in 0.01717 seconds (files took 0.07062 seconds to load)
5 examples, 3 failures

Failed examples:

rspec rspec_issue.rb:82 # test support with test_type system
rspec rspec_issue.rb:95 # test support with test_js
rspec rspec_issue.rb:109 # test

[pirj]

This would be a breaking change, and we plan to release all this in RSpec 4.
Please see #2878 for trigger inclusion; #1821 / #2874 for unification if multi-condition filtering.

I kindly appreciate it if you could check if the code on 4.0-dev branch meets your expectations. The branch is behind our main branch and may miss some latest fixes. Please let me know if it doesn’t work properly, it will be a good motivation to forward-port fixes to it.

[timdiggins]

I've tested 4-0-dev against the inline code and it passes. 🎉

FYI using

  gem "rspec-support", github: "rspec/rspec-support", branch: "4-0-dev"
  gem "rspec-core", github: "rspec/rspec-core", branch: "4-0-dev"
  gem "rspec-expectations", github: "rspec/rspec-expectations", branch: "4-0-dev"
  gem "rspec-mocks", github: "rspec/rspec-mocks", branch: "4-0-dev"

So that's wonderful!

Also tested rspec 4 on a rails project with a decent number of tests and all ran fine (after adapting to rspec 4 changes).

@pirj Had no idea about RSpec 4.0 -- so much great work, and yet seemingly still so far away (looking at rspec/rspec-metagem#61). Yet looks like the main thing to complete on the 4.0 list is testing (there's also a post-4.0 list and a cleanup list - but are those optional?)

[pirj]

Awesome! Thanks for checking this.
I’ll close this as completed.

Almost there actually. We wanted to glue all repos together into one repo. It would allow to shuffle things between repos, simplify our builds, and simplify cross-gem fixes. but it’s tricky and time-consuming with all those tags and branches.

[JonRowe]

4.0 is mostly waiting on me to finish the monorepo setup and prepare the release from existing work, plus any preparatory work for 3.99 I hope to get back to this soon.

I'm going to close this as 4.0 seems to fix your issue and it would be a breaking change to fix it any earlier