GeezerVison is showing github.com — Regression in 0.9.3 · Issue #404 · ruby-i18n/i18n
DescriptionWhat I tried to do
I have a Rails app with RSpec tests that use Faker (1.8.7) for fake data generation.
I updated i18n to 0.9.3 to have all my gems at their most recent version.
What I expected to happen
I expected the test suite to pass since no changes apart from gem updates were made.
What actually happened
The whole test suite exploded with variations of the following message:
I18n::MissingTranslationData:
translation missing: en.faker.name.first_name
I could isolate this to i18n: When I downgraded to 0.9.1, everything worked fine – but with 0.9.3 it breaks, so my assumption is that it's related to i18n, not faker.
Versions of i18n, rails, and anything else you think is necessary
- Ruby 2.4.2
- Rails 5.1.4
- i18n 0.9.3
- faker 1.8.7
Let me know if you need any further information. Activityradar commented on Jan 23, 2018 Thanks for letting us know :)
Do you have some small code that would reproduce this problem?
…
On 23 Jan 2018, at 23:32, Clemens Kofler ***@***.***> wrote:
What I tried to do
I have a Rails app with RSpec tests that use Faker (1.8.7) for fake data generation.
I updated i18n to 0.9.3 to have all my gems at their most recent version.
What I expected to happen
I expected the test suite to pass since no changes apart from gem updates were made.
What actually happened
The whole test suite exploded with variations of the following message:
I18n::MissingTranslationData:
translation missing: en.faker.name.first_name
I could isolate this to i18n: When I downgraded to 0.9.1, everything worked fine – but with 0.9.3 it breaks, so my assumption is that it's related to i18n, not faker.
Versions of i18n, rails, and anything else you think is neccessary
Ruby 2.4.2
Rails 5.1.4
i18n 0.9.3
faker 1.8.7
Let me know if you need any further information.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
clemens commented on Jan 23, 2018 CollaboratorAuthor More actions Sure, here are some snippets with what I think are the most important areas:
# config/application.rb
config.i18n.default_locale = :de
config.i18n.locale = :de
config.i18n.available_locales = [:de]
# spec/support/faker.rb
if defined?(Faker)
# we only set this in environments that support fixture/factory data
# (i.e. currently development, test and staging)
DEFAULT_PASSWORD = '12345678' unless defined?(DEFAULT_PASSWORD)
I18n.enforce_available_locales = false # faker questionably sets this to true
Faker::Config.locale = :de
end
# spec/factories/users.rb
FactoryBot.define do
factory :user do
first_name { Faker::Name.first_name }
last_name { Faker::Name.last_name }
end
end
# spec/models/user_spec.rb
RSpec.describe User, type: :model do
subject(:user) { FactoryBot.create(:user) } # <= whenever a test calls the subject, it explodes
endHope that helps. Reacted by Ricardo Pacheco radar commented on Jan 24, 2018 Thanks @clemens. I've attempted to reproduce this issue in a brand new Rails 5.1.4 application: https://github.com/radar/i18n-404-repro
I was following the steps you've graciously provided. I was unable to reproduce this issue. The test in that application is still passing for me:
.
Finished in 0.45536 seconds (files took 1.89 seconds to load)
1 example, 0 failures
Perhaps there's one (or more) steps that are missing? Could you try cloning down the application and see if you could reproduce the issue?
Very interested to get to the bottom of this but it's hard for me to do that if I am unable to reproduce the issue. Thank you for your assistance. tneems commented on Jan 27, 2018 I had this same/similar problem it was solved by adding 'en' to our our list of available_locales in the test environment. I18n.enforce_available_locales = false was being set, so this shouldn't have been happening. 🤷♂️ YMMV clemens commented on Jan 27, 2018 CollaboratorAuthor More actions @radar I'll try to create a failing app later today/tomorrow.
@tneems The problem is: We're using faker in both development (to fill the database with fake data) and test. While there certainly isn't any harm in adding :en and not enforcing available locales in development, I prefer to have the dev/prod configs differ as little as possible. radar commented on Jan 27, 2018 Thanks @clemens. An app that I can use to reproduce this issue on my machine would be extremely useful :)
On 28 Jan 2018, at 03:55, Clemens Kofler ***@***.***> wrote:
@radar I'll try to create a failing app later today/tomorrow.
@tneems The problem is: We're using faker in both development (to fill the database with fake data) and test. While there certainly isn't any harm in adding :en and not enforcing available locales in development, I prefer to have the dev/prod configs differ as little as possible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
clemens commented on Jan 27, 2018 CollaboratorAuthor More actions clemens commented on Jan 27, 2018 CollaboratorAuthor More actions I think I even just found the culprit by accident in the very first commit after 0.9.1: 5077ef9.
This seems to be an optimization which doesn't store translations for locales that aren't in the available_locales list. So when faker tries to put in its English translations (which it needs for Faker::Internet), i18n rejects them because en is not in the available_locales list and thus isn't considered relevant.
It looks like https://github.com/stympy/faker/pull/1133/files introduces workarounds in faker tests which become necessary for the same reason.
Based on a gut feeling (I haven't fully reflected this), I see three possibilities to approach this:
- revert Don't store translations for locales not set as available #391 since it was a performance optimization but introduced a regression
- add a configuration flag to disable the optimization, e.g.
store_translations_for_unavailable_locales = true so people can explicitly opt into the optimization
- probably the best one: fix faker to not rely on English but instead simply use something else (e.g. lorem ipsum words) instead of a company name as a
domain_word.
clemens commented on Jan 28, 2018 CollaboratorAuthor More actions Thinking about this some more, I actually think the Faker fix should be independent of the fix of this regression.
I could provide PRs for both approaches if desired. Just let me know. radar commented on Jan 30, 2018 Hi @clemens. Thanks for providing some excellent steps to reproduce this issue.
I believe that while there is a change in i18n that introduced this issue, the change is worthwhile keeping as it prevents unused locales from being stored, which can take up memory in the Ruby process and slow down booting of Rails applications. So the fix in i18n will be staying.
I think the way to sort out this problem is to change Faker's behaviour, particularly the with_locale line in this method:
https://github.com/stympy/faker/blob/679fd508c9f41f41b6df62024d67bc2281953395/lib/faker/internet.rb#L17-L20
There is absolutely no guarantee that :en would be an available locale in every application, as you have demonstrated here. I think the wrapping of with_locale is erroneous and should be removed and that will fix this issue that you're seeing, as it will use Faker::Config.locale, which you've configured to be :de.
Faker does have the proper translations (at least in the de locale) to make this method work without the with_locale wrapping.
So what I think you should do is open a PR on faker to remove that with_locale wrapping and then wait for a new release to be made.
As a substitute, may I suggest using SecureRandom.urlsafe_base64(8)? wjordan commented on Feb 12, 2018 This regression also breaks the i18n pipeline in my team's Rails application (code-dot-org/code-dot-org), which sets config.i18n.enforce_available_locales = false in its Rails application initializer.
Even if the performance optimization may be worthwhile, I believe there is still a regression here in that the existing enforce_available_locales setting is no longer being respected. According to "The Public i18n API" section of the Rails i18n API Guide (which seems to be the only reference documentation for this gem), enforce_available_locales is defined as:
Enforce locale whitelisting (true or false)
Given this documentation, the expected behavior is that when enforce_available_locales is false, locale whitelisting would be un-enforced, implying that any optimization enforcing the set of whitelisted locales would not be applied. radar commented on Feb 12, 2018 Thanks for confirming @wjordan. Could you please try 0.9.4? If it still causes an issue, please show me an app which reproduces it and I can take a look.
Thanks! wjordan commented on Feb 13, 2018 @radar here's a minimal repro script in a rails new app that sets enforce_available_locales = false and config.i18n.available_locales to ["en-US"], and calls I18n.t on a key only available in the 'en' locale:
issue-404.sh:
#!/bin/bash
RAILS_PATH=${RAILS_PATH:-/tmp/issue-404}
RAILS_VERSION=${RAILS_VERSION:-5.1.4}
I18N_VERSION=${I18N_VERSION:-0.9.4}
rm -rf ${RAILS_PATH}
rails _${RAILS_VERSION}_ new ${RAILS_PATH} -q > /dev/null
pushd ${RAILS_PATH} > /dev/null
sed -i '/Rails::Application/a config.i18n.enforce_available_locales = false\nconfig.i18n.available_locales = ["en-US"]' config/application.rb
echo "gem 'i18n', '${I18N_VERSION}'" >> Gemfile
bundle update i18n --quiet
DISABLE_SPRING=1 bin/rails r "puts I18n.t 'hello'"
popd > /dev/null$ I18N_VERSION=0.9.4 ./issue-404.sh; I18N_VERSION=0.9.1 ./issue-404.sh
translation missing: en.hello
Hello world
added 2 commits that reference this issue on Feb 13, 2018This issue will close once commit 70789d6 is merged into the 'master' branch.70789d6 This issue will close once commit 596a71d is merged into the 'master' branch.596a71d
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|