docs: Fix references to recursive seal of Mocks #9028
Conversation
Lib/unittest/mock.py
Outdated
| parent. If a mock is assigned to an attribute of an existing mock, | ||
| it is not considered a submock. | ||
|
|
||
| The operation seals the mock passed in and all its children recursively. |
There was a problem hiding this comment.
Can you add here an explanation of what "child mocks" are? The documentation does not clarify exactly what a child mock is and this can be confusing.
mariocj89
force-pushed
the
fix_mock_seal_docs
branch
from
fc95aba
to
0f429c1
Compare
Lib/unittest/mock.py
Outdated
|
|
||
| The operation seals the mock passed in, all the mocks generated | ||
| by accessing an attribute of the mock argument and all assigned | ||
| mocks without a name or a `spec` recursively. |
There was a problem hiding this comment.
Is this supposed to be RST or md? If it's RST I think it needs to be
``spec``instead of:
`spec`I also find the sentence confusing and don't really understand what it means.
There was a problem hiding this comment.
I think you can re-word like this:
The operation recursively seals the mock passed in, meaning that the mock itself, any mocks generated by accessing one of its attributes, and all assigned mocks without a name or
specwill be sealed.
|
Although as I noted I find one sentence a bit confusing, there is nothing grammatically wrong with these documentation changes (in case you want a native English speaker's Seal of Approval). |
mariocj89
force-pushed
the
fix_mock_seal_docs
branch
from
0f429c1
to
d1a6614
Compare
|
I do always want indeed a Native English speaker's SoA. Please have another look :) |
|
@pablogsal I've updated the PR in case you want to re-review. As it was awaiting for merge and want to make sure you are OK with the changes. |
Lib/unittest/mock.py
Outdated
|
|
||
| The operation recursively seals the mock passed in, meaning that | ||
| the mock itself, any mocks generated by accessing one of its attributes, | ||
| and all assigned mocks without a name or spec. |
There was a problem hiding this comment.
The "that" makes this sentence not make sense. I think you need to add "will be sealed" at the end of this.
As the sealing of the mock hierarchy is based on whether the mock has the previous one as a parent or not, remove references to the undefined "submock" concept and explain `seal` just in term of child mocks.
The documentation shows an example where using a new mock prevents `seal` from applying recursively but for that to work the `Mock` instance needs to have a name or a spec. The docs in `library/unittest.mock` have been updated to remove confusing terms about submock and be explicit about the behavior expected.
mariocj89
force-pushed
the
fix_mock_seal_docs
branch
from
d1a6614
to
0eef5c0
Compare
|
Thanks @mariocj89 for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
GH-9974 is a backport of this pull request to the 3.7 branch. |
|
@mariocj89: I simplified the commit message. I'm not sure that it's really useful to elaborate more inside the commit message, the change speaks by itself :-) I also adjusted the commit title. Sorry that the commit stayed open for so long. Thanks again for this nice unittest.mock feature ;-) |
The docs in `library/unittest.mock` have been updated to remove confusing terms about submock and be explicit about the behavior expected. (cherry picked from commit 96200eb) Co-authored-by: Mario Corchero <mariocj89@gmail.com>
|
Thank you! And absolutely no reason for apologies, things take time. |
The documentation references the concept of "submocks" when just referring to child mocks, simplify the docstring and update the example as it assigns the mock without a name, which makes it a valid child mock and the seal recurses into it.
This was misinterpreted at the time of the initial PR as in my tests I've always named all the mocks 😞 .
Happy to open an issue if you think it is worth it.