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

Support for pytest-xdist #6

Open
nicoddemus opened this issue Feb 17, 2022 · 7 comments
Open

Support for pytest-xdist #6

nicoddemus opened this issue Feb 17, 2022 · 7 comments
Assignees

Comments

@nicoddemus
Copy link
Owner

Make sure pytest-rich works with pytest-xdist.

I suspect we can get this support without any special code, given how pytest-rich currently leverages rich's live/progress features.

@AngellusMortis
Copy link

Can confirm, this does not work with xdist. I get an error like the following:

INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/_pytest/main.py", line 268, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/_pytest/main.py", line 322, in _main
INTERNALERROR>     config.hook.pytest_runtestloop(session=session)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_hooks.py", line 265, in __call__
INTERNALERROR>     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/xdist/dsession.py", line 117, in pytest_runtestloop
INTERNALERROR>     self.loop_once()
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/xdist/dsession.py", line 140, in loop_once
INTERNALERROR>     call(**kwargs)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/xdist/dsession.py", line 266, in worker_logstart
INTERNALERROR>     self.config.hook.pytest_runtest_logstart(nodeid=nodeid, location=location)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_hooks.py", line 265, in __call__
INTERNALERROR>     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pytest_rich.py", line 174, in pytest_runtest_logstart
INTERNALERROR>     self._update_task(nodeid)
INTERNALERROR>   File "/usr/local/lib/python3.10/site-packages/pytest_rich.py", line 194, in _update_task
INTERNALERROR>     task = self.runtest_tasks_per_file[fn]
INTERNALERROR> KeyError: PosixPath('/app/tests/test_package.py')

I do really hope someone picks up the package and maintains it as I think this definitely has a lot of potential vs. pytest-sugar.

@nicoddemus
Copy link
Owner Author

Oh bummer. That's because the workers report the file names relative to the rootdir. Should be simple to adjust this however.

I do really hope someone picks up the package and maintains it as I think this definitely has a lot of potential vs. pytest-sugar.

@joshuadavidthomas is already a official maintainer. 🎉

@joshuadavidthomas
Copy link
Collaborator

Yeah, this is definitely on the roadmap.

I've been a bit underwater with my dayjob1, so I haven't had much time lately to devote to OSS projects.

Any PRs to tackle this issue are welcome!

Footnotes

  1. Two projects launching simultaneously in March, why I did this to myself I will never know. 😬

@nicoddemus
Copy link
Owner Author

I've been a bit underwater with my dayjob

No worries, take your time! Thanks for the update. 👍

@joshuadavidthomas
Copy link
Collaborator

I've started to dig in to this. I'll keep this issue updated with what I find, in case I cannot finish it or someone else wants to pick it up.

First thing I did notice is we'll need to rename what the plugin advertises itself as to pytest, as pytest-xdist relies on the terminal reporter being named a certain way:

https://github.com/pytest-dev/pytest-xdist/blob/master/src/xdist/dsession.py#L57-L60

self.terminal = config.pluginmanager.getplugin("terminalreporter")
if self.terminal:
    self.trdist = TerminalDistReporter(config)
    config.pluginmanager.register(self.trdist, "terminaldistreporter")

But when pytest-rich is registered, it's using a not-standard name:

standard_reporter = config.pluginmanager.getplugin("terminalreporter")
config.pluginmanager.unregister(standard_reporter)
config.pluginmanager.register(
RichTerminalReporter(config), "rich-terminal-reporter"
)

Looking at another alternative terminal reporter, pytest-sugar, they just shadow the one that comes standard with pytest:

https://github.com/Teemu/pytest-sugar/blob/efafd9c0d4bfb174db2911beb414bbd4092ffc57/pytest_sugar.py#L196-L201

if IS_SUGAR_ENABLED and not getattr(config, "slaveinput", None):
    # Get the standard terminal reporter plugin and replace it with our
    standard_reporter = config.pluginmanager.getplugin("terminalreporter")
    sugar_reporter = SugarTerminalReporter(standard_reporter)
    config.pluginmanager.unregister(standard_reporter)
    config.pluginmanager.register(sugar_reporter, "terminalreporter")

@joshuadavidthomas
Copy link
Collaborator

@nicoddemus Any tips about what I should be looking at within pytest-xdist? I've only ever been a user of it, never looked at the source. As the lead contributor, I assume you know the internals pretty well 😄 -- anything I need to know or be aware of?

@nicoddemus
Copy link
Owner Author

My suggestion is to do what you are already doing, follow pytest-sugar's lead here. 😁

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

No branches or pull requests

3 participants