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

Unclear how to support subclasses of pathlib.Path #1086

Open
ofek opened this issue Nov 3, 2024 · 17 comments
Open

Unclear how to support subclasses of pathlib.Path #1086

ofek opened this issue Nov 3, 2024 · 17 comments

Comments

@ofek
Copy link

ofek commented Nov 3, 2024

They don't work out-of-the-box and I'm uncertain how to get them to work at all without using lazy imports everywhere or defining the class itself within a function.

@mrbean-bremen
Copy link
Member

Can you elaborate a bit what you are trying to do and what you are expecting?

I guess that this is just not supported out of the box, given that pyfakefs relies on module and function names, but maybe we can add something to support some scenarios.

@ofek
Copy link
Author

ofek commented Nov 3, 2024

It appears to be broken only sometimes, in the following example you can see that the touch method works properly but write_text causes infinite recursion:

import pathlib
import sys
from functools import cached_property

if sys.platform == 'win32':
    _PathBase = pathlib.WindowsPath
else:
    _PathBase = pathlib.PosixPath

class Path(_PathBase):
    @cached_property
    def long_id(self):
        from base64 import urlsafe_b64encode
        from hashlib import sha256

        path = str(self)
        if sys.platform == 'win32' or sys.platform == 'darwin':
            path = path.casefold()

        digest = sha256(path.encode('utf-8')).digest()
        return urlsafe_b64encode(digest).decode('utf-8')

def create_with_touch(path):
    path.parent.mkdir(parents=True, exist_ok=True)
    path.touch()

def create_with_write_text(path):
    path.parent.mkdir(parents=True, exist_ok=True)
    path.write_text('', encoding='utf-8')
import pytest

from p import Path, create_with_touch, create_with_write_text

@pytest.mark.usefixtures("fs")
def test_create_with_touch():
    path = Path.home() / 'Desktop' / 'test_touch.txt'
    create_with_touch(path)
    assert path.exists()
    assert path.is_file()

@pytest.mark.usefixtures("fs")
def test_create_with_write_text():
    path = Path.home() / 'Desktop' / 'test_write_text.txt'
    create_with_write_text(path)
    assert path.exists()
    assert path.is_file()

@mrbean-bremen
Copy link
Member

Which os and Python version are you using? With Python 3.13 under Windows I could not reproduce this.

@ofek
Copy link
Author

ofek commented Nov 3, 2024

Windows 11

❯ python --version; pip --version; pytest --version
Python 3.12.6
pip 24.2 from C:\Users\ofek\AppData\Local\Programs\Python\Python312\Lib\site-packages\pip (python 3.12)
pytest 8.3.3

@mrbean-bremen
Copy link
Member

Hm, that's odd. Under Windows, with Python 3.8 both tests fail, with 3.9 the second (but with no recursion), with 3.10 the first and with 3.11-3.13 none fails.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Nov 3, 2024

I cannot reproduce this locally with Windows 10, Python 3.12.7, pytest 8.3.3 and pyfakefs 3.7.1 (though as I wrote, I got failures with Python < 3.11). There is obviously a problem with the older Python versions, but I have no idea why it behaves differently for 3.12. You are using the latest pyfakefs version, right?

@ofek
Copy link
Author

ofek commented Nov 3, 2024

Yes I am using the latest version. Interesting! Indeed I cannot reproduce when I bump from Python 3.12.6 to 3.12.7. This doesn't exactly fix my situation however because we must support Python 3.9+ for now.

@ofek
Copy link
Author

ofek commented Nov 3, 2024

cc @barneygale the pathlib expert for any insight/hunch you may have about this 😄

@mrbean-bremen
Copy link
Member

Ok, so the problem appears with 3.12.6, but not with 3.12.7? That's strange... anyway, I will try to understand the problem at least with the older Python versions, though this may take a while.

@barneygale
Copy link

barneygale commented Nov 3, 2024

I think this is the only pathlib change that landed in that timeframe: python/cpython#125156

That code runs if you pass a PurePath object as an initialiser argument to PurePath. If the parsers don't match, we now call as_posix() to stringify the path (rather than extending with its _raw_paths)

