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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FIx: Indifference to I18n errors.format translation #1546

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

FIx: Indifference to I18n errors.format translation #1546

wants to merge 2 commits into from

Conversation

HeitorMC
Copy link
Contributor

Fixes #1492

I didn't find anything related to internationalization tests 馃, also both spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb and spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb passed without errors.

@vsppedro @mcmire do you have any suggestions on how to test this case, or is this enough?

@vsppedro
Copy link
Collaborator

Hello @HeitorMC, thanks for your PR. I'll take a look at how to do a test for this ASAP. Probably tomorrow or next week.

@HeitorMC HeitorMC marked this pull request as ready for review March 22, 2023 18:35
@HeitorMC
Copy link
Contributor Author

Hey, @vsppedro any updates on this case?

@mcmire
Copy link
Collaborator

mcmire commented Mar 29, 2023 •
edited

Hi @HeitorMC. Sorry for the delay in responding. We fake i18n in tests. You can see existing usage of this in the following context in the tests for allow_value:

shoulda-matchers/spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb

Lines 417 to 441 in fababc3

context 'when the validation error message was provided via i18n' do
it 'passes given a valid value' do
builder = builder_for_record_with_different_error_attribute_using_i18n
expect(builder.record).
to allow_value(builder.valid_value).
for(builder.attribute_to_validate).
with_message(
builder.validation_message_key,
against: builder.attribute_that_receives_error,
)
end
it 'fails given an invalid value' do
builder = builder_for_record_with_different_error_attribute_using_i18n
invalid_value = "#{builder.valid_value} (invalid)"
expect(builder.record).
not_to allow_value(invalid_value).
for(builder.attribute_to_validate).
with_message(
builder.validation_message_key,
against: builder.attribute_that_receives_error,
)
end
end
end

It's a bit hard to see because of the indirection, but these tests verify that if with_message is given a symbol, shoulda-matchers uses the same i18n key to resolve that symbol as Rails does. You can see I18n being stubbed here:

shoulda-matchers/spec/support/unit/record_builder_with_i18n_validation_message.rb

Line 38 in fababc3

I18nFaker.stub_translation(keys, default_message)

So, I think in order to test your PR, you'd want to add a new context and a new test inside of that test that verifies that when with_message is not specified, but strict is, and errors.format is stubbed to return a certain string that's different than what Rails uses, then the test still passes. You don't have to create a new class like we've done for RecordBuilderWithI18nValidationMessage; I think you could keep it simpler and do something like this:

shoulda-matchers/spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb

Lines 70 to 71 in fababc3

expect(validating_format(with: /abc/)).
to allow_value('abcde').for(:attr)

I can suggest something a little more specific later, but let me know if that helps in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsppedro vsppedro Awaiting requested review from vsppedro vsppedro is a code owner

@matsales28 matsales28 Awaiting requested review from matsales28 matsales28 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Indifference to I18n errors.format translation
3 participants
@HeitorMC @vsppedro @mcmire