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

Missing mechanism to extend the --dist choices when adding a new scheduler results in pytest execution failure

Open vitaly-krugl opened this issue 2 years ago • 8 comments

Documentation says to use pytest_xdist_make_scheduler to add a new scheduler. However, command line parsing fails because the choices for the --dist command line option are fixed by xdist.plugin.pytest_addoption. Also, help string that documents each of the available options in --dist is not extensible to include the new scheduler choices.

$ pytest --basetemp ~/xdist-debug/basetemp/ -p my_xdist_schedulers.plugin -n 3 --dist mycoolnewscheduler
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: argument --dist: invalid choice: 'mycoolnewscheduler' (choose from 'each', 'load', 'loadscope', 'loadfile', 'loadgroup', 'worksteal', 'no')

vitaly-krugl avatar Nov 14 '23 06:11 vitaly-krugl

Hi @vitaly-krugl,

Indeed this is a problem that we overlooked.

From the top of my head I don't know how we could solve this, TBH.

nicoddemus avatar Nov 14 '23 11:11 nicoddemus

We could consider a entry point

RonnyPfannschmidt avatar Nov 14 '23 12:11 RonnyPfannschmidt

I ended up working around this limitation as follows:

  1. Handle pytest_addoption hook and add a group/option for my own scheduler's --custom-dist to the parser
  2. Handle pytest_cmdline_parse with hookwrapper=True where, after the non-hookwrappers run, I overwrite config.option.dist with the name of my own scheduler's --custom-dist option value, if it was passed on command line. This overwrite of config.option.dist was necessary - if I didn't overwrite it, then xdist's plugin.pytest_configure would find the default "no" in config.option.dist and bypass distributed execution altogether (wouldn't create DSession, etc.)
  3. Handle pytest_xdist_make_scheduler and instantiate my own scheduler if config.option.dist value matches my scheduler.

NOTE: I needed to pass my plugin via both the -p command line arg and also via the plugins' function argument to pytest.main()` to make this work.

  • Without -p, I got an exception when xdist re-parses the args in its remote worker - "error: unrecognized arguments: --custom-dist".

  • Without the plugins' function argument to pytest.main(), my hookwrapper for pytest_cmdline_parse` doesn't get called.

vitaly-krugl avatar Nov 15 '23 05:11 vitaly-krugl

@nicoddemus and @RonnyPfannschmidt, the workaround that I documented in my previous comment isn't necessarily a substitute for a well-documented and supported scheduler plugin mechanism. However, it demonstrates a relatively-clean (I think) working prototype for bootstrapping an add-on xdist scheduler into xdist and into the pytest command-line. It also suggests that an add-on scheduler may have a need to set its own specific options via pytest command line.

vitaly-krugl avatar Nov 16 '23 20:11 vitaly-krugl

Maybe there should be a hook that returns a list of available schedulers (or a name => callable (factory) dictionary)?

amezin avatar Nov 17 '23 04:11 amezin

Maybe there should be a hook that returns a list of available schedulers (or a name => callable (factory) dictionary)?

Yeah that's an idea, however it does not work currently on how options are added currently: pytest_addoption (which declares dist, along with the list of available options like no, load, each, etc) is called during discovery by pluggy (because it is pytest_addoption is marked as historic), and at this point, no other hooks have been "found" yet. This is something we would need to change in pluggy itself to make this work.

nicoddemus avatar Nov 23 '23 12:11 nicoddemus

the workaround that I documented in my previous comment isn't necessarily a substitute for a well-documented and supported scheduler plugin mechanism

Definitely, unfortunately (at least to me) there is no clear way forward yet on how to support this nicely.

nicoddemus avatar Nov 23 '23 12:11 nicoddemus

Hi @nicoddemus, here is a proposal that I think could work:

Presently, when xdist's plugin.pytest_configure would finds the default "no" in config.option.dist, it bypasses distributed execution altogether (wouldn't create DSession, etc.).

  1. The proposed change is to not do the above. Instead, consider config.option.dist to be the name of the built-in scheduler and assume that other scheduler providers may have their own command line options for their own scheduler.
  2. Modify xdist's handler of pytest_xdist_make_scheduler to return None if `config.option.dist == 'no'.
  3. Invoke the hook pytest_xdist_make_scheduler. If the hook doesn't return a scheduler, then - and only then - bypass distributed execution.

vitaly-krugl avatar Dec 10 '23 01:12 vitaly-krugl