gh-69714: Make calendar module fully tested
#93655
Conversation
…}Calendar(locale='')`
….Locale{Text|HTML}Calendar`
There are 3 paths to use `locale` argument in
`calendar.Locale{Text|HTML}Calendar.__init__(..., locale=None)`:
(1) `locale=None` -- denotes the "default locale"[1]
(2) `locale=""` -- denotes the native environment
(3) `locale=other_valid_locale` -- denotes a custom locale
So far case (2) is covered and case (1) is in 78935da (same branch).
This commit adds a remaining case (3).
[1] In the current implementation, this translates into the following
approach:
GET current locale
IF current locale == "C" THEN
SET current locale TO ""
GET current locale
ENDIF
This condition cannot be true. `_locale.setlocale()` from the C module raises `locale.Error` instead of returning `None` for `different_locale.__enter__` (where `self.oldlocale` is set).
This patch has already been applied to `main` branch (via pythongh-93468), but with wrong copyright. After merging this commit to `main`, git `Author` metadata will be updated to the original author.
|
The following commit authors need to sign the Contributor License Agreement: Click the button to sign: |
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
AA-Turner
added
stdlib
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
|
You will need a NEWS entry, along the lines of A |
Thanks! I just added it. Another problem is the lack of CLA signatures from @jesstess and @rohitmediratta. The applied patches were quite old, so I'm not sure the email addresses are up to date (I don't know if the bot is sending emails about the CLA). Anyway, it looks like GitHub has assigned these commits to the correct usernames, so I'm tagging Jessica and Rohit in this comment to help notify :) |
|
You'll need to fix the Ubuntu failures in Traceback (most recent call last):
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_calendar.py", line 650, in test_locale_calendar_formatmonthname
self.assertEqual(cal.formatmonthname(2022, 6, 2, withyear=False), "June")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/calendar.py", line 588, in formatmonthname
with different_locale(self.locale):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/calendar.py", line 555, in __enter__
_locale.setlocale(_locale.LC_TIME, self.locale)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/locale.py", line 626, in setlocale
return _setlocale(category, locale)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
locale.Error: unsupported locale settingThis may be another test-skipping scenario, I don't know what the values it is trying to set are though so I can't guess much further. A |
You might also want to consider emailing the two of them if you don't hear back, neither seem to have much recent publicly shown activity on GH. A |
Or maybe (based on the traceback) it's related to one of the corner cases from #93655 (comment)? |
…verage)" This reverts commit a96786c.
Sadly it doesn't seem to have worked :( A |
…()`. This method temporarily changes the current locale to the given locale, so `_locale.setlocale()` may raise `local.Error`.
Yes, I misread the traceback in a rush before disconnecting, but I wanted to kick off builds to get results when I get back. Time saving ;) For now, I have restored a96786c. However, it is true that the cause of the problem is related to #31214 and as you correctly predicted, this is another test-skipping scenario. Fixed in 2b6f1f9. |
|
|
No need to process the function. This also helps in supporting CLI testing.
These tests are now part of `test_illegal_arguments`. This reverts commit 238c527.
Fixed in 8b7bc06. |
|
Ok, all comments are now addressed and resolved. All tests pass successfully. ✔️ I think the PR is ready for another round of reviews ;-) |
|
Good news for the weekend. Rohit signed the CLA. Thanks again @rohitmediratta! I also looked at @jesstess GH profile and I believe the missing CLA has already been signed by Jessica as she is a former director of the Python Software Foundation and a member of @python. I've emailed contributions@python.org to sort this out. |
|
Here is the holdup situation: When moved to gh, issue became #57539. Irit Katriel requested gh PR and mentioned this issue. On June 9, bxsx added ab069a4, which added Jessica's original code, without the Serhiy correction. It somehow credited Jessica as author. Fortunately, the deletion of the correction is somehow not in the merged patch. Unfortunately, it fooled the CLA bot into thinking that she was an author of this PR. @ambv This PR, which has had a lot of work by multiple people, is blocked by the false 'need' for an unobtainable signature from @jesstess Jessica McKeller, who did not contribute to this PR. Should reverting ab069a4 fix the mistake or is something else needed. If so, can you override the bot? (Though a final review may be needed.) |
Summary
Resolves #69714 by increasing test coverage of
calendarmodule to 100%[1].Coverage reports
Before
mainbranch test coverage report forcalendarmodule:After
The PR branch test coverage report for
calendarmodule:Footnotes
[1] The remaining three lines are:
and they are, in fact, already covered by tests (only not reported by coveragepy).