Re: [PATCH 4/4] tests: remove most uses of test_i18ncmp

On Thu, Feb 11 2021, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

As a follow-up to d162b25f956 (tests: remove support for
GIT_TEST_GETTEXT_POISON, 2021-01-20) remove most uses of test_i18ncmp
via a simple s/test_i18ncmp/test_cmp/g search-replacement.

I am a bit hesitant to take this step, primarily because the
distinction between test_cmp and test_i18ncmp is serving as a sign
for tests to identify if they are dealing with translatable messages
or plumbing/machine-readable messages.

If a totally different approach to test i18n messging part
(e.g. Dscho's rot13) appears any time soon, it would be helpful to
leave these distinction in, rather than ripping them out, with two
caveats.

 - If such a change does happen soon enough, then the codebase this
   huge patch touches would not be all that different by the time
   when it happens, so reverting this patch should not be too
   involved, which means my hesitation is not all that warranted.

 - If such a change does not happen for some time, then it would
   become impractical to revert this patch when it does happen due
   to changes in the codebase, if we apply it with the intention of
   "we can easily revert if we need to".  But even if we drop this
   step, until such a change appears, we surely will gain more calls
   to test_cmp (and possibly test_i18ncmp) in the codebase, and at
   that point, we can trust the distinction between test_cmp and
   test_i18ncmp even less, which means it does not help all that
   much if we tried to keep test_i18ncmp.

So, perhaps we should bite the bullet and just take this step.

I think it either makes sense to take this series as a whole, or not
take any of it and also revert the already merged 73c01d25fe (tests:
remove uses of GIT_TEST_GETTEXT_POISON=false, 2021-01-20).

I.e. if we're going to not remove test_i18ncmp uses then we should leave
C_LOCALE_OUTPUT & GIT_TEST_GETTEXT_POISON=false alone as well (or
convert them to the test_i18n* forms).

As noted before I'm not invested in the outcome here, I just submitted
this to not leave the obvious post-cleanup undone.

My reading of the list discussion/consensus[1] (such as it was) was that
even if we had this rot13 mode ready we wouldn't think it's worth the
time investment overall, hence the follow-up patches.

Also, if we have it down the line we're probably better off with these
patches. The parts that'll revert cleanly can likely be resurrected
as-is, but conflicts would be a good thing, as that code needs to be
looked at anyway (it will be re-arranged tests that never used
GETTEXT_POISON in their current form).

We also won't have cargo-culting in the meantime, with new code using
test_i18ncmp or whatever just because the test above them did.

1. https://lore.kernel.org/git/20210120182759.31102-1-avarab@gmail.com/ (local)