Predicate matcher always passes when chained on expect change #276

Closed
cupakromer opened this Issue Jun 28, 2013 · 4 comments

Comments

Projects
None yet
3 participants
@cupakromer
Member

cupakromer commented Jun 28, 2013

A very similar issue was brought up in #161. I tested both on 2.14.0.rc1 and 2.13.1.

When chaining the be_xxxx matcher on the expect change the test always passes.

Example Issue

Here's a very simple and contrived example which clearly shows the false positive result:

it 'incorrectly passes the spec' do
  hash = { a: 1 }

  expect{ hash[:a] = 2 }.to change{ hash }.to be_empty
end

A more typical example may be something like:

expect{ obj.command! }.to change{ obj.query }.to be_available

Inconsistent behavior

However, the be matchers (be_true and be_false) work as expected, though the error message could be improved a tad.

it 'correctly fails the spec' do
  hash = { a: 1 }

  expect{ hash.clear }.to change{ hash.empty? }.to be_false
end

The be_within matcher also works as expect:

it 'correctly fails the spec' do
  expect{ record.save }.to change{ record.created_at }
    .from(nil).to( be_within(1.second).of(Time.now) )
end

Workarounds

There are several workaround for this currently:

Move the predicate check into the change block

it 'fails as expected' do
  hash = { a: 1 }

  expect{ hash.clear }.to change{ hash.empty? }.to false
end

The above tests was modified slightly so that the result of the change block is accurate. Also, instead of .to false it could also have been written .to be_false and the test would still have failed as expected, stating it was now true.

Do not use expect change

it 'also fails as expected' do
  hash = { a: 1 }

  expect(hash.empty?).to be_false
  hash[:a] = 2
  expect(hash.empty?).to be_true
end
@samphippen

This comment has been minimized.

Show comment
Hide comment
@samphippen

samphippen Jun 28, 2013

Member

I've got a patch that will make this fail but the error message is AWFUL.

Member

samphippen commented Jun 28, 2013

I've got a patch that will make this fail but the error message is AWFUL.

@cupakromer

This comment has been minimized.

Show comment
Hide comment
@cupakromer

cupakromer Jun 28, 2013

Member

@samphippen the error message for most of the other matchers when chained on change are pretty awful; this holds true for be_false, be_true, and be_within.

Member

cupakromer commented Jun 28, 2013

@samphippen the error message for most of the other matchers when chained on change are pretty awful; this holds true for be_false, be_true, and be_within.

@cupakromer

This comment has been minimized.

Show comment
Hide comment
@cupakromer

cupakromer Jun 28, 2013

Member

Updated the description to express this really is the predicate matches using (be_predicate) and not the core be matchers such as be_true.

Member

cupakromer commented Jun 28, 2013

Updated the description to express this really is the predicate matches using (be_predicate) and not the core be matchers such as be_true.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Jul 12, 2013

Member

Closing since @samphippen fixed this in #277. We're also discussing a more complete solution in #280.

Member

myronmarston commented Jul 12, 2013

Closing since @samphippen fixed this in #277. We're also discussing a more complete solution in #280.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment