Repository navigation
Python implementation of datetime module is not being tested correctly. #75005
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-07-26.11:52:55.129>
created_at = <Date 2017-06-30.23:47:48.837>
labels = ['3.7', 'type-bug', 'tests']
title = 'Python implementation of datetime module is not being tested correctly.'
updated_at = <Date 2017-09-08.22:44:35.784>
user = 'https://github.com/musically-ut'bugs.python.org fields:
activity = <Date 2017-09-08.22:44:35.784>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-07-26.11:52:55.129>
closer = 'vstinner'
components = ['Tests']
creation = <Date 2017-06-30.23:47:48.837>
creator = 'musically_ut'
dependencies = []
files = []
hgrepos = []
issue_num = 30822
keywords = []
message_count = 48.0
messages = ['297455', '297470', '297515', '297517', '297518', '297519', '297520', '297522', '297523', '297524', '297525', '297530', '297546', '297548', '297554', '297555', '297557', '297559', '297562', '297571', '297573', '297612', '297734', '297736', '297743', '297752', '297753', '297760', '297762', '297763', '298032', '298033', '298059', '298664', '298717', '298721', '298725', '298726', '298729', '298732', '298755', '298756', '298786', '299221', '299222', '299223', '299224', '301749']
nosy_count = 5.0
nosy_names = ['belopolsky', 'vstinner', 'ned.deily', 'serhiy.storchaka', 'musically_ut']
pr_nums = ['2530', '2534', '2550', '2551', '2588', '2775', '2777', '2781', '2782', '2783', '2788', '2815', '2816', '3405']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue30822'
versions = ['Python 3.6', 'Python 3.7']
Activity
musically-ut commentedon Jun 30, 2017
While investigating http://bugs.python.org/issue30302 it was discovered that the Python implementation of the datetime module was not being tested correctly.
The datetimetester.py was supposed to execute tests twice, once for the _Fast implementation (i.e. C extension code in _datetimemodule.c) and once for the _Pure implementation (i.e the Python code).
The fix is contained in the following two commits:
The bug does not effect Python 2.7 as the C version of the datetime module had not been introduced back then. However, as fas as I can tell, the fix needs to be applied to all Python 3.x branches.
serhiy-storchaka commentedon Jul 1, 2017
Please provide a separate PR for this. After merging it with the master branch it should be backported to 3.6 and 3.5 (3.4 and 3.3 are for security only fixes). Add your name in Misc/ACKS. Changes in Misc/NEWS.d/ are not required.
serhiy-storchaka commentedon Jul 2, 2017
New changeset 98b6bc3 by Serhiy Storchaka (Utkarsh Upadhyay) in branch 'master':
bpo-30822: Fix testing of datetime module. (bpo-2530)
98b6bc3
musically-ut commentedon Jul 2, 2017
Are there any guides re: backporting commits?
Just to confirm verify that I'm going about it right; I’m planning on cherry-picking relevant commits back to branch 3.5, 3.6, resolving the merge conflicts, and then opening two separate PRs for them with
bpo-30822in their titles.Sounds about right?
~
ut
ned-deily commentedon Jul 2, 2017
Serhiy, it appears that the added tests are taking a long time to run and thus causing many buildbot failures, for example:
http://buildbot.python.org/all/builders/ARMv7%20Ubuntu%203.x/builds/1013
serhiy-storchaka commentedon Jul 2, 2017
Oh, test.datetimetester.ZoneInfoTest[Europe/Andorra]_Pure_Pure_Pure -- something looks wrong.
musically-ut commentedon Jul 2, 2017
I can reproduce the names with
_Pure_Pure_Pureif I run the tests with-utzdata. Investigating.~
ut
serhiy-storchaka commentedon Jul 2, 2017
The quick solution -- just deduplicate test classes.
But the code needs rewriting. Current code is tricky and fragile. It doesn't work with --list-cases (and I suppose it doesn't work well with other advanced unittest features). This would be not easy.
serhiy-storchaka commentedon Jul 2, 2017
All right. See https://docs.python.org/devguide/committing.html#backporting-changes-to-an-older-version . You can cherry-pick manually or using cherry_picker.py which adds correct prefix to commit message and helps to create a pull request.
26 remaining items
vstinner commentedon Jul 20, 2017
New changeset 5b392bb by Victor Stinner in branch 'master':
bpo-30822: Exclude tzdata from regrtest --all (bpo-2775)
5b392bb
vstinner commentedon Jul 20, 2017
New changeset 96ef06f by Victor Stinner in branch '3.6':
bpo-30822: Exclude tzdata from regrtest --all (bpo-2775) (bpo-2777)
96ef06f
vstinner commentedon Jul 20, 2017
New changeset 645e503 by Victor Stinner in branch '3.5':
bpo-30822: Exclude tzdata from regrtest --all (bpo-2775) (bpo-2781)
645e503
vstinner commentedon Jul 20, 2017
(Oops, I gone too fast in my 3.5 backport. tzdata was only introduced in Python 3.6, for me it was something older. I wrote a new fix for 3.5, to only add extralargefile, but don't add tzdata.)
vstinner commentedon Jul 20, 2017
New changeset bf3a1e9 by Victor Stinner in branch '3.5':
bpo-30822: regrtest: remove tzdata (bpo-2782)
bf3a1e9
vstinner commentedon Jul 21, 2017
New changeset 80ebc43 by Victor Stinner in branch '2.7':
bpo-30822: regrtest: fix -u extralargefile (bpo-2788)
80ebc43
vstinner commentedon Jul 21, 2017
New changeset 287c559 by Victor Stinner (Utkarsh Upadhyay) in branch 'master':
bpo-30822: Fix testing of datetime module. (bpo-2530) (bpo-2783)
287c559
vstinner commentedon Jul 21, 2017
Utkarsh comment on the PR:
Yeah, sorry about this awful view :-) We now have a mailing list getting notifications when a buildbot fails:
https://mail.python.org/mm3/mailman3/lists/buildbot-status.python.org/
We got two new failures since yesterday on Windows buildbots, but it's related to IDLE and so unrelated to your change. So a quick check says that test_datetime doesn't time out anymore!
Previously, the test failed on a few buildbots. I checked the test output to look if test_datetime became the new slowest test and ... no. For example, on PPC64 Fedora 3.x, test_datetime is not even listed in the "10 slowest tests", which means that it took 36 sec or less, whereas the test took longer than 15 minutes when tzdata was used.
IMHO you can already cook a first backport for 3.6, but I would prefer to wait another day just to make sure to everything is fine (to wait for new buildbot builds).
vstinner commentedon Jul 26, 2017
New changeset c52cea4 by Victor Stinner (Utkarsh Upadhyay) in branch '3.6':
[3.6] bpo-30822: Fix testing of datetime module. (GH-2530) (GH-2783) (bpo-2816)
c52cea4
vstinner commentedon Jul 26, 2017
Ok, I merged the change in the 3.6 branch.
I decided to not merge the 3.5 change, since it's now too late: 3.5 slowly enters a new security-only fixes:
#2815
Thanks Utkarsh Upadhyay for your work on datetime, not only this issue, but also repr(timedelta) (and maybe also timedelta constructor doc later? ;-)).
musically-ut commentedon Jul 26, 2017
Thanks Victor! \o/
Secretly, I'm just happy that my legacy will not be a commit which broke all (?) the build-bots and had to be reverted. :P
W.r.t. the docs; in retrospect, that'll probably have a larger impact on the end-users and is less likely to cause disagreement. I probably should have led with that. :)
~
ut
vstinner commentedon Jul 26, 2017
Reverting is a new policy that we decided last June:
https://mail.python.org/pipermail/python-committers/2017-June/004588.html
Sorry that your commit was the first guilty, but honestly, it was a wise decision since it allowed to discuss how to handle tzdata without the pressure of broken buildbots. Nowadays, Python relies much more on CI for pull requests than before the migration to GitHub.
Well, read the thread if you want to know the full rationale.
vstinner commentedon Sep 8, 2017
New changeset 3892799 by Victor Stinner (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-30822: Deduplicate ZoneInfoTest classes in test_datetime. (GH-2534) (bpo-3405)
3892799