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

Refactored all unit tests that were failing due to language localizati… #4543

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

FilippoZazzeroni
Copy link

Phabricator:https://phabricator.wikimedia.org/T259859

Notes

Added localizable in the test folder for each supported language and implemented for italian.
Adjusted all unit tests that were failing due to language issue except for MWKLanguageLinkControllerTests that is built to check the en device language.
In order to have unit tests passing in all the supported languages is needed to implement the other localizable files.

Test Steps

In order to test just set the simulator language to italian and run the whole unit tests

Screenshots/Videos

Screenshot 2023-06-09 at 12 49 17 Screenshot 2023-06-09 at 12 49 24

…on issue except for MWKLanguageLinkControllerTests
@FilippoZazzeroni FilippoZazzeroni marked this pull request as draft June 9, 2023 10:55
@FilippoZazzeroni FilippoZazzeroni marked this pull request as ready for review June 9, 2023 10:55
@FilippoZazzeroni
Copy link
Author

FilippoZazzeroni commented Jun 9, 2023

@tonisevener can you please review it when you are available, thanks in advance.

@tonisevener
Copy link
Collaborator

Hi @FilippoZazzeroni. Thanks for looking into this one, and my apologies for the long delay in review. Instead of creating a new set of Localizable.strings that translators will need to keep up with, I wonder if we could just lean on the localized strings that the app is already using? I think that would reduce the setup burden for all languages.

I made a commit here to hopefully illustrate what I mean. The NotificationsCenterCellViewModelGenericTests I believe work across all languages with this commit. For these:

  1. I moved shared strings between the test class and the app into CommonStrings.swift, for easy reference. Then I used those strings as the expected string value in the tests. Note that for this particular view model, it is expected that some of these strings will be populated directly from the server (or the case of this unit test, the fixture json notifications-generic.json). This is why I was able to leave some expected English strings in here like "Thank" - this is how my faked server fixture is set up.
  2. For dates, I dug into the cell view model dateText logic, to determine what date formatter it will ultimately use. Then I used that formatter to translate the expected date string in the test.

Alternatively, since this is a heavier lift, perhaps a quick alternative to all of this would be to simply update our schemes to run tests under English / United States:

screenshot

Let me know what you think - thank you!

Copy link

@nishith142 nishith142 left a comment

Choose a reason for hiding this comment

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

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