Fix a regression in our metadata backwards compatibility #1490
Conversation
myronmarston
commented
Apr 17, 2014
myronmarston
commented
This is a bit confusing, but the behavior in 2.x was: * a group's `metadata` did not have it's computed keys directly available; they were exposed off of the `:example_group` subhash. * The `:example_group` subhash had a `:example_group` key that exposed the parent group's metadata (both user metadata and computed keys). Before this, we were inserting extra `:example_group` subhashes between the layers.
myronmarston
changed the title
|
/cc @JonRowe |
|
Oops, LGTM, merge when green. |
|
Actually, I found a case that still has a bug...digging in now. |
| @@ -460,13 +460,29 @@ def value_for(*args) | |||
| describe("nested") { child = metadata } | |||
| end | |||
|
|
|||
| expect(child[:example_group][:example_group]).to include( | |||
| expect(child[:example_group][:example_group].to_h).to include( | |||
There was a problem hiding this comment.
Out of curiosity, is this not a hash already?
There was a problem hiding this comment.
Nope. It's a LegacyExampleGroupHash instance.
In RSpec 2.x, an example's `metadata[:example_group]` did not returned it's example group's `metadata`, but rather it's example group's `metadata[:example_group]`, which is where all the computed values for the example group went. In RSpec 3, we've simplified this nesting structure (it's silly to put an example group's computed properties on a nested subhash keyed by `:example_group`), but our backwards compatibility didn't work properly with this. The problem was that `example.metadata[:example_group][:example_group]` returned the `LegacyExampleGroupHash` available off of `example.example_group.metadata[:example_group]`, which contains the computed properties of the example group, but on 2.99 this returned the computed properties of the parent group of the example's group. Unfortunately, I had to make one spec pending as I can't find a way to keep it passing with the difference in how the metadata is exposed from an example vs a group, but it really only matters when people mutate the metadata hash and that's not a normal, supported use case. I think the backwards compatibility is more important right now. We may revert this at some point after 3.0's been out for awhile and people have had time to adjust to the deprecation warnings issued for the metadata changes.
|
So, one major use case where metadata is mutated is rspec-rails. What happens in that situation? |
|
Well, I thought our plan was to provide an official API to meet rspec-rails' needs so that it wouldn't have to mutate anymore? Anyhow, the change here only affects mutation when it's done to the example group metadata after examples in that group have been defined. I think rspec-rails' mutation is done before examples are defined in the group? |
|
That's the plan, but we're not there yet, I think |
|
Anyway you understand this better than me so it gets my sign off. |
Nope, mutation like this happens before any examples are defined (I just ran some tests to confirm it). There is, however, this mutation that could happen after examples are defined: But the rspec-rails build appears to be green with this change. Also, mutation isn't necessarily a problem; it just means that |
Fix a regression in our metadata backwards compatibility
See rspec/rspec-core#1490, rspec/rspec-core@9ffcc38 --- This commit was imported from rspec/rspec-core@9daaf1f.
