Absolute Path vs Relative Path on deprecations

[sobrinho]

Hi there!

When recording deprecations as suggested on https://about.gitlab.com/blog/2021/02/03/how-we-automatically-fixed-hundreds-of-ruby-2-7-deprecation-warnings/ we are seeing recordings like:

-  DEPRECATION WARNING: /Users/sobrinho/Developer/my-app/models/concerns/logable.rb:36: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call

But when running on CI, we have:

+  DEPRECATION WARNING: /app/app/models/concerns/logable.rb:36: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call

Which leads to a DeprecationToolkit::Behaviors::DeprecationMismatch failure.

I'm currently fixing those with a dirty sed:

find spec/deprecation -type f | xargs gsed -i 's/.Users.sobrinho.Developer.my-app./\/app\//g'

Does it worth to have a way of replacing paths/values from deprecations or at least mention how on README?

[sobrinho]

Another way:

class DeprecationToolkit::Collector
  def initialize(deprecations)
    # This will remove the absolute paths from the deprecations since it varies
    # from environment to environment.
    #
    # See https://github.com/Shopify/deprecation_toolkit/issues/70
    self.deprecations = deprecations.map do |deprecation|
      deprecation
        .gsub(Gem.dir.to_s, "[GEM_DIR]")
        .gsub(Rails.root.to_s, "[RAILS_ROOT]")
    end
  end
end

[rafaelfranca]

What is adding paths to the deprecation message? Active Support deprecation doesn't do it

[rafaelfranca]

Ah, it is the message Ruby generates.

We ignore stacktraces when writing and comparing deprecations (for deprecation using ActiveSupport::Deprecation). It seems the fact that the location of the deprecation is on the Ruby warning message being recorded is just a mistake from our part when we implemented that.

But, if we remove it from the recorded deprecation, like we do with ActiveSupport::Deprecation, the approach used in that blog post doesn't work anymore.

Tough position to be since I want to allow automation, but I also want to match the same behavior as ActiveSupport::Deprecation which doesn't allow automation right now.

I think we should really think on how to allow automation in both contexts in order to fix this issue properly.

[sobrinho]

The code snippet is working fine, probably allowing to change the collector (to avoid the monkey patch) or either allowing to scrub data from the deprecation messages would work (if do not want to couple to Rails, for instance, even though we could loose couple with a if defined? Rails or something).

[henrahmagix]

Would this be solved by #60, if that were in a releasable state?

[brkn]

I had to add 3rd gsub to handle psych deprecation warnings

# At spec/support/deprecation_toolkit.rb

# frozen_string_literal: true

require 'deprecation_toolkit'
require 'deprecation_toolkit/rspec'

# Monkeypatch the DeprecationToolkit
# This will remove the absolute paths from the deprecations since it varies from environment to environment.
# See https://github.com/Shopify/deprecation_toolkit/issues/70
module DeprecationToolkit
  class Collector
    def initialize(deprecations)
      self.deprecations = deprecations.map do |deprecation|
        deprecation
          .gsub(Gem.dir.to_s, '[GEM_DIR]')
          .gsub(Rails.root.to_s, '[RAILS_ROOT]')
          .gsub(RbConfig::CONFIG['rubylibdir'].to_s, '[RUBY_LIBS_DIR]') # For default ruby gems such as psych
      end
    end
  end
end

DeprecationToolkit::Configuration.test_runner = :rspec
DeprecationToolkit::Configuration.deprecation_path = 'spec/fixtures/deprecations'

# Empty regex matches every warning to be treated as a deprecation error
DeprecationToolkit::Configuration.warnings_treated_as_deprecation = [//]