pytest-xdist icon indicating copy to clipboard operation
pytest-xdist copied to clipboard

Allow other plugins to completely disable pytest-xdist output

Open allisonkarlitskaya opened this issue 2 years ago • 6 comments

When creating the DSession object, pytest-xdist checks to see if the terminalreporter plugin is registered with pytest, and if so, creates a TerminalDistReporter object and registers it as a pytest plugin named terminaldistreporter.

pytest-tap tries to disable all terminal output (since it interferes with the tap output). This works by removing the terminalreporter plugin. If you have xdist and pytest_tap both installed as site packages, this approach works fine — but only because the pytest_tap configure hook happens to run first.

If you try to do the same thing from your own conftest.py, though, you fall over fast. It's difficult to arrange for your configure hook to be called after terminalreporter has been created but before xdist checks for it. No worries — our conftest.py can run last and also remove the terminaldistreporter plugin. Great!

Unfortunately, this doesn't work. In addition to registering the plugin, the DSession takes a reference on it:

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

and uses this trdist later, from worker_collectionfinish:

if self.terminal:
    self.trdist.setstatus(
        node.gateway.spec, WorkerStatus.CollectionDone, tests_collected=len(ids)
    )

and

if self.terminal and not self.sched.has_pending:
    self.trdist.ensure_show_status()
    self.terminal.write_line("")

It would be nice if there was a better way to silence pytest-xdist entirely. The "big hammer", of course, is to redirect sys.stdout entirely, but I'd prefer to avoid that if possible.

So some alternatives:

  • provide a simple mechanism to disable all output without playing around with unregistering plugins
  • defer creation of the TerminalDistReporter until after the config step is done, making it more sensitive to changes in the terminalwriter plugin
  • turn the .setstatus() and .ensure_show_status() APIs into hooks and call them that way. If the plugin gets unloaded then they won't be called — not harm done. That would also allow dropping the extra self.terminal check (although there are some direct uses of those, as well, that would need to be fixed up).
  • provide some official API on pytest-xdist to disable output. For now, I'm getting the dsession plugin and manually setting terminal to None, but this doesn't really feel great...

If any of these approaches are acceptable, I'm happy to write a patch.

Thanks!

allisonkarlitskaya avatar Jul 06 '23 11:07 allisonkarlitskaya

I'm under the impression that we ought to coordinate this with pytest and terminalweiter in some way

Personally I'd like to completely anhillate terminalweiter as is, It's a rather unfit api for UX in many ways

Unfortunately the design and migration work for that size of undertaking far exceeds the bounds of reasonably effort for your outlined usecase

A starting point might be a better tool to disable terminalweiter to begin with and a accompanying patch to xdist for using it on worker's and for respecting it's in normal operation

RonnyPfannschmidt avatar Jul 06 '23 12:07 RonnyPfannschmidt

provide a simple mechanism to disable all output without playing around with unregistering plugins

A starting point might be a better tool to disable terminalweiter to begin with and a accompanying patch to xdist for using it on worker's and for respecting it's in normal operation

Perhaps we can add a new pytest option that completely disables pytest's own output? Then pytest-xdist could honor that option too.

nicoddemus avatar Jul 06 '23 14:07 nicoddemus

For full safety we probably want something that ensures no output is made,which includes enforcement of output capture and terminalweiter silence

RonnyPfannschmidt avatar Jul 06 '23 16:07 RonnyPfannschmidt

PS I believe this should be a api of config, and xdist,sugar,tap as well as bdd output plugins ought to use it in pytest-configure

RonnyPfannschmidt avatar Jul 06 '23 16:07 RonnyPfannschmidt

For full safety we probably want something that ensures no output is made,which includes enforcement of output capture and terminalweiter silence

Not sure, I was thinking we could guarantee pytest itself will not output anything (effectively disabling the terminal writer), not go out of our way to ensure nothing gets written in the terminal... I believe this flag is enough for the OP?

PS I believe this should be a api of config, and xdist,sugar,tap as well as bdd output plugins ought to use it in pytest-configure

What API do you have in mind?

nicoddemus avatar Jul 06 '23 16:07 nicoddemus

That's unclear as of now, naming and granularity needs Brainstorming

RonnyPfannschmidt avatar Jul 06 '23 17:07 RonnyPfannschmidt