ci(translation-tests): verify reproducibility of gettext machine objects #6953
Conversation
cfm
force-pushed
the
i18n-verify-mos
branch
3 times, most recently
from
aaebd0b to
71774a7
Compare
From freedomofpress/securedrop-client#1666, from which "verify-mo.py" has been copied verbatim. The "verify-mo" Make target is largely the same, except for some fancy (but temporary) "find" footwork to put "i18n_tool.py"'s outputs where "verify-mo.py" expects them.
cfm
force-pushed
the
i18n-verify-mos
branch
4 times, most recently
from
66c1f83 to
f5b02f7
Compare
Adapted from freedomofpress/securedrop-client@6fcd4dd. Here we do treat "make verify-mo" as a linter, to avoid running it (over n languages) n times in each of n "translation-tests" jobs.
When typing[1] exc_type as Optional[type[BaseException]], there seems to be no way around "TypeError: 'type' object is not subscriptable" in our current Python 3.8, even using __future__.annotations. [1]: https://adamj.eu/tech/2021/07/04/python-type-hints-how-to-type-a-context-manager/
cfm
force-pushed
the
i18n-verify-mos
branch
from
f5b02f7 to
6d3f427
Compare
There was a problem hiding this comment.
This all looks great, @cfm! No requested changes from me on the code, and the test plan checks out. I think this is all set.
- CI passes, especially
lint. -
make verify-mosucceeds locally. - If you change a random
install_files/ansible-base/roles/tails-config/templates/*.pofile,make verify-mofails and highlights the change (because the "original".mofile for that language now differs from the recompiled one). - If you
pip3 uninstall diffoscope,make verify-mofails with asubprocess.CalledProcessErrorfor return code127.
|
Thanks, @nathandyer! |
| @@ -94,6 +94,13 @@ jobs: | |||
| DOCKER_BUILD_ARGUMENTS="--cache-from securedrop-test-focal-py3:${fromtag:-latest}" securedrop/bin/dev-shell \ | |||
| bash -c "/opt/venvs/securedrop-app-code/bin/pip3 install --require-hashes -r requirements/python3/develop-requirements.txt && make -C .. lint" | |||
|
|
|||
| - run: | |||
| name: Check that translation machine objects are reproducible | |||
There was a problem hiding this comment.
Why does this need to be its own step? Can it not be part of the make lint target?
There was a problem hiding this comment.
I'd originally assumed it would be a step in translation-tests. But it doesn't benefit from translation-tests's parallelism, since we might as well verify all languages at once, not just supported languages one at a time.
The lint job seemed like the next best logical place. But I take make lint itself to be analogous to spellcheck for human-modified files, not machine-generated ones. And I don't see any benefit to running make verify-mo locally at all. I resisted the temptation to move it into its own job altogether. ;-)
Would you prefer to see this run somewhere else? See also some extra CI bits in #6954.
There was a problem hiding this comment.
It's fast enough that the overhead of re-installing the pip dependencies takes more time than actually running the verification script. I think merging into lint is probably the right option, I'll roll that into my upcoming PR.
Philosophically, I think it doesn't matter what is being linted/tested, rather it's that "lint" is all the fast tools that just analyze code while "test" is slower stuff that does execution, etc.
Status
Ready for review
Description of Changes
securedrop-clientto keep these scripts in sync: chore: makeverify-mo.pycompatible withsecuredropsecuredrop-client#1668i18n_tool.py#6917Testing
Just like freedomofpress/securedro-client#1666:
lint.make verify-mosucceeds locally.install_files/ansible-base/roles/tails-config/templates/*.pofile,make verify-mofails and highlights the change (because the "original".mofile for that language now differs from the recompiled one).pip3 uninstall diffoscope,make verify-mofails with asubprocess.CalledProcessErrorfor return code127.Deployment
CI-only; no deployment considerations.