search

pytest-dev / pytest-xdist

pytest plugin for distributed testing and loop-on-failures testing modes.
https://pytest-xdist.readthedocs.io
MIT License
1.46k stars 232 forks source link

Options with nonserializable types break under xdist #384

Open Hawk777 opened 5 years ago

Hawk777 commented 5 years ago

I wanted a command-line option to take a filename as its value. It works fine with vanilla py.test, but not with xdist.

$ cat test_stuff.py 
def test_stuff():
    pass

$ cat conftest.py 
import pathlib
import pytest
@pytest.hookimpl
def pytest_addoption(parser):
    parser.addoption("--my-option", type=pathlib.Path)

$ py.test --my-option=/
============================= test session starts ==============================
platform linux -- Python 3.6.5, pytest-4.0.1, py-1.7.0, pluggy-0.8.0
rootdir: /tmp/test, inifile:
plugins: xdist-1.24.1, forked-0.2
collected 1 item                                                               

test_stuff.py .                                                          [100%]

=========================== 1 passed in 0.01 seconds ===========================

$ py.test --my-option=/ -n2
================================================== test session starts ==================================================
platform linux -- Python 3.6.5, pytest-4.0.1, py-1.7.0, pluggy-0.8.0
rootdir: /tmp/test, inifile:
plugins: xdist-1.24.1, forked-0.2
gw0 C / gw1 IINTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1383, in _save
INTERNALERROR>     dispatch = self._dispatch[tp]
INTERNALERROR> KeyError: <class 'pathlib.PosixPath'>
INTERNALERROR> 
INTERNALERROR> During handling of the above exception, another exception occurred:
INTERNALERROR> 
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/_pytest/main.py", line 183, in wrap_session
INTERNALERROR>     config.hook.pytest_sessionstart(session=session)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/hooks.py", line 284, in __call__
INTERNALERROR>     return self._hookexec(self, self.get_hookimpls(), kwargs)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/manager.py", line 67, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/manager.py", line 61, in <lambda>
INTERNALERROR>     firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/callers.py", line 208, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/callers.py", line 80, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/callers.py", line 187, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/dsession.py", line 81, in pytest_sessionstart
INTERNALERROR>     nodes = self.nodemanager.setup_nodes(putevent=self.queue.put)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/workermanage.py", line 67, in setup_nodes
INTERNALERROR>     nodes.append(self.setup_node(spec, putevent))
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/workermanage.py", line 76, in setup_node
INTERNALERROR>     node.setup()
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/workermanage.py", line 246, in setup
INTERNALERROR>     self.channel.send((self.workerinput, args, option_dict))
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 717, in send
INTERNALERROR>     self.gateway._send(Message.CHANNEL_DATA, self.id, dumps_internal(item))
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1354, in dumps_internal
INTERNALERROR>     return _Serializer().save(obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1372, in save
INTERNALERROR>     self._save(obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1390, in _save
INTERNALERROR>     dispatch(self, obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1475, in save_tuple
INTERNALERROR>     self._save(item)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1390, in _save
INTERNALERROR>     dispatch(self, obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1471, in save_dict
INTERNALERROR>     self._write_setitem(key, value)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1465, in _write_setitem
INTERNALERROR>     self._save(value)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1388, in _save
INTERNALERROR>     raise DumpError("can't serialize %s" % (tp,))
INTERNALERROR> execnet.gateway_base.DumpError: can't serialize <class 'pathlib.PosixPath'>
nicoddemus commented 5 years ago

Indeed, that's a limitation of our backend, we should add this to the documentation.

In addition, we could also register a serializer for a Path object.

Hawk777 commented 5 years ago

I had assumed that the options were passed textually and reparsed in the child. Obviously that would not have had this limitation, since the CLI parameters were strings to start with before argparse got its hands on them.

nicoddemus commented 5 years ago

You are right, but to reparse in the worker, you need specialized code for it that knows about pathlib.Path objects, and currently execnet does not do it, which is unfortunate. 😕

Hawk777 commented 5 years ago

Oh, no, I meant I assumed that the options weren’t passed as parsed objects at all. I originally assumed that sys.argv was handed over from parent to child, and of course sys.argv has nothing but strings in it, so then all the options could just be reparsed in the child. So nobody would need to know about Path objects, because they’d just be strings during marshalling.

nicoddemus commented 5 years ago

Oh I see what you mean, makes sense. It seems self.channel.send((self.workerinput, args, option_dict)) is trying to send the parsed options directly instead of sys.argv, which explains the error.

I believe (without looking at the code) that xdist will first parse and handle/filter out some of the arguments, because workers won't know what to do with some options (-n for example). Not sure how easy is this to fix, it would need more investigation I'm afraid.

Hawk777 commented 5 years ago

Yes, I realize it will need some investigation. I guess there might be cases where, if workers are being run on other machines, then xdist might not be installed on the worker machines so they might not understand -n. Or even if not that, I suppose the provision of -n might make the worker try to spawn more workers… Could be complicated, but I think it might be helpful to do sometime.

RonnyPfannschmidt commented 5 years ago

structurally pytest itself simply does not support required arguments at all we should really warn or error on it instead of failing at "interesting" places

Hawk777 commented 5 years ago

@RonnyPfannschmidt comment on wrong ticket?

RonnyPfannschmidt commented 5 years ago

@Hawk777 whops, my fault, one shouldn't open all the images right after a oversize dinner

graingert commented 5 years ago

@nicoddemus it looks like this is still happening and options_dict is still being sent