Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails uniqueness validation with Shoulda Matchers #28

Closed
philtr opened this issue Aug 8, 2016 · 8 comments
Closed

Fails uniqueness validation with Shoulda Matchers #28

philtr opened this issue Aug 8, 2016 · 8 comments

Comments

@philtr
Copy link
Contributor

philtr commented Aug 8, 2016

Failing test case:

https://github.com/philtr/challah/blob/pr-shoulda/spec/models/user_spec.rb#L6-L8

@thewatts and I tested this, and even having Challah in the Gemfile causes the uniqueness matcher to fail.

@thewatts
Copy link
Contributor

thewatts commented Aug 8, 2016

Context:

my comment and subsequent comments and pip's comment

Background:

For some reason - shoulda/matchers validation of uniqueness is failing (just in specs) when challah is included in a project.

Example App:

https://github.com/thewatts/uniqueness

( models provided are User and Trait )

To Reproduce:

clone, bundle, db creation/migration, etc

run: bundle exec rspec spec -> all tests will pass.

Add challah to Gemfile and bundle.

run: bundle exec rspec spec -> uniqueness specs fail.

Failing Specs Details

.F..F

Failures:

  1) Trait should validate that :color is case-sensitively unique
     Failure/Error: it { should validate_uniqueness_of :color }
       Trait did not properly validate that :color is case-sensitively unique.
         After taking the given Trait, whose :color is ‹"red-2"›, and saving it
         as the existing record, then making a new Trait and setting its :color
         to ‹"red-2"› as well, the matcher expected the new Trait to be
         invalid, but it was valid instead.
     # ./spec/models/trait_spec.rb:7:in `block (2 levels) in <top (required)>'

  2) User should validate that :email is case-sensitively unique
     Failure/Error: it { should validate_uniqueness_of :email }
       User did not properly validate that :email is case-sensitively unique.
         After taking the given User, whose :email is ‹"user-3@example.com"›,
         and saving it as the existing record, then making a new User and
         setting its :email to ‹"user-3@example.com"› as well, the matcher
         expected the new User to be invalid, but it was valid instead.
     # ./spec/models/user_spec.rb:8:in `block (2 levels) in <top (required)>'

Finished in 0.06393 seconds (files took 1.51 seconds to load)
5 examples, 2 failures

Failed examples:

rspec ./spec/models/trait_spec.rb:7 # Trait should validate that :color is case-sensitively unique
rspec ./spec/models/user_spec.rb:8 # User should validate that :email is case-sensitively unique

@jdtornow
Copy link
Owner

jdtornow commented Aug 8, 2016

I'd be fine just removing that test, we can make a manual check in there that doesn't use the fancy matchers.

Is that what you guys are thinking?

@philtr
Copy link
Contributor Author

philtr commented Aug 8, 2016

@jdtornow That test case is one I just added to show that Challah is incompatible with the current Shoulda::Matchers.

Take a look at @thewatts' comment. Adding Challah to an otherwise working project causes it that uniqueness matcher to fail.

@thewatts
Copy link
Contributor

thewatts commented Aug 8, 2016

@jdtornow and to be more specific - we're not adding anything like challah/test to the rails_helper. Just adding challah to the Gemfile will cause the failures.

@jdtornow
Copy link
Owner

jdtornow commented Aug 8, 2016

@thewatts does 766da3f fix your issue? Try pointing your gemfile to github to check it before we push a patch fix.

@thewatts
Copy link
Contributor

thewatts commented Aug 8, 2016

@jdtornow that's the ticket!

I had pinpointed the issue in Challah::Audit, but had to jump on a call before I found the culprit. Ya beat me to the punch! 😀

Thanks for making the quick fix!


As a side note - if you know offhand what and why was causing the issue, I'd be super curious to know!

@jdtornow
Copy link
Owner

jdtornow commented Aug 8, 2016

Great, thanks for checking. I'll push up a patch version shortly.

I suspect the initialize_dup was failing within the matcher so it never duplicated the record, and therefore couldn't validate that a dup'd record was invalid.

@thewatts
Copy link
Contributor

thewatts commented Aug 8, 2016

@jdtornow thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants