Skip to content

Commit

Permalink
Fix frozen string in validatable, use multiline string instead. (#5563)
Browse files Browse the repository at this point in the history
Expand tests to check for the actual validatable exception message

This was raising a `FrozenError` on Ruby < 3 where interpolated strings
were considered frozen. This [changed in Ruby 3], since such strings are
dynamic there's no point in freezing them by default.

The test wasn't catching this because `FrozenError` actually inherits
from `RuntimeError`:

>> FrozenError.ancestors
=> [FrozenError, RuntimeError, StandardError, Exception, Object ...]

So the exception check passed. Now we're also checking for the error
message to ensure it raised the exception we really expected there.

Closes #5465

[changed in Ruby 3] https://bugs.ruby-lang.org/issues/17104

Co-authored-by: Martin <martin@edv-beratung-meier.de>
  • Loading branch information
carlosantoniodasilva and mameier committed Mar 1, 2023
1 parent 41e2db2 commit ee8f0f8
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
### Unreleased

* bug fixes
* Fix frozen string exception in validatable. [#5563](https://github.com/heartcombo/devise/pull/5563) [#5465](https://github.com/heartcombo/devise/pull/5465) [@mameier](https://github.com/mameier)

### 4.9.0 - 2023-02-17

Expand Down
2 changes: 1 addition & 1 deletion lib/devise/models/validatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def self.assert_validations_api!(base) #:nodoc:
unavailable_validations = VALIDATIONS.select { |v| !base.respond_to?(v) }

unless unavailable_validations.empty?
raise "Could not use :validatable module since #{base} does not respond " <<
raise "Could not use :validatable module since #{base} does not respond " \
"to the following methods: #{unavailable_validations.to_sentence}."
end
end
Expand Down
5 changes: 4 additions & 1 deletion test/models/validatable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,12 @@ class ValidatableTest < ActiveSupport::TestCase
end

test 'should not be included in objects with invalid API' do
assert_raise RuntimeError do
exception = assert_raise RuntimeError do
Class.new.send :include, Devise::Models::Validatable
end

expected_message = /Could not use :validatable module since .* does not respond to the following methods: validates_presence_of.*/
assert_match expected_message, exception.message
end

test 'required_fields should be an empty array' do
Expand Down

0 comments on commit ee8f0f8

Please sign in to comment.