Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Fix filtering based on :example_group to work for example groups. #2234

Merged
merged 1 commit into from
May 1, 2016

Conversation

myronmarston
Copy link
Member

This regressed in #1749. The changes there affected code like this:

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 example_group is not longer supported #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.

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.
@JonRowe
Copy link
Member

JonRowe commented May 1, 2016

LGTM!

@JonRowe JonRowe merged commit 49b8a3f into master May 1, 2016
@JonRowe JonRowe deleted the myron/fix-2232 branch May 1, 2016 08:21
JonRowe added a commit that referenced this pull request May 22, 2016
Fix filtering based on `:example_group` to work for example groups.
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020
Fix filtering based on `:example_group` to work for example groups.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
…2232

Fix filtering based on `:example_group` to work for example groups.

---
This commit was imported from rspec/rspec-core@faa28ac.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants