Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

example_group is not longer supported #2232

Closed
marcandre opened this issue Apr 29, 2016 · 8 comments
Closed

example_group is not longer supported #2232

marcandre opened this issue Apr 29, 2016 · 8 comments

Comments

@marcandre
Copy link
Contributor

rspec currently states that "Filtering by an :example_group subhash is deprecated. Use the subhash to filter directly instead.". I took "deprecated" to mean that it still worked but wouldn't in a later version. It doesn't work.

Could it please either be supported, or else the warning should be an error with a different wording.

@myronmarston
Copy link
Member

I thought it was still supported. You're the first to report a problem with it. Can you provide an example of how it is not working for you?

@marcandre
Copy link
Contributor Author

I didn't investigate much, tbh, but here's an issue I encountered. The following tests do not pass, unless one replaces the example_group with the subhash directly. Or am I missing something?

require 'spec_helper'

module Foo
  def foo
    42
  end
end

module Bar
  def foo
    43
  end
end

RSpec.configure do |config|
  config.include Foo, example_group: { file_path: /./ }  # One test fails
  #  config.include Foo, file_path: /./  # If this is used instead, tests pass
  config.include Bar, :bar
end

describe 'Foo', :foo do
  it "works" do
    foo.should == 42
  end
end

describe 'Bar', :bar do
  it "works" do
    foo.should == 43
  end
end

@myronmarston
Copy link
Member

Thanks for the example. When I run those, inclusion of Foo via the example_group: { file_path: /./ } filter _does_work, but the Foo inclusion is taking precedence over the Bar inclusion so that the bar test fails (having gotten 42 instead of 43). If I swap which config.include line is uncommented, it passes like you said -- which makes it look like it changes the inclusion error. Definitely confusing and odd. I'll dig in some more.

@myronmarston
Copy link
Member

It gets more interesting. Apparently this regressed between RSpec 3.1 and 3.2. The example passes in 3.0 and 3.1 and fails in 3.2. Digging in some more...

@myronmarston
Copy link
Member

It's looking like #1749 is at fault here, causing the regression. If I change the spec file to this:

module Foo
  def self.included(base)
    puts "Included Foo in #{base.description}"
  end

  def foo
    42
  end
end

module Bar
  def self.included(base)
    puts "Included Bar in #{base.description}"
  end

  def foo
    43
  end
end

RSpec.configure do |config|
  config.include Foo, example_group: { file_path: /./ }  # One test fails
  # config.include Foo, file_path: /./  # If this is used instead, tests pass
  config.include Bar, bar: true
end

describe 'group: Foo', foo: true do
  it "example: works" do
    expect(foo).to eq 42
  end
end

describe 'group: Bar', bar: true do
  it "example: works" do
    expect(foo).to eq 43
  end
end

Then run it, it produces this output:

✗ bin/rspec -f d
Included Bar in group: Bar

group: Foo
Included Foo in example: works
  example: works

group: Bar
Included Foo in example: works
  example: works (FAILED - 1)

Failures:

  1) group: Bar example: works
     Failure/Error: expect(foo).to eq 43

       expected: 43
            got: 42

       (compared using ==)
     # ./spec/foo_spec.rb:35:in `block (2 levels) in <top (required)>'

Deprecation Warnings:

Filtering by an `:example_group` subhash is deprecated. Use the subhash to filter directly instead. Called from /Users/myron/code/rspec-scratch/issue-2232/spec/foo_spec.rb:22:in `block in <top (required)>'.


If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 0.02 seconds (files took 0.14583 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/foo_spec.rb:34 # group: Bar example: works

Then if I swap the uncommented Foo inclusion line, it prints this:

✗ bin/rspec -f d
Included Foo in group: Foo
Included Foo in group: Bar
Included Bar in group: Bar

group: Foo
  example: works

group: Bar
  example: works

Finished in 0.00154 seconds (files took 0.14555 seconds to load)
2 examples, 0 failures

In the first case, Included Foo in example: works is printed twice (once for each example), showing that Foo is being included in the singleton class of the examples. In the latter case, Included Foo in group: Foo is printed and Included Foo in group: Bar is printed, showing it is being included in the groups. This causes foo to return a different value in the Bar group due to module inclusion precedence.

More investigation is needed but that's what I've found so far.

@myronmarston
Copy link
Member

I don't have a root cause yet, but I need to head to bed.

myronmarston added a commit that referenced this issue Apr 30, 2016
This regressed in #1749. The changes there affected code like this:

``` ruby
RSpec.configure do |c|
  c.include Mod, :example_group => { :file_path => /./ }
