-
Notifications
You must be signed in to change notification settings - Fork 2
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
ARCH-2109 ensure errors.messages doesn't raise in have_error_on_attribute matcher #445
ARCH-2109 ensure errors.messages doesn't raise in have_error_on_attribute matcher #445
Conversation
…ute_matcher in rspice
@@ -26,6 +26,7 @@ | |||
@errors = (record.errors.details[attribute.to_sym] || []).pluck(:error).map(&:to_sym) | |||
|
|||
expect(@errors).to include(@detail_key.to_sym) | |||
expect(record.errors.messages).to_not raise_error(I18n::MissingTranslationData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems a little heavy-handed. maybe record.errors[attribute]
just to limit the scope?
@@ -26,6 +26,7 @@ | |||
@errors = (record.errors.details[attribute.to_sym] || []).pluck(:error).map(&:to_sym) | |||
|
|||
expect(@errors).to include(@detail_key.to_sym) | |||
expect(record.errors[attribute.to_sym]).to_not raise_error(I18n::MissingTranslationData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe raise_error shouldn't take an argument I18n::MissingTranslationData
since that is specific to our use case and this is a public gem. expect(record.errors[attribute.to_sym]).to_not raise_error
should still work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
@MichelleMik question: are there no tests to test this matcher? That might be a dumb question 😬 |
@tbconroy didnt find any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested locally 👍
https://freshly.atlassian.net/browse/ARCH-2109
To test :
Use referral service branch:
ARCH-2019-rraise-on-missing-i18n-translations-in-development
, and run the specs . They should be all green .update
spicerack
line in Gemfile to following :gem "spicerack", git: "git@github.com:Freshly/spicerack.git", branch: "ARCH-2109-assert-have-error-on-attribute-matcher-doesnt-raise"
run
bundle install
run the rspecs, you should have a failure for a missing translation :