search

Shopify / deprecation_toolkit

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

DeprecationToolkit currently can't catch deprecations that are generated before tests are executed #20

Open rmacklin opened 5 years ago

rmacklin commented 5 years ago

Consider this example of a deprecation warning: https://github.com/thoughtbot/shoulda-matchers/blob/94b28e4ef4cc001b1c0b4386143b8c05ac163550/lib/shoulda/matchers/warn.rb#L20-L25

A test like this would trigger that deprecation:

class FooModelTest < ActiveSupport::TestCase
  should define_enum_for(:status).with([:pending, :completed, :failed])
end

Here's what gets logged:

************************************************************************
Warning from shoulda-matchers:

The `with` qualifier on `define_enum_for` is deprecated and will be
removed in the next major release. Please use `with_values` instead.
************************************************************************

However, because the deprecated method (with) is called in the code that declares the test cases, not called from within a test, DeprecationToolkit won't catch this deprecation. The deprecation gets logged before attach_subscriber has been invoked.

Is it possible to subscribe to deprecations earlier in the life of the test process?

rmacklin commented 5 years ago

@Edouard-chin Any thoughts on this issue? Also, would you be able to cut a new release?

Edouard-chin commented 5 years ago

👋 I dug a bit on this issue, it's non trivial to fix since Minitest will init the plugin only only during the ruby at_exit hook https://github.com/seattlerb/minitest/blob/e6bc4485730403faff6966c1671cf5de72b2d233/lib/minitest.rb#L53 (way after the class and any class method call happen). For now I don't have a solution without monkypatching minitest which I want to avoid. Will keep the issue open for now and find when I have the time if there is a better alternative.

I released v1.0.3 https://rubygems.org/gems/deprecation_toolkit/versions/1.0.3

rmacklin commented 5 years ago

Thanks for releasing that!

I took a stab at one way we could catch deprecations that are emitted at class-evaluation time: https://github.com/Shopify/deprecation_toolkit/pull/25

It's not perfect - see https://github.com/Shopify/deprecation_toolkit/pull/25#issuecomment-433220570