Skip to content
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

Inconsistent PurePosixPath / PureWindowsPath behavior #1053

Closed
camillobruni opened this issue Aug 26, 2024 · 14 comments
Closed

Inconsistent PurePosixPath / PureWindowsPath behavior #1053

camillobruni opened this issue Aug 26, 2024 · 14 comments
Labels

Comments

@camillobruni
Copy link
Contributor

Describe the bug

It seems like is_absolute and __str__ don't behave exactly the same betwen pathlib.PureWindowsPath and pyfakefs.PureWindowsPath. Likely there are more methods that suffer from the same issue.

This is the output of the added tests in fake_pathlib_test.py. I hope I'm holding this the right way :).

======================================================================
FAIL: test_windows_pure_path_is_absolute (__main__.FakePathlibModulePurePathTest)
Verify faked pure POSIX paths are formatted using POSIX separator.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cbruni/Documents/pyfakefs/pyfakefs/tests/fake_pathlib_test.py", line 1380, in test_windows_pure_path_is_absolute
    self.assertEqual(
AssertionError: False != True

======================================================================
FAIL: test_windows_pure_path_str (__main__.FakePathlibModulePurePathTest)
Verify faked pure Windows paths are formatted using windows separators
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cbruni/Documents/pyfakefs/pyfakefs/tests/fake_pathlib_test.py", line 1343, in test_windows_pure_path_str
    self.assertEqual(
AssertionError: 'C:/Windows/cmd.exe' != 'C:\\Windows\\cmd.exe'
- C:/Windows/cmd.exe
?   ^       ^
+ C:\Windows\cmd.exe
?   ^       ^

Your environment

macOS-14.6.1-arm64-arm-64bit
Python 3.10.8 (v3.10.8:aaaf517424, Oct 11 2022, 10:14:40) [Clang 13.0.0 (clang-1300.0.29.30)]
pyfakefs 5.6.0
pytest 7.4.4
@mrbean-bremen
Copy link
Member

Thanks - these are always a bit of a pain... I will see what I can do here.

@camillobruni
Copy link
Contributor Author

I can imagine :) would be nice to have.
I'm trying to get some cross-platform mock tests going locally (with both involve win and posix platforms). Would be awesome to get more testing working.
Until then testing platform-based systems in isolation will work.

@mrbean-bremen
Copy link
Member

Yes - cross-platform testing with pyfakefs will never be perfect, but we can at least try to make it more usable.

@mrbean-bremen
Copy link
Member

I had a closer look, and I think this is not a bug.

As far as I can see, the failing test is FakePathlibModulePurePathTest, which did not initialize the fake filesystem - this won't work. If you make sure that the base test also initializes the fake file system, for example by using:

class FakePathlibModulePurePathTest(fake_filesystem_unittest.TestCase):
    def setUp(self) -> None:
        self.setUpPyfakefs()
    
    ...

it should work.

The problem is that the fake pure posix/windows paths need to know the fake filesystem in order to know which system is currently emulated, and what delimiters to use etc., and this is done in the fake filesystem setup.

@mrbean-bremen mrbean-bremen removed the bug label Aug 26, 2024
@camillobruni
Copy link
Contributor Author

Sorry I've uploaded a half cleaned up version of my hacked repro.
This still repros, the key is the capture that original pathlib classes before setting up the fake FS.

Maybe my assumption that the the PureXXXPaths should always work the same way is wrong?

@mrbean-bremen
Copy link
Member

Thanks! I couldn't reproduce it yesterday with the changed test, will check again your adapted tests tonight...

@mrbean-bremen
Copy link
Member

the key is the capture that original pathlib classes before setting up the fake FS.

Right, this is the problem. This is a shortcoming of pyfakefs (and any mocking) - if you capture a value in a variable before you start patching, there is no way it can be patched automatically afterwards, except by reloading the module during the test (which is of course not possible if the module is the test).

So no, this won't work, and while it is a limitation, I do not consider it a bug. This should not be done in a test, and if done in a module, it can be worked around by adding the module to modules_to_reload.

@camillobruni
Copy link
Contributor Author

camillobruni commented Aug 27, 2024 via email

@mrbean-bremen
Copy link
Member

Sure - as I wrote, emulating another OS is not complete in pyfakefs. There are tests missing, and certainly some bugs hiding.
Generally, we expect to get the same results in the real and faked fs, though there are some edge cases where this won't work. If you find something, I'm happy to have a look and try to fix it.

@mrbean-bremen
Copy link
Member

You can also just make a real PR with the failing test if you have it, this would make the results for different OSes visible and would be easier to discuss.

@camillobruni
Copy link
Contributor Author

camillobruni commented Aug 29, 2024

Created PR #1055

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Aug 29, 2024

Thanks for your investigations and PR - that was helpful!

I just had a look, and there is indeed a problem. PosixPath under Windows and WindowsPath under Posix call splitroot during initialization, which uses os.splitroot in the fake filesystem and thus the delimiters set set globally, instead of the ones in the path class. I'm not sure yet how to fix this properly, but I will probably have a go at the weekend.

@mrbean-bremen
Copy link
Member

Just as an update: this is a bit more complicated than I had hoped, especially in Python >= 3.12, as these versions use the os.path module for the implementation in pathlib. Currently, the filesystem-specific attributes (separators etc) all live in the FakeFilesystem class, which is used in the FakePath class that fakes os.path. I'm thinking of a way to somewhat decouple these attributes, so that different versions can be used in the Posix/WindowsPath implementations more easily.

As I wrote elsewhere, I had introduced emulated filesystems as an afterthought for easier testing of pyfakefs itself, and initiallly hadn't thought about making it an external feature. Especially in pathlib this was always incomplete. Recent PRs have improved the behavior substantially, so it is probably time to put a bit more effort into this to clean it up and fix the remaining problems.

This may take a while, so no promises...

@mrbean-bremen
Copy link
Member

Fixed with #1055 (accidentally put the wrong issue number into the commit message).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants
@camillobruni @mrbean-bremen and others