Skip to content

Commit

Permalink
Deprecate assert_valid
Browse files Browse the repository at this point in the history
  • Loading branch information
@josh
josh committed Nov 25, 2008
1 parent 759183c commit d475467
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
1 change: 1 addition & 0 deletions actionpack/lib/action_controller/assertions/model_assertions.rb
View file
Expand Up @@ -11,6 +11,7 @@ module ModelAssertions
# assert_valid(model)
#
def assert_valid(record)
::ActiveSupport::Deprecation.warn("assert_valid is deprecated. Use assert record.valid? instead", caller)
clean_backtrace do
assert record.valid?, record.errors.full_messages.join("\n")
end
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/controller/action_pack_assertions_test.rb
View file
Expand Up @@ -461,14 +461,14 @@ def test_redirected_to_with_nested_controller

def test_assert_valid
get :get_valid_record
assert_valid assigns('record')
assert_deprecated { assert_valid assigns('record') }
end

def test_assert_valid_failing
get :get_invalid_record

begin
assert_valid assigns('record')
assert_deprecated { assert_valid assigns('record') }
assert false
rescue ActiveSupport::TestCase::Assertion => e
end
Expand Down

6 comments on commit d475467

@joshgoebel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is assert_valid being depreciated? I find it’s listing of validation errors to be very useful in testing.

assert @model.valid? is just going to show generically that true is not false, yes? Or am I missing something?

@stephencelis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yyyc514: assert takes another argument that is called for inspection when it fails. Something like this should work:

assert @model.valid?, @model.errors

@josh
Copy link
Contributor Author

@josh josh commented on d475467 Nov 26, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was supposed to be removed a while back, but we forgot about it.

@joshgoebel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I knew assert took another argument but I never though of using it in such a fashion… rather obvious in hindsight. :-) And easy enough to code one’s own assert_valid to do just that I suppose.

@dgm
Copy link
Contributor

@dgm dgm commented on d475467 Jun 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using assert @model.valid?, @model.errors doesn't print anything useful, as it just prints: #ActiveModel::Errors:0x007fedc685ea60

I still can't see why this was depreciated and removed.

@calebhearth
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

def assert_valid(record)
  assert record.valid?, record.errors.full_messages.join("\n")
end

Is probably a little more useful, and can be put into your test/test_helper.rb file.

I'm a little unclear on why this was removed as well, as it seems like it is a pretty useful feature.

Please sign in to comment.