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

Fix frozen string in validatable module #5563

Merged
merged 1 commit into from Mar 1, 2023

Conversation

carlosantoniodasilva
Copy link
Member

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

Copy link
Collaborator

@nashby nashby left a comment

Choose a reason for hiding this comment

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

looks good!

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
@carlosantoniodasilva carlosantoniodasilva merged commit ee8f0f8 into main Mar 1, 2023
96 checks passed
@carlosantoniodasilva carlosantoniodasilva deleted the ca-fix-frozen-string branch March 1, 2023 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants