rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

raise_errors_for_deprecations! doesn't let test fail #3106

Closed 0llirocks closed 3 months ago

0llirocks commented 3 months ago

Test with warning should fail

I am currently trying to let my test fail, if a warning from ruby is thrown. I assumed using _raise_errors_fordeprecations! would be the correct config, but I am not sure whether a warning is considered as deprecation. I also tried _raise_onwarning but as the comment from SO pointed out, this only applies to rspec warnings (if this is true, the documentation isn't 100% clear to me). Maybe this isn't an issue, but just a misunderstanding from my point of view. What would be the correct config to let my test fail?

Your environment

Steps to reproduce

.rspec

--warning

spec_helper.rb

$LOAD_PATH.unshift File.expand_path('../lib', __dir__)
require 'rspec/its'

RSpec.configure do |config|
   config.raise_errors_for_deprecations!
   config.raise_on_warning = true
   config.warnings = true
   config.expect_with :rspec do |c|
     c.syntax = :expect
   end
end

test_spec.rb

require_relative './spec_helper'

RSpec.describe 'Test' do
  it "test" do
    expect(''.concat('test')).to eq('test')
  end
end

Expected behavior

Test would fail.

Actual behavior

Test

/home/user/git/test_project/spec/test_spec.rb:5: warning: literal string will be frozen in the future

 test

Finished in 0.00121 seconds (files took 0.10739 seconds to load)

1 example, 0 failures
pirj commented 3 months ago

The config options RSpec provides changes the behaviour to raise if RSpec warns you about some deprecations, not just any part. So there’s no out of the box solution.

However, RSpec’s own spec suite is strict about warnings issued in other parts of the app, catches them all and fails examples if there were warnings. You are welcome to explore our spec suite to see how this is done.

JonRowe commented 2 months ago

Apologies, I agree that the docs are a bit confusing here, the --warning and config.warning = settings turn on a flag within Ruby called $VERBOSE which is a bit of a legacy handover as Ruby now tends to warn about things directly.

Our raise_on_warning flag conversley is intended for our own warnings. I've tweaked the documentation here to better reflect that.

If we could raise on Ruby warnings easily I would totally support that, but the problem is they are essentially output to std err without anyway (that I know of) to be notified of them. What @pirj is referring to above is what we do within our own test suite, we replace stderr with essentially a mock which allows us to error our own suite on any output to std err.

That is something we could expose publically as an opt in, but it would probably have to be quite explicit that its on any output to std err and not specifically Ruby warnings.

0llirocks commented 2 months ago

@JonRowe Thank you for the explanation and the improved documentation 👍