classification
Title: Test OrderedDict subclass
Type: behavior Stage: resolved
Components: Extension Modules, Tests Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: eric.smith, eric.snow, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-10-20 21:12 by serhiy.storchaka, last changed 2015-11-04 20:40 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
odict_subclass_test.patch serhiy.storchaka, 2015-10-20 21:12 review
odict_subclass_test_2.patch serhiy.storchaka, 2015-10-21 17:29 review
Messages (7)
msg253254 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-20 21:12
C implementation of OrderedDict often has two paths. One for exact OrderedDict and other for subclasses. But only one of the paths is tested.
Proposed patch adds tests for a subclass of C implemented OrderedDict.

test_issue24347 is failed with OrderedDict subclass. Either there is a bug in OrderedDict implementation, or the test should be more lenient.
msg253260 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-10-20 21:47
LGTM as long as you also add PurePythonOrderedDictSubclassTests to match CPythonOrderedDictSubclassTests (per PEP 399).
msg253277 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-21 06:32
The patch was not ready for commit. The problem is that test_issue24347 fails in CPythonOrderedDictSubclassTests. The failure is random, you need to run test several times to encounter it.

Experimenting with this test I found other bug probably related to issue24347.

>>> from collections import OrderedDict
>>> od = OrderedDict()
>>> dict.__setitem__(od, 1, 2)
>>> od
OrderedDict([<NULL>])

I expected rather raising an exception or showing an empty OrderedDict, that exposing NULL.
msg253292 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-10-21 15:29
Regarding dict.__setitem__, see issue #24726.  Raymond outlined what needs to be fixed.
msg253302 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-21 17:29
New patch adds PurePythonOrderedDictSubclassTests, moves all tests except test_key_change_during_iteration from CPythonOrderedDictTests to OrderedDictTests to test Python implementation too, fixes a bug in values and items iteration that caused test_issue24347 to fail, and adds additional explicit checks for values() and items() to test_issue24347.
msg254068 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-11-04 20:34
New changeset 499398d91b97 by Serhiy Storchaka in branch '3.5':
Issue #25449: Iterating OrderedDict with keys with unstable hash now raises
https://hg.python.org/cpython/rev/499398d91b97

New changeset 9db4191723eb by Serhiy Storchaka in branch 'default':
Issue #25449: Iterating OrderedDict with keys with unstable hash now raises
https://hg.python.org/cpython/rev/9db4191723eb

New changeset 59c7615ea921 by Serhiy Storchaka in branch '3.5':
Issue #25449: Fixed a crash and leaking NULL in repr() of OrderedDict that
https://hg.python.org/cpython/rev/59c7615ea921

New changeset 76e848554b5d by Serhiy Storchaka in branch 'default':
Issue #25449: Fixed a crash and leaking NULL in repr() of OrderedDict that
https://hg.python.org/cpython/rev/76e848554b5d
msg254070 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-04 20:40
59c7615ea921 and 76e848554b5d were related to issue24726.
History
Date User Action Args
2015-11-04 20:40:14serhiy.storchakasetstatus: open -> closed
messages: + msg254070

assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2015-11-04 20:34:10python-devsetnosy: + python-dev
messages: + msg254068
2015-10-21 17:29:44serhiy.storchakasetfiles: + odict_subclass_test_2.patch
type: enhancement -> behavior
messages: + msg253302
2015-10-21 15:29:44eric.snowsetstage: commit review -> patch review
2015-10-21 15:29:37eric.snowsetmessages: + msg253292
2015-10-21 06:32:00serhiy.storchakasetmessages: + msg253277
2015-10-20 21:47:49eric.snowsetnosy: + eric.snow

messages: + msg253260
stage: commit review
2015-10-20 21:12:18serhiy.storchakacreate