Shopify / deprecation_toolkit

⚒Eliminate deprecations from your codebase ⚒
MIT License
460 stars 40 forks source link

RSpec integration causes collisions when recording deprecations generated by shared examples #41

Closed aergonaut closed 9 months ago

aergonaut commented 5 years ago

I have discovered what I believe is a bug in the RSpec integration. It comes up when using shared examples:

  1. Have a shared example in a separate file that defines a nested context, e.g.

    # spec/support/shared/shared_example.rb
    RSpec.shared_examples "shared example" do
     context "nested shared context" do
       it "breaks here" do
         # call some other app code that raises deprecations
       end
     end
    end
  2. Now include the shared examples in two different specs:

    # spec/first_file_spec.rb
    include_examples "shared example"
    # spec/second_file_spec.rb
    include_examples "shared example"
  3. Both first_file_spec.rb and second_file_spec.rb will generate deprecations, but the record behavior will record these deprecations as though they were generated by the spec/support/shared/shared_example.rb file, as opposed to either of the spec files. I would expect that the deprecations would be attributed to the spec files, not the shared example file.

Since shared examples can take parameters, different usages of shared examples could generate different deprecations. Because the RSpec integration links all deprecations generated by all usages of the shared examples to the same file, the different lists of deprecations clobber each other when recording. Then with the raise behavior, the usages that generate different deprecations than the one that won out when recording cause exceptions.

The solution I came up with, and what is implemented in this PR, is to modify recorded_deprecations_path when the test runner is RSpec to use location_rerun_argument instead of file_path. The difference is that, when the shared example is in a nested context, the context will report its file path as the file that defined the shared example, whereas the location_rerun_argument will point to the specific spec file that used the shared example.

The biggest problem I could see with this approach was that it would be breaking a change for current users of the RSpec integration, because the gem would now expect to find their recordings in a different location. To maintain backwards compatibility with current users of the RSpec integration, I introduced a new config option called use_legacy_rspec_recorded_deprecations_path, which is defaulted to false. When set to true, the previous behavior of recorded_deprecations_path is used instead of the new behavior. In the release notes for the gem, we can recommend to current users of the RSpec integration to set this parameter to true until they can re-record deprecations.