`expect(...).not_to be == ...` passes when it should fail, when certain exceptions are raised

[tomdalling]

Subject of the issue

The operator form of be swallows ArgumentError and NoMethodError exceptions, so that it can fail with a nicer error message. However, this means that expect(...).not_to be behaves as if those exceptions are intended, resulting in passing tests when those exceptions should have caused a failure.

I have traced this back to BeComparedTo#matches? introduced in PR #972 to fix issue #971.

My first thought was to workaround this with RSpec::Matchers.define_negated_matcher :not_be, :be but that results in:

ArgumentError:
  The expect syntax does not support operator matchers, so you must pass a matcher to `#to`.

Your environment

• Ruby version: 2.7.1
• rspec-expectations version: 3.9.2

Steps to reproduce

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.9.0" # Activate the gem and version you are reporting the issue against.
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

class Cranky
  def ==(whatever)
    self.problem == whatever
  end
end

RSpec.describe Cranky do
  it 'defines equality based on `problem`' do
    expect(Cranky.new).not_to be == 5
  end
end

Expected behavior

F

Failures:

  1) Cranky defines equality based on `problem`
     Failure/Error: self.problem == whatever

     NoMethodError:
       undefined method `problem' for #<Cranky:0x00007ff81e13fbc8>
     # tmp.rb:21:in `=='
     # tmp.rb:27:in `block (2 levels) in <main>'

Finished in 0.00174 seconds (files took 0.0622 seconds to load)
1 example, 1 failure

Failed examples:

rspec tmp.rb:26 # Cranky defines equality based on `problem`

Actual behavior

.

Finished in 0.0031 seconds (files took 0.06511 seconds to load)
1 example, 0 failures

[pirj]

I have an idea for a quick fix.

[JonRowe]

This is a deliberate behaviour, and thus cannot be changed as a bugfix.

In this case, it is not == to as an error was raised so the assertion is correct, if you wish to assert on a specific result of == you could use expect(Cranky.new == 5).not_to eq true, which is more explicit.

Personally I'm leaning towards closing this as "won't fix" but I'm willing to hear arguments for why this behaviour should be changed in a future major release.

[tomdalling]

The main argument is that it results in false positives — the implementation raises unhandled exceptions, but the tests pass.

In my specific case, I was testing an implementation of #===, something like:

it "matches integers" do
  expect(subject).to be === 5
end

it "does not match strings" do
  expect(subject).not_to be === '5'
end

The implementation was broken due to a typo in a method name, something like:

def ===(obj)
  obj.is_a?(Integer) || typod_method_here(obj)  
end

The tests passed, but it was crashing in development. I thought I was going crazy, because I had tests to prove that it was working correctly. Eventually I was able to determine that something was wrong with the test, walked up the stack trace in the debugger, and found RSpec swallowing the NoMethodError caused by the typo. That was very unexpected behaviour for me, and I've been using RSpec for a long time, so I just assumed that it must be a bug.

[JonRowe]

Ok thats a fair point, the matcher should fail when negated, thats the point of catching the error in terms of behaviour of the matcher (that its a failure).

[JonRowe]

1208 preserves the rescue behaviour, which I still think should stay, but will cause the matcher to fail when encountering errors in the negated matcher too.

[tomdalling]

Looks good. That was fast!

[JonRowe]

A potential followup improvement if anyone wanted to tackle it, is to capture that an exception happened, so that it can be reported in the failure message, something like:

but `NameError` was encountered whilst calling `==`