bpo-40129: Add fake number classes in test.support. #19262
Conversation
serhiy-storchaka
requested review from
abalkin,
pganssle and
rhettinger
as code owners
serhiy-storchaka
force-pushed
the
test-support-fake-numbers
branch
from
7ccc57f to
cac62dd
Compare
There was a problem hiding this comment.
Thanks for doing this. The idea seems sound and the datetime implementation seems right.
One suggestion: you should add documentation for this. There is documentation for test.support, but even a block comment before Fake would be helpful to give some additional context to future maintainers and devs about why it's there and when and where you should use it.
| __complex__ = Fake._return | ||
|
|
||
| class FakePath(Fake): | ||
| """Simple implementing of the path protocol. |
There was a problem hiding this comment.
| """Simple implementing of the path protocol. | |
| """Simple implementation of the path protocol. |
|
|
||
| class FakePath(Fake): | ||
| """Simple implementing of the path protocol. | ||
| """ |
There was a problem hiding this comment.
Should this be on the previous line? Looks weird to have it sitting on its own following a short one-line docstring like this.
| def f(): return 7 | ||
|
|
||
| values = [INT(9), IDX(9), | ||
| values = [FakeInt(9), FakeIndex(9), |
There was a problem hiding this comment.
I don't quite understand the name, there's nothing really fake about these classes. Also, it's nice to see what the classes do without leaving the file.
There was a problem hiding this comment.
I agree with both points here.
When scanning through the PR on GitHub and seeing FakeInt(10.9), my first thought is that I don't know what that means without guessing, so I want to quickly find the FakeInt definition. That's harder when that definition is in another file. (And yes, if you have a decent editor open and set up properly, the definition should be just a click or keystroke away, but not everyone has that setup available all of the time.)
The problem could potentially be mitigated with a name that more clearly describes what the class is doing (HasDunderIntReturning(3.2)?), but as you can tell I'm having trouble coming up with a name that's going to be immediately unambiguous for people reading the code without also being unwieldy.
|
IMO tests should be primitive and locally understandable. If they get too fancy, it's easy to lose track of what is being tested. So I'm not sure if much is gained here. |
|
The main purpose is to make easier to test that the specified function supports an object that implements I used name |
https://bugs.python.org/issue40129