Skip to content

gh-124295: Add translation tests for argparse #124803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Oct 27, 2024

Conversation

tomasr8
Copy link
Member

@tomasr8 tomasr8 commented Sep 30, 2024

This uses Tools/i18n/pygettext.py to extract all translatable messages from argparse.py. The messages are kept in a snapshot file in the test folder. The test extracts the current messages and compares them against this snapshot.

The snapshot can be updated using ./python Lib/test/test_argparse/test_translations.py --snapshot-update.

There is currently no way to read a PO file using the stdlib (gettext only reads MO files, maybe it should be able to do both?) so I just use a simple regex to get the msgids out of the PO file.

Feedback welcome!

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you, @tomasr8.

Since argparse is currently actively fixed, there are some conflicts. After fixing them I'll merge this PR.

@serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 1, 2024
@serhiy-storchaka
Copy link
Member

I will wait for the 3.13 release before merging this PR because it will conflict with many backports waiting for 3.13. It would be easier to resolve conflicts in this PR (just move test_argparse.py) than in other PRs which modify the part of the file.

@serhiy-storchaka serhiy-storchaka self-assigned this Oct 2, 2024
@tomasr8
Copy link
Member Author

tomasr8 commented Oct 2, 2024

I will wait for the 3.13 release before merging this PR because it will conflict with many backports waiting for 3.13. It would be easier to resolve conflicts in this PR (just move test_argparse.py) than in other PRs which modify the part of the file.

Ok! Let me know if I can help with anything :)

@serhiy-storchaka
Copy link
Member

Don't worry, I'll merge this PR when the time comes.

@tomasr8
Copy link
Member Author

tomasr8 commented Oct 15, 2024

Don't worry, I'll merge this PR when the time comes.

Github was complaining about some merge conflicts so I just wanted to update the branch ;)

@savannahostrowski savannahostrowski linked an issue Oct 18, 2024 that may be closed by this pull request
This avoids having to turn tests into packages.
@tomasr8
Copy link
Member Author

tomasr8 commented Oct 27, 2024

On Serhiy's suggestion, I moved the translation data into a separate folder and merged the new tests with the existing ones to avoid turning the tests into a package.

@serhiy-storchaka serhiy-storchaka merged commit 0922a4a into python:main Oct 27, 2024
35 checks passed
@miss-islington-app
Copy link

Thanks @tomasr8 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @tomasr8 and @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0922a4ae0d2803e3a4e9f3d2ccd217364cfd700e 3.13

@miss-islington-app
Copy link

Sorry, @tomasr8 and @serhiy-storchaka, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0922a4ae0d2803e3a4e9f3d2ccd217364cfd700e 3.12

@bedevere-app
Copy link

bedevere-app bot commented Oct 27, 2024

GH-126046 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 27, 2024
@serhiy-storchaka serhiy-storchaka removed the needs backport to 3.12 only security fixes label Oct 27, 2024
@serhiy-storchaka serhiy-storchaka removed their assignment Oct 27, 2024
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Oct 27, 2024
…124803)

(cherry picked from commit 0922a4a)

Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
@tomasr8
Copy link
Member Author

tomasr8 commented Oct 27, 2024

There seems to be a buildbot failure related to this PR: https://buildbot.python.org/#/builders/1244/builds/3363
I think it's because scripts are not always included in python installations. We'll probably need to guard the test with skip_if_missing.

Edit: maybe something like this?

  @requires_subprocess()
+ @requires_tool('i18n')
  class TestTranslations(unittest.TestCase):

with this helper function in test_tools?

def requires_tool(tool=None):
    exists = True
    try:
        skip_if_missing(tool)
    except unittest.SkipTest:
        exists = False
    return unittest.skipUnless(exists, f'{tool} not available')

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Oct 27, 2024

Do you mind to create a PR? I paused backporting.

Simply call skip_if_missing() in the test. Or in setUpClass() if there were several tests.

@tomasr8
Copy link
Member Author

tomasr8 commented Oct 27, 2024

Do you mind to create a PR? I paused backporting.

Simply call skip_if_missing() in the test. Or in setUpClass() if there were several tests.

On it :)

@tomasr8
Copy link
Member Author

tomasr8 commented Oct 27, 2024

PR is here: #126051

@tomasr8 tomasr8 deleted the argparse-tests branch October 27, 2024 17:45
serhiy-storchaka added a commit that referenced this pull request Oct 27, 2024
…126046)

(cherry picked from commit 0922a4a)

Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Oct 27, 2024
…thonGH-124803) (pythonGH-126046)

(cherry picked from commit 0922a4a)

(cherry picked from commit ff044ed)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
serhiy-storchaka added a commit that referenced this pull request Oct 27, 2024
…126046) (GH-126054)

(cherry picked from commit 0922a4a)
(cherry picked from commit ff044ed)

Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add translation tests for argparse
3 participants