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

weird error information #207

Closed
lidaobing opened this issue Dec 20, 2012 · 16 comments
Closed

weird error information #207

lidaobing opened this issue Dec 20, 2012 · 16 comments

Comments

@lidaobing
Copy link

  1) MaterialOptionImage validations 
     Failure/Error: it { should validate_uniqueness_of(:material_image_id).scoped_to(:material_option_id) }
       Expected errors to include "has already been taken" when material_image_id is set to 1, got errors: ["material_image_id has already been taken (1)"] (with different value of material_option_id)
     # ./spec/models/material_option_image_spec.rb:16:in `block (3 levels) in <top (required)>'

for the error information, I think this test should be passed, but it does not pass.

@gabebw
Copy link

gabebw commented Dec 20, 2012

Do you have internationalized errors? Have you ever added anything in config/locales?

@lidaobing
Copy link
Author

it's the same error after I remove config/locales/en.yml

  1) MaterialOptionImage validations 
     Failure/Error: it { should validate_uniqueness_of(:material_image_id).scoped_to(:material_option_id) }
       Expected errors to include "has already been taken" when material_image_id is set to 1, got errors: ["material_image_id has already been taken (1)"] (with different value of material_option_id)
     # ./spec/models/material_option_image_spec.rb:16:in `block (3 levels) in <top (required)>'

@gabebw
Copy link

gabebw commented Dec 20, 2012

If you do a test without using shoulda-matchers, does it pass?

Try creating a record, then creating a second record that violates the uniqueness constraint and checking object.errors.full_messages on the second one. Do the expected errors show up?

@lidaobing
Copy link
Author

[13] pry(main)> MaterialOptionImage.last.dup.tap(&:valid?).errors.full_messages
  MaterialOptionImage Load (0.3ms)  SELECT `material_option_images`.* FROM `material_option_images` ORDER BY `material_option_images`.`id` DESC LIMIT 1
  MaterialOption Load (0.3ms)  SELECT `material_options`.* FROM `material_options` WHERE `material_options`.`id` = 8 LIMIT 1
  MaterialImage Load (0.3ms)  SELECT `images`.* FROM `images` WHERE `images`.`type` IN ('MaterialImage') AND `images`.`id` = 255 LIMIT 1
  MaterialOptionImage Exists (0.2ms)  SELECT 1 AS one FROM `material_option_images` WHERE (`material_option_images`.`material_image_id` = BINARY 255 AND `material_option_images`.`material_option_id` = 8) LIMIT 1
=> ["Material image has already been taken"]

@xaethos
Copy link

xaethos commented Mar 28, 2013

Just looking at the error messages it seems clear that the matcher is looking for the short message in the object.errors.full_messages array. Shouldn't the matcher be looking instead in object.errors[:material_image_id]?

@mxie
Copy link
Contributor

mxie commented Apr 4, 2013

Could someone put together a barebones repo that would reproduce this issue? It will make it easier for us to investigate. Thanks!

@ivanvc
Copy link
Contributor

ivanvc commented Apr 26, 2013

I have exactly this same issue, I'm not sure if it's worth setting up a barebones repo, since my app has this matcher called in several places and it's just failing in one place. This is what I'm getting:

  1) Hourly
     Failure/Error: it { should validate_uniqueness_of(:timestamp).scoped_to(:source_id) }
       Expected errors to include "has already been taken" when timestamp is set to Fri, 26 Apr 2013 18:00:00 UTC +00:00, got errors: ["timestamp has already been taken (Fri, 26 Apr 2013 18:00:00 UTC +00:00)"] (with different value of source_id)

Hourly has the following validation:

class Hourly < ActiveRecord::Base
  ...
  validates :timestamp,
    uniqueness: { scope: :source_id },
    presence: true
  ...
end

I've tried removing the presence validation, and deleting locale files with no luck . As I mentioned, there are other instances in the app where this matcher is working. Just updated to shoulda 3.4.0, and still have the same problem. Not sure what else can I add to this issue.

@drapergeek
Copy link
Contributor

Please try using verison 2.1.0 and if this is still a failure please let us know. Thanks.

@opsdeveloper
Copy link

yes, it's still there with 2.1.0

@ivanvc
Copy link
Contributor

ivanvc commented May 10, 2013

As @opsdeveloper said, it's still present...

@mcmire
Copy link
Collaborator

mcmire commented May 10, 2013

Hey @ivanvc, I am not able to reproduce this with a fresh Rails 3 app using your example. Here is the test app that I set up -- is this what you are doing?

@ivanvc
Copy link
Contributor

ivanvc commented May 10, 2013

Hi @mcmire, I know it's odd, but as said before in some models that problem is occurring, and in others not. This weekend will try to strip down my application in order to isolate the problem, and that would be a starting point.

@ivanvc
Copy link
Contributor

ivanvc commented May 14, 2013

Hi @mcmire, I've forked your repo (http://github.com/ivanvc/sm-207-test-app), and added some code in order to reproduce it. I've found that in my case the problem appears to be somehow related, to have some nested parent models. Hope it helps to fix it.

@allenwyma
Copy link

Also having this issue:

Expected errors to include "has already been taken" when date is set to Tue, 14 May 2013, 
got errors: ["organizable can't be blank (nil)", "organizable can't be blank (nil)", "note can't be blank (nil)", 
"note can't be blank (nil)", "date has already been taken (Tue, 14 May 2013)", 
"date has already been taken (Tue, 14 May 2013)"] (with different value of organizable_id)

But I'm scoping to a polymorphic relationship with 2 columns. Or should I use something like subject.organizable to set it's polymorphic attribute to the previously created one?

ivanvc added a commit to ivanvc/shoulda-matchers that referenced this issue May 17, 2013
Re-implement how to get previous value in order to test
validate_uniquenes_of matcher, after the scope changes. This way, new
value should not be taken by a previous record. Fixes thoughtbot#207.
@ivanvc
Copy link
Contributor

ivanvc commented May 17, 2013

After some investigation, I've found that the problem (that was manifesting in my fork of sm-207-test-app), relates to a comment from validate_uniqueness_of_matcher.rb. And the problem is that when setting the new scoped value, it's already taken by another record. That makes validate_after_scope_change? to return false, so the test fails.

I've fixed this issue, but not completely sure that is the best approach. Anyway, I'm creating a pull request right now, so you can review it.

@mcmire
Copy link
Collaborator

mcmire commented May 20, 2013

@ivanvc Ahhh... right. That makes sense. Thanks, I'll take a look.

ivanvc added a commit to ivanvc/shoulda-matchers that referenced this issue May 21, 2013
Re-implement how to get previous value in order to test
validate_uniquenes_of matcher, after the scope changes. This way, new
value should not be taken by a previous record. Fixes thoughtbot#207.
@mcmire mcmire closed this as completed in 2065a65 May 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants