Page MenuHomePhorge

Make i18n string extraction ignore strings in test case files
ClosedPublic

Authored by pppery on May 7 2024, 22:01.

Details

Summary

Do not extract strings from test case files for localization / translation.
They are not exposed to users, and localizing wastes translators' time.

Closes T15815

Test Plan
  • Install Wikimedia downstream translations extension via git clone https://gerrit.wikimedia.org/r/phabricator/translations and put it on the same filesystem level as phorge and arcanist, and follow https://we.phorge.it/w/docs/extensions/install/
  • Apply https://gerrit.wikimedia.org/r/c/phabricator/translations/+/1028887 on top, to make the translations extension work with upstream Phorge.
  • In the downstream translations extension path, run ./export.sh, then run grep -r "__tests__" . and get a list of qqq.json files with context for strings from test files.
  • Apply this patch.
  • In the downstream translations extension path, run ./export.sh, then run grep -r "__tests__" . to verify that no qqq.json files include any strings from test files anymore.

Diff Detail

Repository
rP Phorge
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1233
Build 1233: arc lint + arc unit

Event Timeline

pppery requested review of this revision.May 7 2024, 22:01

Re-diff with linting and tests working hopefully

aklapper subscribed.

I tested this one-liner as part of the downstream conversation that pppery and I had in https://phabricator.wikimedia.org/T363364, still I would ideally prefer another upstream opinion before accepting this revision as O1.

Thanks. To reviewers: maybe relevant, maybe not:

T15815#17250

Unfortunately I don't have the big picture here. Still learning what TranslateWiki heroes are doing and could do.

I'd be worried that this might break some tests that do relate to translation/extraction, but running arc unit --everything should solve that.

I think it makes sense not to try to translate the tests themselves.

(run the full tests, and then count it as my + O1 Accept).

This revision is now accepted and ready to land.May 8 2024, 07:29

arc unit --everything passed. But I don't think this file is covered by tests at all,

This revision was automatically updated to reflect the committed changes.

(that was me, I didn't have git author configured properly from on the system I landed from)

For the record: If I replace the entire file with just:

<?php

final class PhabricatorInternationalizationManagementExtractWorkflow
  extends PhabricatorInternationalizationManagementWorkflow {


}

arc unit --everything still passes. So that file doesn't appear to be covered by tests at all, which is what I expected.