Skip to content

Configure RSpec to fail on potential false positives

Peter Leitzen requested to merge pl-rspec-false-positive-raise into master

What does this MR do and why?

Previously RSpec only warned in the following cases:

   expect(foo).to raise_error

   expect(foo).not_to raise_error(SomeSpecifcError)

and suggested to correct potential false positives with:

   expect(foo).to raise_error(SomeSpecifcError)

   expect(foo).not_to raise_error

This commit corrects all warnings and make RSpec fail instead of warn in such situations preventing future false positives.

⚠ The following false positives were found:

Screenshots or screen recordings

Before

WARNING: Using `expect { }.not_to raise_error(SpecificErrorClass)` risks false positives, since literally any other error would cause the expectation to pass, including those raised by Ruby (e.g. `NoMethodError`, `NameError` and `ArgumentError`), meaning the code you are intending to test may not even get reached. Instead consider using `expect { }.not_to raise_error` or `expect { }.to raise_error(DifferentSpecificErrorClass)`. This message can be suppressed by setting: `RSpec::Expectations.configuration.on_potential_false_positives = :nothing`. Called from /home/peter/devel/gitlab/gdk/gitlab/spec/models/clusters/platforms/kubernetes_spec.rb:497:in `block (4 levels) in <top (required)>'.

After

  1) Clusters::Platforms::Kubernetes#calculate_reactive_cache_for when kubernetes responds with 500s does not raise kubeclient http error
     Failure/Error: expect { subject }.not_to raise_error(Kubeclient::HttpError)

     ArgumentError:
       Using `expect { }.not_to raise_error(SpecificErrorClass)` risks false positives, since literally any other error would cause the expectation to pass, including those raised by Ruby (e.g. `NoMethodError`, `NameError` and `ArgumentError`), meaning the code you are intending to test may not even get reached. Instead consider using `expect { }.not_to raise_error` or `expect { }.to raise_error(DifferentSpecificErrorClass)`. This message can be suppressed by setting: `RSpec::Expectations.configuration.on_potential_false_positives = :nothing`

How to set up and validate locally

Run specs.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Peter Leitzen

Merge request reports