How to deal with deprecations triggered on load?

[iangreenleaf]

Related to #20, but seemed different enough to put in a separate ticket.

I'm working on setting up this gem in our app and am liking it a lot, but I'm having trouble figuring out how to manage the deprecations that are triggered by class-level code execution. If files are autoloaded, those deprecations pop up unpredictably depending on which test happens to load the class first.

I'm curious how you manage these at Shopify, since surely you've run into this. I've tried setting config.eager_load = true in test, but that seems to cause the warning to completely vanish, which isn't what I want. Right now I'm just whitelisting them one by one in allowed_deprecations, which works okay but is awkward and time-consuming.

For example, here's a fairly normal invocation of a deprecated method as of Rails 7.1:

class MyJob < ApplicationJob
  retry_on(StandardError, attempts: 25, wait: :exponentially_longer) do |job, _exception|
    job.on_permanent_failure
  end
  ...
end

And now running a handful of tests, the deprecation lands on whichever test first references that job:

Error:
MyTest#test_call:_does_thing_one:
DeprecationToolkit::Behaviors::DeprecationIntroduced: You have introduced new deprecations in the codebase. Fix or record them in order to discard this error.
You can record deprecations by adding the `--record-deprecations` flag when running your tests.

******* The following deprecations were added: *******
DEPRECATION WARNING: `wait: :exponentially_longer` will actually wait polynomially longer and is therefore deprecated. Prefer `wait: :polynomially_longer` to avoid confusion and keep the same behavior. (called from ...)
******************************************************

    /usr/local/bundle/gems/deprecation_toolkit-2.2.4/lib/deprecation_toolkit/behaviors/raise.rb:17:in `trigger'
    /usr/local/bundle/gems/deprecation_toolkit-2.2.4/lib/deprecation_toolkit/test_triggerer.rb:11:in `trigger_deprecation_toolkit_behavior'
    /usr/local/bundle/gems/deprecation_toolkit-2.2.4/lib/deprecation_toolkit/minitest_hook.rb:8:in `trigger_deprecation_toolkit_behavior'

Error:
MyTest#test_call:_does_thing_two:
DeprecationToolkit::Behaviors::DeprecationRemoved: You have removed deprecations from the codebase. Thanks for being an awesome person.
The recorded deprecations needs to be updated to reflect your changes.
You can re-record deprecations by adding the `--record-deprecations` flag when running your tests.

****** The following deprecations were removed: ******
DEPRECATION WARNING: `wait: :exponentially_longer` will actually wait polynomially longer and is therefore deprecated. Prefer `wait: :polynomially_longer` to avoid confusion and keep the same behavior.
******************************************************

    /usr/local/bundle/gems/deprecation_toolkit-2.2.4/lib/deprecation_toolkit/behaviors/raise.rb:17:in `trigger'
    /usr/local/bundle/gems/deprecation_toolkit-2.2.4/lib/deprecation_toolkit/test_triggerer.rb:11:in `trigger_deprecation_toolkit_behavior'
    /usr/local/bundle/gems/deprecation_toolkit-2.2.4/lib/deprecation_toolkit/minitest_hook.rb:8:in `trigger_deprecation_toolkit_behavior'

I did take advantage of #26 and have added this at the top of my test_helper to try to catch things as early as possible, but it doesn't seem perfect, and at any rate would not be able to record things caught outside of a test:

# test_helper.rb
require "deprecation_toolkit"
DeprecationToolkit.add_notify_behavior
DeprecationToolkit.attach_subscriber

[Edouard-chin]

Hello, thanks for reporting the issue. It's indeed slightly similar as #20.
I'm not sure if we have had this problem at Shopify, at least we don't have it right now.

The whole assumption of the toolkit was that the deprecations thrown would be deterministic each time a test runs. I haven't thought of class level deprecations which makes me think that the toolkit should have a separate "global" deprecation file for deprecations that will get triggered only once and can't be tied to a specific test.

I'll have to explore how to implement this feature "if a deprecation is triggered while a constant is being loaded, then ...". I think one of the way to solve this problem could be to enable a Tracepoint in toolkit, something like (writing it down for my future me as well):

Tracepoint.trace(:class, :end) do |tp|
  if @tp.event == :class
    DeprecationToolkit.class_level_deprecation = true
  else
    DeprecationToolkit.class_level_deprecation = false
  end
end

# Now the deprecation toolkit will be aware if a deprecation is thrown when a const is loaded and
# we can implement whatever logic we want.

---

One of the tricky bit is that the toolkit has a feature to compare the deprecations recorded and the one actually thrown. This allow to warn users when the list of recorded deprecations is outdated, either because deprecations were removed (DeprecationRemoved error) from the code or added (DeprecationAdded).

The challenging part is about the DeprecationRemoved, because we can only do the comparison of the recorded list at the very end of the whole test suite (after we are sure that all deprecations will be thrown). This is problematic for CI test suites that run on multiple machines, as we'll only compare the deprecations triggered for a subset of the tests ran on one machine.

[iangreenleaf]

Here's another idea I had that might be simpler and more generally useful: what if you offered a method that takes a string "name" and a block, captures any deprecations emitted by the block, and stores them in a file identified by the name? With that, I could solve my issue with something like this in my test helper:

# gets stored in test/deprecations/class_loading_deprecations.yml
DeprecationToolkit.capture "class loading deprecations" do
  Zeitwerk::Loader.eager_load_all
end

This would be fairly flexible and could be used for other purposes too, which would be nice. It's sort of a general solution to the problem of capturing deprecations for the gem from outside a test case.

[Edouard-chin]

Thanks for the idea, I think this make sense. For Rails application we can provide a Railtie that will do this automatically.

It doesn't solve the problem when eager loading is disabled, or for deprecations thrown in modules that don't get eager loaded. but I think it's a good addition and we can probably piggy back on top of it later.

[iangreenleaf]

Yep, it's not necessarily a perfect solution, but it at least gives gem users the flexibility to figure out a solution that works for their situation.