Skip to content

gettext: remove unecessary test cases testing single/double quotes #107510

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

Closed
tomasr8 opened this issue Jul 31, 2023 · 6 comments
Closed

gettext: remove unecessary test cases testing single/double quotes #107510

tomasr8 opened this issue Jul 31, 2023 · 6 comments
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@tomasr8
Copy link
Member

tomasr8 commented Jul 31, 2023

Tests for the gettext module include cases which test both single and double quotes:

def test_double_quotes(self):
eq = self.assertEqual
# double quotes
eq(_("albatross"), 'albatross')
eq(_("mullusk"), 'bacon')
eq(_(r"Raymond Luxury Yach-t"), 'Throatwobbler Mangrove')
eq(_(r"nudge nudge"), 'wink wink')

As the comment below suggests, these are not needed to properly test gettext as we are only testing translation lookup. The distinction between single and double quotes would be relevant for testing string extraction but that is not part of gettext. I suggest we remove these test cases.

# TODO:
# - Add new tests, for example for "dgettext"
# - Remove dummy tests, for example testing for single and double quotes
# has no sense, it would have if we were testing a parser (i.e. pygettext)

Linked PRs

@tomasr8 tomasr8 added the type-feature A feature request or enhancement label Jul 31, 2023
@serhiy-storchaka
Copy link
Member

It is not so simply. Yes, the comment suggests to remove these tests, but they should be not syply removed, but replaced with proper tests for pygettext.

So I suggest to leave the tests and the comment as a reminder.

@tomasr8
Copy link
Member Author

tomasr8 commented Oct 8, 2023

If that's the case, would it not make more sense to remove the tests and just update the comment then? So we keep the reminder but not the unnecessary tests

@tomasr8
Copy link
Member Author

tomasr8 commented Oct 9, 2023

FWIW, I have a PR which adds more tests to pygettext as well: #108173 Feel free to have a look :)

@iritkatriel iritkatriel added the extension-modules C modules in the Modules dir label Dec 1, 2023
@tomasr8
Copy link
Member Author

tomasr8 commented Dec 14, 2024

Now that tests have been added to pygettext, perhaps it's time to revisit this? @serhiy-storchaka

@serhiy-storchaka
Copy link
Member

Sure.

serhiy-storchaka pushed a commit that referenced this issue Feb 14, 2025
There are now separate pygettext tests.
@StanFromIreland
Copy link
Contributor

Can this be closed? They have been removed and replaced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement
Projects
Status: Done
Development

No branches or pull requests

4 participants