-
Notifications
You must be signed in to change notification settings - Fork 172
Implementation of equality for Settings class #464
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
Changes from 64 commits
ba33d81
dcf9a07
5d56989
a7cc38e
dbda648
8fb8355
c79a101
3e7eb7b
bbb3ffa
ba3cb01
d81e0c9
da5915f
d85e7ad
dbed3a5
a7332d6
df10df7
9639952
2fc7304
0463579
52a3d68
1d4be39
40e4189
773eb03
72ef00a
df6f48b
8c388e2
cb88c04
5517c30
8ff0e52
a899b76
62ce856
d49f486
586287a
d8f7e81
64a7dd2
dfcb40a
653a93a
1ebeca1
4bb58ed
450d1b2
e06d823
5331bca
4d8a287
95cd6dd
fe3e75c
032b172
0772d8f
28ddc0a
6fd23aa
bfd7c3b
f63c37c
16a01c5
73bf0d1
d8a096e
80524f1
abe5962
bb771c8
665a33b
76d4c38
d44ddde
43bc4c5
10a9380
b26bc3d
de2c6bc
675f3b8
1741a0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,6 +253,15 @@ def __iter__(self): | |
| def __len__(self): | ||
| return len(self._settings) | ||
|
|
||
| def __eq__(self, other): | ||
| if isinstance(other, Settings): | ||
| return self._settings == other._settings | ||
| else: | ||
| return NotImplemented | ||
|
|
||
| def __ne__(self, other): | ||
| return not self == other | ||
|
||
|
|
||
|
|
||
| def _validate_setting(setting, value): | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,3 +350,132 @@ def test_cannot_set_invalid_values_for_max_header_list_size(self, val): | |
|
|
||
| with pytest.raises(KeyError): | ||
| s[h2.settings.MAX_HEADER_LIST_SIZE] | ||
|
|
||
|
|
||
| class TestSettingsEquality(object): | ||
| """ | ||
| A class defining tests for the standard implementation of == and != . | ||
| """ | ||
|
|
||
| def an_instance(self): | ||
| """ | ||
| Return an instance of the class under test. Each call to this method | ||
| must return a different object. All objects returned must be equal to | ||
| each other. | ||
| """ | ||
| overrides = { | ||
| h2.settings.HEADER_TABLE_SIZE: 0, | ||
| h2.settings.MAX_FRAME_SIZE: 16384, | ||
| h2.settings.MAX_CONCURRENT_STREAMS: 4, | ||
| h2.settings.MAX_HEADER_LIST_SIZE: 2**16, | ||
| } | ||
| return h2.settings.Settings(client=True, initial_values=overrides) | ||
|
|
||
| def another_instance(self): | ||
| """ | ||
| Return an instance of the class under test. Each call to this method | ||
| must return a different object. The objects must not be equal to the | ||
| objects returned by anInstance. They may or may not be equal to | ||
| each other (they will not be compared against each other). | ||
| """ | ||
| overrides = { | ||
| h2.settings.HEADER_TABLE_SIZE: 8080, | ||
| h2.settings.MAX_FRAME_SIZE: 16388, | ||
| h2.settings.MAX_CONCURRENT_STREAMS: 100, | ||
| h2.settings.MAX_HEADER_LIST_SIZE: 2**16, | ||
| } | ||
| return h2.settings.Settings(client=False, initial_values=overrides) | ||
|
|
||
| def test_identical_eq(self): | ||
| """ | ||
| An object compares equal to itself using the == operator. | ||
| """ | ||
| o = self.an_instance() | ||
| assert (o == o) | ||
|
|
||
| def test_identical_ne(self): | ||
| """ | ||
| An object doesn't compare not equal to itself using the != operator. | ||
| """ | ||
| o = self.an_instance() | ||
| assert not (o != o) | ||
|
|
||
| def test_same_eq(self): | ||
| """ | ||
| Two objects that are equal to each other compare equal to each other | ||
| using the == operator. | ||
| """ | ||
| a = self.an_instance() | ||
| b = self.an_instance() | ||
| assert (a == b) | ||
|
|
||
| def test_same_ne(self): | ||
| """ | ||
| Two objects that are equal to each other do not compare not equal to | ||
| each other using the != operator. | ||
| """ | ||
| a = self.an_instance() | ||
| b = self.an_instance() | ||
| assert not (a != b) | ||
|
|
||
| def test_different_eq(self): | ||
| """ | ||
| Two objects that are not equal to each other do not compare equal to | ||
| each other using the == operator. | ||
| """ | ||
| a = self.an_instance() | ||
| b = self.another_instance() | ||
| assert not (a == b) | ||
|
|
||
| def test_different_ne(self): | ||
| """ | ||
| Two objects that are not equal to each other compare not equal to each | ||
| other using the != operator. | ||
| """ | ||
| a = self.an_instance() | ||
| b = self.another_instance() | ||
| assert (a != b) | ||
|
|
||
| def test_another_type_eq(self): | ||
| """ | ||
| The object does not compare equal to an object of an unrelated type | ||
| (which does not implement the comparison) using the == operator. | ||
| """ | ||
| a = self.an_instance() | ||
| b = object() | ||
| assert not (a == b) | ||
|
|
||
| def test_another_type_ne(self): | ||
| """ | ||
| The object compares not equal to an object of an unrelated type (which | ||
| does not implement the comparison) using the != operator. | ||
| """ | ||
| a = self.an_instance() | ||
| b = object() | ||
| assert (a != b) | ||
|
|
||
| def test_delegated_eq(self): | ||
| """ | ||
| The result of comparison using == is delegated to the right-hand | ||
| operand if it is of an unrelated type. | ||
| """ | ||
| class Delegate(object): | ||
| def __eq__(self, other): | ||
| return [self] | ||
|
|
||
| a = self.an_instance() | ||
| b = Delegate() | ||
| assert (a == b) == [b] | ||
|
|
||
| def test_delegate_ne(self): | ||
| """ | ||
| The result of comparison using != is delegated to the right-hand | ||
| operand if it is of an unrelated type. | ||
| """ | ||
| class Delegate(object): | ||
| def __ne__(self, other): | ||
| return [self] | ||
|
|
||
| a = self.an_instance() | ||
| b = Delegate() | ||
| assert (a != b) == [b] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entire class has its method names in camel case (that is, with the bumpy bits). It shouldn't: it should be snake case (with underscores). That means you need to change each method name appropriately. For example, |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
otherisn't aSettingsobject we can pretty easily say we're not equal. Should returnFalsein this case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @SethMichaelLarson, but again I disagree. =) Per the documentation,
NotImplementedshould be returned when we have no method of determining equality with respect to the other type. This is to allow other rich comparison methods to potentially be invoked that may know how to do an equality comparison.The correct thing to do here is to return
NotImplementedwhen we don't have matching types.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lukasa Huh! TIL that
NotImplementedexists. I never knew of this construct. I will keep this in mind! Thanks! :)