rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

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

Closed timdiggins closed 7 months ago

timdiggins commented 7 months ago

Subject of the issue

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

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

Your environment

Steps to reproduce

inline ruby ```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
' 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
' 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
' 2) test support with test_js Got 2 failures from failure aggregation block. # rspec_issue.rb:96:in `block (2 levels) in
' 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
' 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
' 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
' # rspec_issue.rb:110:in `block (3 levels) in
' 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 commented 7 months ago

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 commented 7 months ago

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 https://github.com/rspec/rspec/issues/2). 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 commented 7 months ago

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 commented 7 months ago

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