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

Add a unit test to check if all the translation in messages_{LOCALE}.properties files are synchronized with the default one #3196

Merged
merged 5 commits into from
Nov 21, 2024

Conversation

katysaintin
Copy link
Contributor

As it mentioned in the last CS-meeting with @georgweiss
Function checkMessageFilesDifferences added in org.phoebus.framework.nls.NLS class in core-framework module.
It lists the differences between all the existing messages_{LOCALE}.properties and messages.properties file.
If there is a missing Key in the messages_{LOCALE}.properties or if a key should be removed.

A unit test in org.phoebus.framework.nls.NLSMessagesTest is added .
checkAllMessagesResources() lists all the differences in all the Phoebus project.
Using this unit test, all the messages_fr.properties are updated now.

@katysaintin katysaintin self-assigned this Nov 19, 2024
Copy link
Collaborator

@georgweiss georgweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the static methods in class NLS (e.g. checkMessageFileDifferences) to be in the unit test class since they are not used outside that test.
When I run the test class it fails on one test. If differences are detected it's OK to print that, but a unit test failure will fail a build of Phoebus. I think we should avoid it in this case.

@katysaintin
Copy link
Contributor Author

I'd prefer the static methods in class NLS (e.g. checkMessageFileDifferences) to be in the unit test class since they are not used outside that test. When I run the test class it fails on one test. If differences are detected it's OK to print that, but a unit test failure will fail a build of Phoebus. I think we should avoid it in this case.

OK, no problem, I understand, what I can do is :

  • move the code in a static method in NLS
  • modify the unit test , just in printing the differences but, consider that it is not a failure.

Katy

@georgweiss
Copy link
Collaborator

Yes, I think this is what should be changed.

print a warning message instead of build failure.
@georgweiss
Copy link
Collaborator

Now I am a bit confused... the static methods used in the test class should not be in NLS.java but in NLSMessagesTest.java

@katysaintin
Copy link
Contributor Author

Ah OK, I understand the invert, sorry, it's my bad. I will move all the code in NLSMessagesTest.java
but we'll get there eventually

@katysaintin
Copy link
Contributor Author

It should be OK now, NLS.java is revert to the original version.
Only NLSMessagesTest.java changes, without considering differences in messages.properties as a failure.
Update all the messages_fr.properties according to messages.properties

@georgweiss
Copy link
Collaborator

georgweiss commented Nov 21, 2024

Interesting to see that with a change in Locale, SEVERE becomes SCHWERWIEGEND in German when logger prints messages.

@georgweiss georgweiss merged commit a2e804d into ControlSystemStudio:master Nov 21, 2024
1 check passed
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.

2 participants