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.

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

fix: force language in tests #1819

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreabergia
Copy link
Contributor

Fixes #1803

@rPraml
Copy link
Contributor

rPraml commented Feb 5, 2025

I would like to point out that it might be a bad idea to only run the tests in English.

In principle, I have nothing against changing it like that, but if we change it, then developers might not find errors in local build, that would, for example, appear in production in their language setting.

For example, my machine runs on de_DE and time zone Europe/Berlin and all tests are passing.

/edit: and the reason, why test runs in german locale is, that for the particular error message, there are translations only for "fr" and "zh_CN" ... and a german translation file is missing
grafik

@gbrail
Copy link
Collaborator

gbrail commented Feb 8, 2025

I think that this is in response to an issue, but I don't recall what. I'd like to make sure we know what problem we're fixing first. In general, setting a locale in the tests so that they always run the same way is a good idea, although if we want to do that we should probably do it somewhere in the build/src directory so that it affects all the modules.

However, that would imply that we only test in English, so we should probably follow up with running the tests in CI in a few different locales.

Does anyone else have experience doing this who could maybe look in to that?

@andreabergia
Copy link
Contributor Author

I think that this is in response to an issue, but I don't recall what. I'd like to make sure we know what problem we're fixing first.

The issue is linked in the PR description - #1803

However, that would imply that we only test in English, so we should probably follow up with running the tests in CI in a few different locales.

Does anyone else have experience doing this who could maybe look in to that?

I don't think that is really scalable. There's a lot of test that check the text of an error message, which is generated via the properties files. All error messages have a description in English, but very few have a description in the other two languages for which properties file exist (French and Chinese). Duplicating those tests to check the error message in, say, French wouldn't really give us a lot more confidence in the code, IMHO.

@gbrail
Copy link
Collaborator

gbrail commented Feb 11, 2025

I think that our best bet is to:

  1. Force en/US and Pacific time in all the tests -- this should be done in "rhino.java-conventions.gradle" so that it gets picked up by all the modules
  2. See if there are a few specific tests we can run with other languages to sanity-check the messages and such

I'm happy to tackle the first part but for the second I'd like someone to try who knows what kinds of errors we'd be looking for.

@rPraml
Copy link
Contributor

rPraml commented Feb 12, 2025

I'm fine, if we set the default language in the tests for now to en/US etc.

If I find time, I can try to create a GH-action that runs in various locales/TZ.
At least, I would try tr-TR: http://www.moserware.com/2008/02/does-your-code-pass-turkey-test.html
but I'm on holiday for the next 2 weeks (in Istanbul ;-) ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests assume that error messages are always in English
3 participants