Warning[:deprecated] = true by default in RSpec

[eregon]

Subject of the issue

See https://bugs.ruby-lang.org/issues/17591.

The plan of ruby-core hiding deprecation warnings in Ruby 2.7.2 is that test frameworks, like rspec, should enable them by default (with Warning[:deprecated] = true if Warning.respond_to?(:[]=), which only enables deprecation warnings).
This is important so that developers see warnings in development, and that they see the warnings before updating to the next Ruby version.

Of course, RSpec users can still choose to disable deprecation warnings explicitly if they really want with Warning[:deprecated] = false (but then they should not be surprised when it breaks hard when updating to a newer Ruby, that's the cost of ignoring deprecation warnings).

Somewhat related to #2809.

Your environment

• Ruby version: 2.7.2
• rspec-core version: 3.10.1

Steps to reproduce

# some_spec.rb 
def foo(**kwargs)
  kwargs
end

RSpec.describe 'Test' do
  it "should warn about keyword arguments" do
    expect(foo({a: 1})).to eq({a: 1})
  end
end

rspec some_spec.rb

Expected behavior

$ rspec some_spec.rb
/home/eregon/code/rspec-ws/some_spec.rb:7: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/home/eregon/code/rspec-ws/some_spec.rb:1: warning: The called method `foo' is defined here
.

Finished in 0.00154 seconds (files took 0.05064 seconds to load)
1 example, 0 failures

Actual behavior

No warnings in 2.7:

$ rspec some_spec.rb
.

Finished in 0.00157 seconds (files took 0.05947 seconds to load)
1 example, 0 failures

Breaks hard in 3.0:

$ rspec some_spec.rb
F

Failures:

  1) Test should warn about keyword arguments
     Failure/Error:
       def foo(**kwargs)
         kwargs
       end
     
     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./some_spec.rb:1:in `foo'
     # ./some_spec.rb:7:in `block (2 levels) in <top (required)>'

Finished in 0.00121 seconds (files took 0.05637 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./some_spec.rb:6 # Test should warn about keyword arguments

[JonRowe]

For RSpec 3.x I won't change the default, I'd consider that a breaking change. If there was overwhelming support for it in 4.x I might consider making it the default as long as it was suppressible, but I'd prefer it remained an opt in behaviour (because there are very verbose libraries out there) which we already allow (with --warnings, which is supported via .rspec etc).

An alternative I'd happily consider, is to place it into our generated spec helper, with a comment explaining what it is. This has the effect of effectively being on by default (as most new projects end up using our default spec helper from what I've seen), but also explaining how to not have it for verbose projects without having to look it up.

[eregon]

I think you are confusing with ruby -w/$VERBOSE = true.
This is only about deprecation warnings.
Such warnings if ignored pretty much guarantee the library will not work and break on the next Ruby version.
I'd bet most gem maintainers wouldn't want that, they'd rather be warned in advance if something is going to change in an incompatible way, that's the whole purpose of these warnings.

For instance, it doesn't make much sense to test a gem on 2.7 but not address or even see the keyword arguments deprecation warnings.
Also, this would have no effect on Ruby < 2.7, since Warning#[]= does not exist before 2.7.

In Ruby <= 2.6, deprecation warnings would typically be shown even with $VERBOSE = false (the default), so in one way the breaking change would be to not show them on 2.7+ with RSpec.

because there are very verbose libraries out there

Do you have examples that actually emit many deprecation warnings and support Ruby 2.7?

to place it into our generated spec helper

It would only apply to new projects though, which seems a big drawback.

Finally I'd like to highlight this is not a personal request but part of the plan of ruby-core for deprecation warnings in Ruby 2.7+, as mentioned on the ruby-core issue.
The fact that test-unit also did this change is IMHO a good motivation for the other popular test frameworks to consider it.

[eregon]

For more background on the change in 2.7.2, I think this comment is helpful: https://bugs.ruby-lang.org/issues/16345#note-31
The idea is: deprecation warnings are useless for end users, but important for developers. The chosen way to address that is to hide them by default, and enable them by default in development (test frameworks, REPLs, etc).

[JonRowe]

I think you are confusing with ruby -w/$VERBOSE = true.
This is only about deprecation warnings.

I am not confused, it is not up to us to decide to emit deprecation warnings, if Ruby makes that choice, that is there decision, but they got push back and added this option to turn them on at a users discretion.

For instance, it doesn't make much sense to test a gem on 2.7 but not address or even see the keyword arguments deprecation warnings.

But that is not our decision to make for other libraries. We already check that RSpec itself produces no warnings, it's even part of our spec suite.

Do you have examples that actually emit many deprecation warnings and support Ruby 2.7?

Rails projects where the majority of code lives upstream in gems of various ages.

It would only apply to new projects though, which seems a big drawback.

It prevents us making a breaking change and allowing to introduce it earlier.

Finally I'd like to highlight this is not a personal request but part of the plan of ruby-core for deprecation warnings in Ruby 2.7+, as mentioned on the ruby-core issue.

They made this opt in after push back by the community about the level of deprecation warnings, that same push back applies here, we are following the Ruby community here.

[eregon]

I am not confused,

I guessed that because you mentioned --warnings, -w and warnings in general, which are basically unrelated to this issue.

They made this opt in after push back by the community about the level of deprecation warnings, that same push back applies here, we are following the Ruby community here.

That's a simplistic view of it. It was made opt-in because for end users (!= developers) the deprecation warnings cannot be acted on.
For developers, the warnings should be on by default, I believe ruby-core agrees to that without a doubt.
RSpec is a clear case of being run in development, by developers, as such it's important to enable deprecation warnings, which BTW are already shown by default in Ruby <= 2.6.

If you still disagree, how about asking RSpec users or gem maintainers using RSpec about it?

@mame @jeremyevans Could you share your opinion here?

[eregon]

BTW, part of my motivation to fix this is I've seen many gems that try to be compatible with Ruby 3, often do it incorrectly, and they probably completely missed the deprecation warnings of 2.7, which could have been useful to migrate earlier or more easily. Due in part to test frameworks not showing these important warnings by default.

[jeremyevans]

My opinion is that Ruby should always emit deprecation warnings, and software designed for end users should use $VERBOSE = nil. I think it was a mistake to change Ruby to not emit deprecation warnings by default.

@eregon is right that ruby-core expected that test library developers would enable deprecation warnings by default. If RSpec doesn't want to do that, considering RSpec's popularity, we (ruby-core) should consider reverting the change in Ruby and enable deprecation warnings by default.

[pirj]

It's not up to us to ease the migration of the code that is outside of our control.
We'll do everything that is possible to properly delegate in our code.
Even if it was a mistake to turn warnings off I'm Ruby 2.7.2, it would be a strange move to turn them back on.

[JonRowe]

I guessed that because you mentioned --warnings, -w and warnings in general, which are basically unrelated to this issue.

Deprecations are warnings. I mentioned RSpec's own --warning mode which includes setting Warning[:deprecated] = true a change stemming from an earlier issue you yourself opened.

That's a simplistic view of it. It was made opt-in because for end users (!= developers) the deprecation warnings cannot be acted on.

I saw developer push back about it on the Ruby issue tracker, as I am not privy to the internal debates of the Ruby team I took that to be the cause of the subsequent change, I apologise for not investigating further at the time, but I've yet to come across a user of Ruby who is not a developer.

@eregon is right that ruby-core expected that test library developers would enable deprecation warnings by default. If RSpec doesn't want to do that, considering RSpec's popularity, we (ruby-core) should consider reverting the change in Ruby and enable deprecation warnings by default.

It would have been an option for us to do that at the release of Ruby 2.7 had someone reached out and made that expectation, we could have flagged this mode for Ruby 2.7 only and turned it on. However it is now a breaking change, in that previous Ruby code bases that passed without warnings would suddenly gain warnings from a point release of RSpec.

I'm happy to discuss turning this on in RSpec 4.

[eregon]

Actually up to and included Ruby 2.7.1, deprecation were shown by default.
So if RSpec did this, I see it as no breaking change but rather a continuation of existing behavior (it was like that up to Ruby 2.7.1) and I would consider it a help in development and for testing (e.g., might save a lot of time to understand why some specs don't work).
I do see it might actually break CI for someone when updating RSpec though, if they test on Ruby 2.7.2 or 3.0, they check stderr output and still have deprecation warnings in the tested code.

It would have been an option for us to do that at the release of Ruby 2.7 had someone reached out and made that expectation

Yes, I think such issues should have been opened right after the decision was made to disable deprecation warnings by default in Ruby 2.7.2. Unfortunately it seem committers at the time forgot or did not think about it (I guess they might have been busy with the 2.7.2 release).

I think RSpec 4 would be a fine target for this change if it's deemed too incompatible in RSpec 3.

[pirj]

@JonRowe I can work on this for RSpec 4, ok to tag this ticket as such?
Should I include an opt-out configuration option? Any suggestion for the name? config.suppress_ruby_2_7_2_deprecation_warnings!?

[JonRowe]

We have a couple of options either it could just be a true/false config that is used to decide wether we call Warning[:deprecated] = true or not before a spec run or not but then theres an argument for where we turn it on, wether it should be before spec file loading (which would be my vote) or before specs run. We could defer that decision to the user however, and default to before spec files, e.g. something like:

def ruby_deprecation_warnings=(value)
  @ruby_deprecation_warnings =
    case value
      when -> x { [:both, :on_spec_load, :on_spec_run].include?(x) } then value
      when true then :on_spec_load
    else
      :none
    end
end

[eregon]

FWIW, RSpec is the only popular test framework still not showing deprecation warnings to its users (https://bugs.ruby-lang.org/issues/17591). That means it makes it much harder for RSpec users to notice deprecated methods, and most likely they won't notice until something breaks hard, effectively destroying a significant part of ruby-core's efforts to have a nice transition path with deprecation, etc.

Yes, it's unfortunate this wasn't asked immediately after 2.7.2 came out, but the current situation is problematic.

[eregon]

Maybe we should think wider how to address this and be compatible enough. It has been two years.
I'm thinking the next release of RSpec 3 could maybe print a single line asking if one wants to enable deprecation warnings or not, and then when set that line would not be printed.
Like:

[rspec] this spec suite does not specify if deprecation warnings should be printed. Set it via `config.ruby_deprecation_warnings = true/false`.
[rspec] It is recommended to enable them to address such warnings in time, before it breaks hard.

Or maybe a message as gem/bundle install time suggesting to enable deprecation warnings and showing how.

Any thoughts on that?

[JonRowe]

I'll happily review a PR with a config option that sets this, and prints a warning if its not set for Rubies that support it; the message should specify how to remove the warning, and the config option should support just silencing the warning without turning it on.

I will not support adding an install time message, I do not wish to party hard in this project.