Repository navigation
Add tests for collection mixins (see issue #2531) - #2535
DiegoBaldassarMilleuno wants to merge 2 commits into
Conversation
Sorry, something went wrong.
|
|
||
| def test_len(self): | ||
| assert type(len(self.dct)) is int | ||
| assert len(self.dct) == 4 if not self.nullable else 5 |
There was a problem hiding this comment.
This line is very hard to read
Sorry, something went wrong.
There was a problem hiding this comment.
| assert len(self.dct) == 4 if not self.nullable else 5 | |
| expected_length = 4 if not self.nullable else 5 | |
| assert len(self.dct) == expected_length |
Something like this?
Sorry, something went wrong.
| def test_contains(self): | ||
| assert self.ktype(10) in self.dct | ||
| assert self.ktype(50) not in self.dct | ||
| assert 12.34 not in self.dct |
There was a problem hiding this comment.
We need to pay attention here because .NET semantics might be different from Python semantics. In .NET Dictionary is a ICollection of KeyValuePair, so it has different Contains based on that interface.
Basically need a 2x2 explicit asserts: 2 for key vs KVP and 2 for in vs not in
Sorry, something went wrong.
There was a problem hiding this comment.
@lostmsu standard Python semantics for mappings want key in mapping to check for keys (equivalent of ContainsKey) and kvp in mapping.items() to check for key-value pairs (as tuples, otherwise analogous to Contains); they also have a mapping be an iterable of its keys, so key in mapping is equivalent to key in list(mapping).
Like you said, this is different from .NET where an IDictionary<TKey,TValue> is an ICollection<KeyValuePair<TKey,TValue>>.
I'm for using Python semantics when using Python operators and .NET semantics when using .NET methods, both for interoperability with existing libraries and to avoid confusion.
That said I could add tests checking that .items() returns an object implementing ItemsView (also see here), including what you said.
Would that be ok?
Sorry, something went wrong.
| with pytest.raises(Exception): | ||
| dct[12.34] = self.vtype(0) | ||
|
|
||
| @pytest.mark.xfail(reason='Known to crash', run=False) |
There was a problem hiding this comment.
Address that?
Sorry, something went wrong.
There was a problem hiding this comment.
Ok, it doesn't crash anymore, will reenable this and test_delitem_raise next commit
Sorry, something went wrong.
Adding testing for adherence to collections.abc protocols for the following classes: Array, List, ImmutableArray, ImmutableList, Dictionary, ImmutableDictionary and ReadOnlyDictionary Tests for Python list and dict are also present as a reference but commented out.
Also made sequence __deitem__ test not rely on negative indexes
As requested in #2531 (comment), submitting a draft pull request testing for compliance to collections.abc protocols by a few standard containers.
Currently many tests fail.
A few notes: