Shopify / deprecation_toolkit

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

Rspec support? #17

Closed andrewmarkle closed 5 years ago

andrewmarkle commented 5 years ago

Currently only minitest is supported but it would be great to include out-of-the-box support for rspec as well.

I managed to get this (sort-of) working by adding it to my rspec rails_helper file.

config.before(:all) do
    notify = ActiveSupport::Deprecation::DEFAULT_BEHAVIORS[:notify]
    behaviors = ActiveSupport::Deprecation.behavior

    unless behaviors.find { |behavior| behavior == notify }
      ActiveSupport::Deprecation.behavior = behaviors << notify
    end

    DeprecationToolkit::Configuration.attach_to.each do |gem_name|
      DeprecationToolkit::DeprecationSubscriber.attach_to gem_name
    end
  end

  config.after do
    current_deprecations = DeprecationToolkit::Collector.new(DeprecationToolkit::Collector.deprecations)
    recorded_deprecations = DeprecationToolkit::Collector.load(self.class)
    if current_deprecations != recorded_deprecations
      DeprecationToolkit::Configuration.behavior.trigger(self.class, current_deprecations, recorded_deprecations)
    end
  end

And this config in my config/test.rb

  DeprecationToolkit::Configuration.deprecation_path = Rails.root.join('spec/deprecations')
  DeprecationToolkit::Configuration.behavior = DeprecationToolkit::Behaviors::Record
  DeprecationToolkit::Configuration.warnings_treated_as_deprecation

While this works (it'll create a yaml file where you tell it to go) it's not in the correct folder structure nor does it have the right test naming. The challenge is in how minitest vs rspec names it's test cases. There would have to be some change in ReadWriteHelper for it to work as self.name is not a thing in rspec. (In my example I've hackily gotten around this by passing in self.class which does respond_to .name).

Happy to help more with this one if you want!

rafaelfranca commented 5 years ago

Not against adding RSpec support but we are not RSpec users so we would have to rely on contributions from the community.

andrewmarkle commented 5 years ago

I'll take a crack at it.

andrewmarkle commented 5 years ago

Hey everyone. I've been thinking about this a fair bit but would like a bit of guidance for how to proceed. Right now minitest is baked-in to the current version. Very much a required dependency. If we were to add in rspec support I'm curious would that would look like?

I've been browsing around different gems and I like how the mutant gem solves this problem. The basic strategy would be to have two additional gemspecs. One for minitest and one for Rspec.

The main difference for users is that in your gemfile one would have to declare which one you want. Either gem 'deprecation_toolkit-minitest' or gem 'deprecation_toolkit-rspec'.

Would appreciate any thoughts!

rafaelfranca commented 5 years ago

Not much a fan of this. I think we can remove the runtime dependency on minitest and only try to load it when the parts that depend on minitest are used.

Edouard-chin commented 5 years ago

https://github.com/Shopify/deprecation_toolkit/pull/37