Skip to content

Add troubleshooting chapter for tmp_path caveat #1145

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 16, 2025
Merged

Add troubleshooting chapter for tmp_path caveat #1145

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions docs/troubleshooting.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@jfindlay jfindlay Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the wording here should be stronger:

You do not need tmp_path if you are using pyfakefs. Both are methods designed to 1) protect the host filesystem and 2) guarantee a reproducible environment for each test case. Both tmp_path and pyfakefs are completely adequate alone and combining them is unnecessary. Tests that use both are likely to introduce unnecessary complexity and confusion. For example, ..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@jfindlay jfindlay Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Mixing pyfakefs with other testing affordances within the testing code that insulate the test case from the filesystem.
  2. 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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_path and 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_open in 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

------------------------------
If you are using the ``tmp_path`` fixture, or a similar pytest fixture
relying on the real filesystem, you may have the opposite problem: now the ``fs``
fixture must be added *after* the ``tmp_path`` fixture. Otherwise, the path will be
created in the fake filesystem, and pytest will later try to access it in the real
filesystem.

.. code:: python

def test_working(tmp_path, fs):
fs.create_file(tmp_path / "foo")


def test_not_working(fs, tmp_path):
# causes an error while pytest tries to access the temporary directory
pass

Note though that ``tmp_path`` and similar fixtures may not make much sense in the
first place if used with ``pyfakefs``. While the directory will be created and
removed in the real filesystem, all files you create will live in the fake filesystem
only. You could as well use hardcoded paths in your tests, as they will not interfere
with paths created in other tests.

Pathlib.Path objects created outside of tests
---------------------------------------------
A pattern which is seen more often with the increased usage of ``pathlib`` is the
Expand Down
Loading