end
```

In RSpec 2, and in RSpec 3 before #1749, this snippet would cause `Mod`
to be included in every example group (since `/./` matches all file
paths). After #1749, it would _not_ be included in any example groups.
Instead, it was included in the singleton class of every example.
In practice, this usually worked OK, as the methods from `Mod` were
available to be called very every example, but it had subtle problems:

* If the same method was defined in multiple included modules,
  the version of the method defined in a module included in this
  fashion would always get precedence due to the singleton class
  getting "first dibs" during method dispatch over the example group
  class. (This is the broken example given in #2232).
* I believe this broke `c.extend Mod, :example_group => {...}` since
  we don't extend onto singleton classes.

The reason is that the optimizations included in #1749 considered
only metadata keys that the groups and examples have in their metadata
hashes. In a group hash, `:example_group` is not a "real" metadata
key--instead it is created lazily on first access using a
`default_proc`. But example hashes still have it (it is a reference to
the metadata of their group), so it allowed example filtering to work
correctly, which in turn hid the bug.

Fixes #2232.
myronmarston added a commit that referenced this issue Apr 30, 2016
This regressed in #1749. The changes there affected code like this:

``` ruby
RSpec.configure do |c|
  c.include Mod, :example_group => { :file_path => /./ }
end
```

In RSpec 2, and in RSpec 3 before #1749, this snippet would cause `Mod`
to be included in every example group (since `/./` matches all file
paths). After #1749, it would _not_ be included in any example groups.
Instead, it was included in the singleton class of every example.
In practice, this usually worked OK, as the methods from `Mod` were
available to be called very every example, but it had subtle problems:

* If the same method was defined in multiple included modules,
  the version of the method defined in a module included in this
  fashion would always get precedence due to the singleton class
  getting "first dibs" during method dispatch over the example group
  class. (This is the broken example given in #2232).
* I believe this broke `c.extend Mod, :example_group => {...}` since
  we don't extend onto singleton classes.

The reason is that the optimizations included in #1749 considered
only metadata keys that the groups and examples have in their metadata
hashes. In a group hash, `:example_group` is not a "real" metadata
key--instead it is created lazily on first access using a
`default_proc`. But example hashes still have it (it is a reference to
the metadata of their group), so it allowed example filtering to work
correctly, which in turn hid the bug.

Fixes #2232.
@myronmarston
Copy link
Member

@marcandre: I've got a fix for this in #2234. Want to give it a try?

@marcandre
Copy link
Contributor Author

Thanks a lot for the fix!
I'll give it a try when it's released

MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this issue Oct 30, 2020
This regressed in rspec#1749. The changes there affected code like this:

``` ruby
RSpec.configure do |c|
  c.include Mod, :example_group => { :file_path => /./ }
end
```

In RSpec 2, and in RSpec 3 before rspec#1749, this snippet would cause `Mod`
to be included in every example group (since `/./` matches all file
paths). After rspec#1749, it would _not_ be included in any example groups.
Instead, it was included in the singleton class of every example.
In practice, this usually worked OK, as the methods from `Mod` were
available to be called very every example, but it had subtle problems:

* If the same method was defined in multiple included modules,
  the version of the method defined in a module included in this
  fashion would always get precedence due to the singleton class
  getting "first dibs" during method dispatch over the example group
  class. (This is the broken example given in rspec#2232).
* I believe this broke `c.extend Mod, :example_group => {...}` since
  we don't extend onto singleton classes.

The reason is that the optimizations included in rspec#1749 considered
only metadata keys that the groups and examples have in their metadata
hashes. In a group hash, `:example_group` is not a "real" metadata
key--instead it is created lazily on first access using a
`default_proc`. But example hashes still have it (it is a reference to
the metadata of their group), so it allowed example filtering to work
correctly, which in turn hid the bug.

Fixes rspec#2232.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants