Skip to content

bpo-30822: Fix testing of datetime module. #2530

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 6 commits into from
Jul 2, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Lib/datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -2271,7 +2271,8 @@ def _name_from_offset(delta):
_check_tzinfo_arg, _check_tzname, _check_utc_offset, _cmp, _cmperror,
_date_class, _days_before_month, _days_before_year, _days_in_month,
_format_time, _is_leap, _isoweek1monday, _math, _ord2ymd,
_time, _time_class, _tzinfo_class, _wrap_strftime, _ymd2ord)
_time, _time_class, _tzinfo_class, _wrap_strftime, _ymd2ord,
_divide_and_round)
# XXX Since import * above excludes names that start with _,
# docstring does not get overwritten. In the future, it may be
# appropriate to maintain a single module level docstring and
Expand Down
7 changes: 4 additions & 3 deletions Lib/test/datetimetester.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def test_constants(self):
self.assertEqual(datetime.MAXYEAR, 9999)

def test_name_cleanup(self):
if '_Fast' not in str(self):
if self._test_type == 'Pure':
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just test '_Fast' not in self.__class__.__name__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather have one property serve one purpose: __qualname__ to show correct class names in verbose mode (cosmetic) and _test_type for determining what kind of test it is (functional). This would, hopefully, prevent future cosmetic changes from interfering with the functional parts.

However, if you think "_Fast" in self.__class__.__qualname__ will minimize changes, I'll be happy to change it.

PS: I believe at some point in time, unittest used the cls.__name__ in verbose mode instead of cls.__qualname__. This would explain the change in the outputs which surprised @Haypo. It would be ideal if such cosmetic changes do not break other parts of the code.

Copy link
Member

Choose a reason for hiding this comment

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

In the past __qualname__ didn't exist. repr(self) contained class' __name__. After introducing __qualname__ that tests were not updated.

I think it would be better to modify both __name__ and __qualname__ (they should be consistent). _test_type is redundant since required information can be extracted from __name__.

return
Copy link
Member

Choose a reason for hiding this comment

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

Please replace return with self.skipTest(...). It is better if the skipped test reported as skipped rather than passed.

datetime = datetime_module
names = set(name for name in dir(datetime)
Expand All @@ -72,8 +72,9 @@ def test_name_cleanup(self):
self.assertEqual(names - allowed, set([]))

def test_divide_and_round(self):
if '_Fast' in str(self):
if self._test_type == 'Fast':
return

dar = datetime_module._divide_and_round

self.assertEqual(dar(-10, -3), 3)
Expand Down Expand Up @@ -2851,7 +2852,7 @@ def tzname(self, dt): return self.tz
self.assertRaises(TypeError, t.strftime, "%Z")

# Issue #6697:
if '_Fast' in str(self):
if self._test_type == 'Fast':
Badtzname.tz = '\ud800'
self.assertRaises(ValueError, t.strftime, "%Z")

Expand Down
8 changes: 5 additions & 3 deletions Lib/test/test_datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
# XXX(gb) First run all the _Pure tests, then all the _Fast tests. You might
# not believe this, but in spite of all the sys.modules trickery running a _Pure
# test last will leave a mix of pure and native datetime stuff lying around.
test_classes = []
all_test_classes = []

for module, suffix in zip(test_modules, test_suffixes):
test_classes = []
Expand All @@ -33,7 +33,8 @@
suit = cls()
test_classes.extend(type(test) for test in suit)
for cls in test_classes:
cls.__name__ = name + suffix
cls.__qualname__ += suffix
Copy link
Member

Choose a reason for hiding this comment

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

Add the suffix also to __name__.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

cls._test_type = suffix[1:]
@classmethod
def setUpClass(cls_, module=module):
cls_._save_sys_modules = sys.modules.copy()
Expand All @@ -46,9 +47,10 @@ def tearDownClass(cls_):
sys.modules.update(cls_._save_sys_modules)
cls.setUpClass = setUpClass
cls.tearDownClass = tearDownClass
all_test_classes.extend(test_classes)

def test_main():
run_unittest(*test_classes)
run_unittest(*all_test_classes)

if __name__ == "__main__":
test_main()
1 change: 1 addition & 0 deletions Misc/ACKS
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,7 @@ Doobee R. Tzeck
Eren Türkay
Lionel Ulmer
Adnan Umer
Utkarsh Upadhyay
Roger Upole
Daniel Urban
Michael Urman
Expand Down