Add troubleshooting chapter for tmp_path caveat #1145
Conversation
mrbean-bremen
force-pushed
the
doc-tmp_path
branch
from
841bf16 to
b2708c6
Compare
| @@ -266,6 +266,30 @@ passed before the ``mocker`` fixture to ensure this: | |||
| # works correctly | |||
| mocker.patch("builtins.open", mocker.mock_open(read_data="content")) | |||
|
|
|||
| tmp_path fixture with pyfakefs | |||
There was a problem hiding this comment.
I wonder if the wording here should be stronger:
You do not need
tmp_pathif you are usingpyfakefs. Both are methods designed to 1) protect the host filesystem and 2) guarantee a reproducible environment for each test case. Bothtmp_pathandpyfakefsare completely adequate alone and combining them is unnecessary. Tests that use both are likely to introduce unnecessary complexity and confusion. For example, ..."
There was a problem hiding this comment.
Merged too soon...
Yes, thanks - I think you are right. I was wondering about it while I was writing it... There should be a general caution about fixtures relying on the real filesystem. I'll probably make a new PR tonight and this time wait for a review - or you could make one yourself if you want.
There was a problem hiding this comment.
Merged too soon...
Not necessarily. I read through the full section and it makes more sense in context. The idea formed in my mind many hours after reading the updated documentation and software is iterative and usually incomplete. Thanks for the consideration.
There should be a general caution about fixtures relying on the real filesystem
For me the main concerns cover two distinct cases:
- Mixing
pyfakefswith other testing affordances within the testing code that insulate the test case from the filesystem. - Writing code such that the unit test can completely mock all access points to files.
The second is much more complex and philosophical as, at least for python, this implies the user has to actually write their code differently so that it can be tested (quite an imposition: complex or clever code requires awful contortions/tenuous introspections/egregious antipatterns just to cover the happy path with unit tests). However, can pyfakefs give the user enough functionality such that they could reasonably cover any defensibly written code that is amenable to (unit) testing, for example, by sending in FakePath objects where the code expects Path, or does tmp_path have a fundamental advantage because it is simpler to rewrite the path to point into a safe (though real) test dir than fake the entire filesystem? I think the latter is better in practice and in principle, but includes more complexity and I'm still trying to work out any further limitations to that approach.
I think the first case is simpler (though not completely orthogonal to the second) as it's easier to just proscribe tmp_path and rely on fs for filesystem abstractions in the test code itself.
There was a problem hiding this comment.
Writing code such that the unit test can completely mock all access points to files.
I think this is a bit too specific. I certainly agree that code should be written with testability in mind (which generally means modular code, single responsibility etc., which are all good things independently of testing), but it shouldn't be needed to code for the shortcomings of a specific testing tool or library.
So I would probably stick to the first point. Generally, I see two points worth mentioning:
- avoid the usage of
tmp_pathand the like that rely on the real fs, as they are not needed (your point 1) - make sure that other file system based libraries (fixtures, in this case) that are working on the test filesystem are fully mocked; point in case is
mock_openin the previous section (actually not the best example, as it is also not really needed with pyfakefs) - maybe that section has to be adapted a bit to make this more clear
Tasks