Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > You prodded me about this and Johannes also did off-list. So given that > this is already in "next" I think it's best to let usage in this mktag > series land as-is. For an issue like this "test_i18ngrep or not?" that would end up giving a useful hint to future developers, a follow-up commit with an explanation why test_i18ngrep is preferred (or not) would be a great way to go forward (if the "fix" is needed in the first place, that is). It is easy to add a patch to fix things up while a series is cooking in 'next', without having to remember doing so after the series hits 'master'. > git some-command >output && > test_i18ngrep "c locale string" output > > But since 6cdccfce1e0 (i18n: make GETTEXT_POISON a runtime option, > 2018-11-08) this hasn't been needed. I did not immediately see why that commit changes any equation, but I am inclined to say that your argument makes sort-of sense, if not 100%, to me. POISON test is not about testing features that ought to work (that is what non-POISON tests are for). The primary objective of POISON test is to ensure that we didn't over-localize; if all the output from some-command is expected to be fully localized, between the above and GIT_TEST_GETTEXT_POISON=no git some-command >output && grep "c locale string" output there wouldn't be much difference. But if the output contains strings, some of which are expected to be localized (e.g. human facing messages) and some are not (e.g. protocol dump), and the test output is inspected for both types of strings, it would not be an equivalent test. Having said that, it may be OK that a mixed-output-command is tested only in C locale without localization. Our tests do *not* make sure that the strings that ought to be localized are indeed localized anyway, so as long as the inspection of the output string does not check for string that are *not* to be localized, we won't break the primary objective of having POISON tests. In any case, if you want to push forward in that direction to use more GIT_TEST_* settings in the test to replace i18ncmp/grep, please make sure you propose something easier to read than "GETTEXT_POISON" for the environment variable name. It is overly long and makes the tests unreadable by pushing the part of the command that are more important off of the right edge of the screen. Thanks.