@mrbean-bremen
Copy link
Member

@barneygale - thanks, indeed the check

if arg.parser is ntpath and self.parser is posixpath:

also might have been the problem - it will not work with the fake filesystem. I had to overwrite a similar check in another place, but this is all a bit fragile.

@barneygale
Copy link

Let me know if there's an alternative formulation that would work better for you!

@ofek
Copy link
Author

ofek commented Nov 3, 2024

Please forgive me, I was actually totally incorrect regarding my test of a newer Python. I installed the Python to a particular path so was invoking path\to\python.exe -m pytest. It turns out that also works on 3.12.6 and on both versions running the pytest script directly from an activated virtual environment fails. Here is a more complete picture of tests I just ran:

  • 3.8.20
    • touch
      • python -m pytest
      • pytest
    • write_text
      • python -m pytest
      • pytest
  • 3.9.20
    • touch
      • python -m pytest
      • pytest
    • write_text
      • python -m pytest
      • pytest
  • 3.10.15
    • touch
      • python -m pytest
      • pytest
    • write_text
      • python -m pytest
      • pytest
  • 3.11.10
    • touch
      • python -m pytest
      • pytest
    • write_text
      • python -m pytest
      • pytest
  • 3.12.7
    • touch
      • python -m pytest
      • pytest
    • write_text
      • python -m pytest
      • pytest
  • 3.13.0
    • touch
      • python -m pytest
      • pytest
    • write_text
      • python -m pytest
      • pytest

@ofek
Copy link
Author

ofek commented Nov 4, 2024

Something odd and temporary happened in Python 3.10 with respect to this library. Since 3.11 everything works but only if you invoke the Python executable (which we couldn't do as it's difficult to change internal tooling for everyone).

@mrbean-bremen
Copy link
Member

Yeah, that is very odd. I will see if I can understand this, but probably not before the next weekend (labouring with some dental issue at the moment that makes thinking straight a bit hard...)

@ofek
Copy link
Author

ofek commented Nov 4, 2024

Thanks, I hope you feel better!

@mrbean-bremen
Copy link
Member

Ok, I had another look at this, and now (with a clearer mind) I see that this is related to the problem of globally defined variables.
The problem is that your own derived version of path is not patched, because _pathBase is set before patching happens, and will not be patched afterwards except if the module is reloaded. Your Path class will derive from that version, not from the patched one. The behavior also depends on the implementation of pathlib in the the different versions, though I can't completely explain every result it at the moment.

If you would rewrite your tests to only import the tested class inside the test, it should work, e.g.:

import pytest

@pytest.mark.usefixtures("fs")
def test_create_with_touch():
    from p import Path, create_with_touch, create_with_write_text

    path = Path.home() / 'Desktop' / 'test_touch.txt'
    create_with_touch(path)
    assert path.exists()
    assert path.is_file()

@pytest.mark.usefixtures("fs")
def test_create_with_write_text():
    from p import Path, create_with_touch, create_with_write_text

    path = Path.home() / 'Desktop' / 'test_write_text.txt'
    create_with_write_text(path)
    assert path.exists()
    assert path.is_file()

This is not nice, of course, and quite fragile.

There is also the possibility to reload the module, but that is also not very convenient, because you cannot use names imported from the (unpatched module), as they would remain in the old state, e.g. something like:

import pytest

import p

@pytest.fixture
def my_fs():
    with Patcher(modules_to_reload=[p]):
        yield

@pytest.mark.usefixtures("my_fs")
def test_create_with_touch():
    path = p.Path.home() / 'Desktop' / 'test_touch.txt'
    p.create_with_touch(path)
    assert path.exists()
    assert path.is_file()

@pytest.mark.usefixtures("my_fs")
def test_create_with_write_text():
    path = p.Path.home() / 'Desktop' / 'test_write_text.txt'
    p.create_with_write_text(path)
    assert path.exists()
    assert path.is_file()

I don't think that I will have a generic solution for this problem, as it is an inherent problem of patching in combination with global variables.

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

No branches or pull requests

3 participants