codegram / date_validator

A simple, ORM agnostic, Ruby >=2.2 compatible date validator for Rails, based on ActiveModel.
http://thoughts.codegram.com/date-validation-with-rails-3
MIT License
496 stars 82 forks source link

Fix test flakes #86

Closed ball-hayden closed 3 years ago

ball-hayden commented 3 years ago

✍️ Description

The current test suite is order dependent. Specifically, the following test fails if it is not run in the correct order:

ActiveModel::Validations::DateValidator::when value does not match validation requirements#test_0012_allows custom validation message to be handled by I18n [/home/runner/work/date_validator/date_validator/test/date_validator_test.rb:93]

The cause of this seems to be related to lazy loading of translations by the I18n gem.

If I18n.backend.store_translations is called before the I18n backend has loaded translations, the "overridden" translation is lost and the default is returned instead. This would only be an issue for ActiveModel versions depending on i18n >= 1.6.0, which appears to have only been required since Rails 6.

Also pulls in 4df94311284af408007696f57638d1f88efff390 to fix issues if Time.zone hasn't been required yet.

✅ Fixes

This should fix the flakes currently blocking https://github.com/codegram/date_validator/pull/85

✅ QA

Before requesting a review, please make sure that:

Documentation

I18n release notes for lazy/eager loading: https://github.com/ruby-i18n/i18n/releases/tag/v1.6.0

ball-hayden commented 3 years ago

Worth commenting that the I18n fix is only really an issue in the test suite.

There are contrived cases where a user has used store_translations in a Rails app which hasn't eager loaded translations (eager loading is done by default for the production env), but that isn't isolated to this project. Arguably, thinking about it, this is a bug in I18n.

mrcasals commented 3 years ago

Hi @ball-hayden! Thanks for taking the time and looking into it! Much appreciated!