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

pyfakefs is not suspended in reporting hooks #904

Closed
RonnyPfannschmidt opened this issue Oct 31, 2023 · 6 comments · Fixed by #907 or #918
Closed

pyfakefs is not suspended in reporting hooks #904

RonnyPfannschmidt opened this issue Oct 31, 2023 · 6 comments · Fixed by #907 or #918
Labels

Comments

@RonnyPfannschmidt
Copy link
Member

Describe the bug
as per pytest-dev/pytest-html#741

pyfakefs is still active in the report hooks, when plugins may want to write out results

as far as i can tell, https://github.com/pytest-dev/pyfakefs/blob/main/pyfakefs/pytest_plugin.py is mussing safe distinction of the different hooking points and thus patchers may create quite a mess

@mrbean-bremen
Copy link
Member

Thanks, that seems like a major problem - obviously we are doing something wrong here.

pytest_plugin.py is mussing safe distinction of the different hooking points

Can you elaborate on these hooking points? What would be the correct way to fix this? Obviously I want the patching be reversed before pytest starts reporting...

@RonnyPfannschmidt
Copy link
Member Author

theres multiple levels for that

first is the differently scoped fixtures

second is that as part of the runtestprotocol hooks, both logging and other stuff happens

basically one would want the patches to be active only in the call hooks, and not the log hooks

the import part is in https://github.com/pytest-dev/pytest/blob/8fb7e8b31efaa55e760c142e26eb82b42081ca28/src/_pytest/runner.py#L218

the makereport hook may not need patching disabled, but the logreport hook does,
https://github.com/pytest-dev/pytest/blob/8fb7e8b31efaa55e760c142e26eb82b42081ca28/src/_pytest/hookspec.py#L586 specifies the hook

the recommended course would be a tryfirst hookwrapper that disables fakef while the log hook is active

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 31, 2023

Thanks, I will look into this! The current way to just exclude some pytest modules from patching is obviously too unspecific and doesn't always work as intented.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Nov 12, 2023
- pause the patching in pytest_runtest_logreport
- fixes pytest-dev#904
@mrbean-bremen
Copy link
Member

@RonnyPfannschmidt - thanks, that worked exactly as you wrote, I'll probably merge this tomorrow if nobody objects. Sorry about the long delay - I wanted to finish a minor issue in another project first, but that took unexpectedly long.

One question though: I used pytest_runtest_logreport with tryfirst. If anyone else uses the same hook also with tryfirst and wants to access the filesystem in that hook, that might or might not work, depending on the order the hooks are called (if I understand that correctly). Is this something I just have to live with (as it is unlikely to happen), or is there a canocical way to handle this?

@RonnyPfannschmidt
Copy link
Member Author

Currently there's no explicit order, I recommend a tryfirst hook wrapper as starting point plus logged a issue that's dependents on The pluggy issues about more explicit ordering

@mrbean-bremen
Copy link
Member

Ok, thanks!

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