soundscape-community
Tests should fail for missing translations
There is a script to check translations in apps/ios/Scripts/LocalizationLinter/main.swift, but it just prints warnings and doesn't cause tests to fail. It would be good if missing translations could cause tests to fail.
Does that mean that given it's current state right now, where only the English and Spanish translations are fully updated, the tests should fail because the other languages aren't up to date yet? Are these tests going to warn Weblate or raise the priority of the translations? Just curious.
Or could we change the process on Weblate where we use an auto translator to create suggestions to help out other translators? These suggested translations will not be part of the app unless approved by human translators on Weblate.
@JJGatchalian To make sure your ideas don't get lost, it would be best to post them in a new issue. We can't have too many issues, they are the best way to track things on GitHub. This is a request for someone to improve the automated tests so that we don't have a repeat of the missing string in 1.3.0 voice settings.
The desired behavior would be to block the build if any localization key referenced in the code is missing from the English strings. The goal here is to avoid situations like #168 where we removed the original English text without realizing the app still used it somewhere. This was exacerbated by Weblate not-so-helpfully removing all the other translations for that key in commit 3b23566.
Currently, if a localization key in the code is missing, it prints a warning, but after changing it to an error and adding a nonzero exit, a Command PhaseScriptExecution emitted errors but did not return a nonzero exit code to indicate failure error appears alongside the intended missing localization key error despite a nonzero exit directly after. I'm having trouble finding an approach to fix the non-intended error, and I'm also confused as to why it appears especially since the code exits with a nonzero exit code.
Would it make sense to revamp the test cases so that if one or more tests fail, a nonzero exit occurs at the end of the script rather than directly after the error output?
Apologies if this is a naive question, I'm pretty new to this. Thanks in advance for any suggestions, I'm looking forward to making my first contribution and learning from you all.
We're all learning together :) I'm not sure I understand what exactly you're describing. Could you push the changes to your fork? That way we can see your changes and the test output in GitHub Actions.
Hi @iEli
That's because xCode runs the swift script via a shell script, and the shell script never exits with a non-zero exit code. You can find it by selecting GuideDogs in the project navigator,then select the Soundscape target. select the build phases tab and then the Localization Validation phase.
I'm not sure that that's a problem though, because we probably don't want it to fail builds during development.
The CI tests are defined in https://github.com/soundscape-community/soundscape/blob/main/.github/workflows/ios-tests.yml#L16
If you add a new step there that calls the LocalizationLinter script directly it should cause the github action to fail when it gets a non-zero exit code.
Note that GitHub will not use your modified ios-tests.yml when you create a PR. It will only work when we merge it into main.
Thanks for the help, I think I understand it better now. Attached is a screenshot of the unit tests ran on my fork, please let me know if this is the intended behavior.
@iEli Yes, although I can't view the screenshot because I'm blind, The failing test run on your fork looks good. If you could re-add the missing string and then make a pull request that would be great.
After the merge, the latest patch still says that the tests have failed.
@JJGatchalian It's just an old string referenced by a file in the project that I need to remove, It'snot actually used by the app. Fix will follow shortly.
Is is a good validation of the work done by @iEli though! 😊
I hope the patch to remove the string will be merged. I fixed some typos in the Spanish text in the Weblate fork, but the tests failed most likely because there's no patch yet. Thanks.
Should I close this issue since the bug fix has already been merged? BTW, I fixed some capitalization and punctuation in the English and Spanish, and most of these inconsistencies were in the original Microsoft text. We're just waiting for the next Weblate update. The next change should be the update of the IOS terminology for IOS 26, where I have to change "Spoken Content" to "Read & Speak." For devices still using IOS 18 or earlier, I'll have to add notes in the documentation that use the old term "Spoken Content" so those users can still follow the instructions.
thanks, closing.
We could leave those strings alone and add new ones for ios 26 if you think it is worth it. We currently support right back to ios 14 so that it can work on older phones.
I don't know how many users are on an older ios version.
@jchudge do you know if the appstore connect analitics includes ios version?