You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Continuing the discussion here since the other issue was closed.
Quoting Serhiy:
I thought that we should run the parser with different options to produce different error messages with installed special translator that transforms the original text in simply recognizable way. But there may be better ways.
I was originally thinking the same, but to me that seems like a lot of effort compared to the benefit we'd get. Instead, I think that using a message extractor (e.g. pygettext) we can assert both the presence of a specific message and the line number it appears at. This should give us pretty much the same confidence as directly checking the argparse output but with an order of magnitude less work. What do you think?
Does it matter that much if you don't need to fix the tests manually? You'd have a snapshot of the PO file and a simple command to regenerate it so all you'd need to do is double-check the diff
You would need to regenerate the snapshot after every change in argparse (and it was changed a lot in last few days), and the diff would be so large that it would be hard to notice a real regression in the noise.
But this is a good first step. Before #27667 was merged, Tools/i18n/pygettext.py produced a warning for _() used with non-literals. They should be treated as errors in the test. Also, line numbers should be ignored in comparison and only used to report errors. We should just collect a set of translated strings, compare it with the expected result and report about missing or additional strings.
We should also add translation tests for getopt and optparse, and maybe for other modules (this is separate issues). I am not a fan of converting many tests into packages just to add a data file. This may break history, add conflicts with existing PRs and make future work with tests less convenient. Maybe add a common directory for translation data?
IIRC pygettext currently does not know how to handle *gettext() functions which take more than one argument.
There is a way to specify additional keywords via --keyword=XXX but I don't see any way to tell it how many arguments
it takes.
Activity
[-]Add translation tests to argparse[/-][+]Add translation tests for argparse[/+]tomasr8 commentedon Sep 24, 2024
Continuing the discussion here since the other issue was closed.
Quoting Serhiy:
I was originally thinking the same, but to me that seems like a lot of effort compared to the benefit we'd get. Instead, I think that using a message extractor (e.g. pygettext) we can assert both the presence of a specific message and the line number it appears at. This should give us pretty much the same confidence as directly checking the argparse output but with an order of magnitude less work. What do you think?
serhiy-storchaka commentedon Sep 24, 2024
Would not this make the tests extremely fragile?
tomasr8 commentedon Sep 25, 2024
Does it matter that much if you don't need to fix the tests manually? You'd have a snapshot of the PO file and a simple command to regenerate it so all you'd need to do is double-check the diff
serhiy-storchaka commentedon Sep 25, 2024
You would need to regenerate the snapshot after every change in
argparse(and it was changed a lot in last few days), and the diff would be so large that it would be hard to notice a real regression in the noise.But this is a good first step. Before #27667 was merged,
Tools/i18n/pygettext.pyproduced a warning for_()used with non-literals. They should be treated as errors in the test. Also, line numbers should be ignored in comparison and only used to report errors. We should just collect a set of translated strings, compare it with the expected result and report about missing or additional strings.tomasr8 commentedon Sep 25, 2024
True, if the module is changing a lot, so would the snapshots. I'll try to come up with a PR based on your suggestions :)
serhiy-storchaka commentedon Oct 26, 2024
We should also add translation tests for
getoptandoptparse, and maybe for other modules (this is separate issues). I am not a fan of converting many tests into packages just to add a data file. This may break history, add conflicts with existing PRs and make future work with tests less convenient. Maybe add a common directory for translation data?8 remaining items
tomasr8 commentedon Oct 27, 2024
I'll look into that :)
gh-124295: Skip translation tests when pygettext is missing (GH-126051)
pythongh-124295: Skip translation tests when pygettext is missing (py…
[3.13] gh-124295: Add translation tests for argparse (GH-124803) (GH-…
[3.12] [3.13] pythongh-124295: Add translation tests for argparse (py…
[3.12] gh-124295: Add translation tests for argparse (GH-124803) (GH-…
serhiy-storchaka commentedon Oct 27, 2024
It does not catch
ngettextstrings.tomasr8 commentedon Oct 27, 2024
IIRC pygettext currently does not know how to handle
*gettext()functions which take more than one argument.There is a way to specify additional keywords via
--keyword=XXXbut I don't see any way to tell it how many argumentsit takes.
For example invoking
Produces several warnings (and does not extract any ngettext strings):
We could fix pygettext to allow functions like ngettext. We could probably use the same syntax that GNU's xgettext and pybabel use to specify keywords and their arguments: https://www.gnu.org/software/gettext/manual/html_node/xgettext-Invocation.html#index-_002dk_002c-xgettext-option
serhiy-storchaka commentedon Oct 28, 2024
For beginning, it should support all
gettextfunctions by default. Later we can make this customizable.pythongh-124295: Add translation tests for argparse (pythonGH-124803)
pythongh-124295: Skip translation tests when pygettext is missing (py…
pythongh-124295: Add translation tests for argparse (pythonGH-124803)
pythongh-124295: Skip translation tests when pygettext is missing (py…