The objective here is to close the discussion started here.
It ended up here, looking like this:
### Never use I18n.t in assertions
When asserting text that is translatable, always assert against the translated string, not the call to I18n.t.
Good `expect(json_response['errors']).to eq "Hm, something went wrong. No order cycle data found."`
Bad `expect(json_response['errors']).to eq I18n.t('admin.order_cycles.bulk_update.no_data')`
But some people disagree.
I dont have an opinion except that I think we need a decision, one way or the other. Having the code mixed up with the two approach is very poor practice…
@luisramos0
I thought we should use t(translation_key) in specs and not the text itself
@maikel
Upside: We can change text without changing the spec.
Downsides:
- The spec doesn’t detect some translation mistakes:
- Duplicate keys often lead to the selection of the wrong key.
- A typo in the key (used in the view) may not be detected.
- Mistakes in string interpolation can’t be tested.
- The spec is harder to read and you don’t know immediately what is displayed.
@luisramos0
I really like to see the I18n.t on the specs, looks very clean, but your arguments against it are very good!
@Matt-Yorkley
I tend to prefer using I18n.t so things can be reworded without changing the specs. Spelling mistakes in the view translation key would still be caught, no? Maybe the usefulness of either option depends on the situation, like with interpolated keys it might be better to check the full output.
Not being able to read the message whilst scanning the specs is a fair one, but my IDE displays the translation automatically when viewing keys in specs, so I’m pretty ambivalent on that one…
@kristinalim
I also vote not to use I18n.t
in the tests.
Spelling mistakes in the view translation key would still be caught, no?
Hmm… @Matt-Yorkley Yes, you’re right, it would catch a typo in the base key, although I think not mistakes in the scope.
Nice list of downsides, @mkllnk. An addition to that, if you have missing keys:
expect(“translation missing: en.missing_key”).to eq(“translation missing: en.missing_key”)
@luisramos0
I’d consider it closed. No one disagrees with this convention, right? link_to_convention
@sauloperez
I actually disagree @luisramos0 . It allows running the specs in any language and provides an extra abstraction that lets you change the copy without having to touch the test. See Better Tests Through Internationalization.