Rails Ruby on Rails

Comments and changes to this ticket

• 

  Tarmo Tänav May 5th, 2008 @ 09:21 PM

  Why not make the messages parameter optional and not assert the specific contents of the validation errors unless it is provided?

• You flagged this item as spam.
  

  Nathan Wilmes May 5th, 2008 @ 11:38 PM

  • Title changed from “[patch] Add assert_not_valid to model assertions” to “Add assert_not_valid to model assertions”

  I don't have a strong problem with that. I'd argue that this function is most useful as a way to confirm which errors you've received, but you could certainly use it for a more basic valid/not valid checker.

• 

  Tarmo Tänav May 6th, 2008 @ 12:08 AM

  A couple of more questions:

  1. Why not "assert_invalid"?

  2. Have you thought about supporting partial error messages checks? Basically, if you have a test for a particular validation then would it not make sense to only check for the presence of that validation error and not pollute the test for one validation with the failure of some other unrelated validation? Though I have no idea what kind interface changes this may require in order to also support the current messages=expected_messages case.

• You flagged this item as spam.
  

  Nathan Wilmes May 6th, 2008 @ 12:20 AM

  1. assert_not_valid opposes assert_valid more closely.. we were attempting to mimic the naming convention of assert_not_equal/assert_equal.

  2. I've thought about it, but as you said, it's easiest to support either partial or exact matches, not both. The exact match format has been more rewarding for us at Pivotal.. when unrelated validations muck up the works, we typically want to know about them so that we can start adjusting the validations accordingly.

• 

  Tarmo Tänav May 6th, 2008 @ 12:52 AM

  1. I agree

  2. I agree, though I still think that if there is a all-in-one assertion helper for the "test more than needed, just in case" people there should be an equivalent for those who want to test one thing and one thing only.

  Anyway, about the not passing any messages, are you going to add this to the patch? If so, it's a +1 from me.

  (btw. besides not running the messages assertion if the messages parameter is blank the assertion failure message for the validation check would also need a different text as it can't really report what errors it was expecting if the errors are not specified)

  Thank you,

• You flagged this item as spam.
  

  Nathan Wilmes May 6th, 2008 @ 05:38 PM

  Ok, here's the updated patch that makes additional parameters optional.

  • add_assert_not_valid.diff 7.8 KB
• 

  Tarmo Tänav May 6th, 2008 @ 05:48 PM

  Thank you. Is there a good reason for not squishing the commits in the patch?

• You flagged this item as spam.
  

  Nathan Wilmes May 6th, 2008 @ 06:23 PM

  No good reason... I've just started using git, so I'm still working out how to do some of these kinds of operations.

• 

  josh May 8th, 2008 @ 06:39 AM

  • State changed from “new” to “invalid”
  • Assigned user set to “josh”

  I think we saw assert_invalid before, and it was turned down.

  We already trimmed out a bunch of superfluous assert_* response helpers. When there is a helper for just about every case, it becomes so hard to remember to use them.

  assert_equal company.errors.on(:email), "can't be blank"