Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Displays wrong failure message for `should validate_presence_of` #295

Merged
merged 2 commits into from May 31, 2013

Conversation

Projects
None yet
3 participants
Contributor

pbrisbin commented May 24, 2013

Maybe I'm being really dumb, but I've tried looking at this 12 different ways and I keep coming to the same conclusion, which is that Shoulda is displaying the wrong test failure message for validate_presence_of.

I am doing this is my test:

class ThingTest < ActiveRecord::TestCase
  should validate_presence_of(:name)
end

I have not yet done validates :name, presence: true on the model, so my Thing model looks like this:

class Thing < ActiveRecord::Base
  attr_accessible :name
end

When I run my tests, I get this test failure message:

test: Thing should require name to be set. (ThingTest)
Did not expect errors to include "can't be blank" when name is set to nil, got errors:

But shouldn't that second line read something along the lines of:

Expected errors to include "can't be blank" when name is set to nil, got errors:

Am I missing something?

I'm using shoulda 3.5.0, shoulda-context 1.1.1, and shouda-matchers 2.1.0. I set up a repo that demonstrates this behaviour: https://github.com/toasterlovin/shoulda-bug

Same issue here:

Failure/Error: it { should validate_presence_of(:institution) }
       Did not expect errors to include "can't be blank" when institution is set to nil, got error:

I don't remember this being a problem in earlier versions of shoulda-matchers. Regression? I'm using Ruby 2.0.0-p0 and shoulda-matchers 2.1.0.

pbrisbin added some commits May 24, 2013

Don't swap failure messages in ValidationMatcher
It's not entirely clear why these were being swapped to begin with, but
it was causing messages to appear backwards in the presence validation
matcher.

Other matchers using these methods were tested by hand and still output
correct messages -- they too may have been backwards before this fix.
Contributor

pbrisbin commented May 24, 2013

Inside ValidationMatcher, the messages for should and should not were being swapped during the match. It's not entirely clear why this is but it seemed to be why the presence matcher was breaking.

The suite still passes, but there's not much testing of matcher messages present, so I found those that share this code and checked them by hand in an example project.

Acceptance, confirmation, exclusion, and uniqueness all still work.

There are some weird messages with negative expectations for numericality and inclusion of, but I'm told we don't support should_not in those cases anyway (which makes sense).

Collaborator

mcmire commented May 27, 2013

Looks good to me :shipit:

@pbrisbin pbrisbin merged commit a1afc57 into master May 31, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment