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

Exit exception #3144

Open
feuillemorte opened this issue Jan 23, 2018 · 4 comments
Open

Exit exception #3144

feuillemorte opened this issue Jan 23, 2018 · 4 comments
Labels
plugin: xdist related to the xdist external plugin type: enhancement new feature or API change, should be merged into features branch

Comments

@feuillemorte
Copy link
Contributor

We need to except Exit before this part of code:

pytest/_pytest/main.py

Lines 110 to 116 in 0d96a5b

except KeyboardInterrupt:
excinfo = _pytest._code.ExceptionInfo()
if initstate < 2 and isinstance(excinfo.value, exit.Exception):
sys.stderr.write('{0}: {1}\n'.format(
excinfo.typename, excinfo.value.msg))
config.hook.pytest_keyboard_interrupt(excinfo=excinfo)
session.exitstatus = EXIT_INTERRUPTED

and pass a message to xdist module

it needed for xdist:
original task:
pytest-dev/pytest-xdist#86

I'm ready to create pull request, but I don't know how to pass a message to xdist module, Please, give me an advice. Thanks!

@pytestbot pytestbot added the plugin: xdist related to the xdist external plugin label Jan 23, 2018
@pytestbot pytestbot added the type: bug problem that needs to be addressed label Jan 23, 2018
@nicoddemus
Copy link
Member

Hi @feuillemorte thanks for looking into this.

This is somewhat more complicated than I expected because we have some moving parts to deal with.

We need to make _pytest.main.py catch _pytest.outcomes.Exit explicitly and in that case return a new error code: EXIT_PYTEST_EXIT=6, meaning that an explicit call to pytest.exit(msg) has been made.

Together with the new exit code, I suggest we add a new reason argument to pytest_sessionfinish stating how the session ended. In case of a pytest.exit exception, we would pass its msg argument as reason to pytest_sessionfinish.

pytest-xdist will then need to understand the new exit status 6 and exit gracefully, using the new reason argument in its implementation of pytest_sessionfinish in pytest-xdist/xdist/remote.py.

This is my take on this problem. @RonnyPfannschmidt what do you think?

@feuillemorte I suggest to keep this on hold until we nail down the actual details.

@blueyed
Copy link
Contributor

blueyed commented Nov 21, 2019

The code is different in this regard already:

pytest/src/_pytest/main.py

Lines 202 to 213 in 8bf343d

except (KeyboardInterrupt, exit.Exception):
excinfo = _pytest._code.ExceptionInfo.from_current()
exitstatus = ExitCode.INTERRUPTED
if isinstance(excinfo.value, exit.Exception):
if excinfo.value.returncode is not None:
exitstatus = excinfo.value.returncode
if initstate < 2:
sys.stderr.write(
"{}: {}\n".format(excinfo.typename, excinfo.value.msg)
)
config.hook.pytest_keyboard_interrupt(excinfo=excinfo)
session.exitstatus = exitstatus

(via fc4aa27)

So this might be improved/changed? (came here via #6257)

@vkomarov-r7
Copy link

Any more progress on this one? Just hit it in a separate instance. We have an autouse=True, scope='session' fixture that sets some stuff up before any test runs (in our case, it runs database migrations before our e2e tests). If the setup of the fixture fails, no tests will pass, so we want to bomb early. I just tried changing the exit from sys.exit() to pytest.exit(), and received a similar message to the above indicating a worker process crash.

In the meantime, is there a good workaround to the above? Specifically, is there a good way to gracefully fail the entire test suite when using xdist?

@nicoddemus
Copy link
Member

Hi @vkomarov-r7,

Any more progress on this one?

Unfortunately it doesn't seem nobody got the time to sit down and work on this one yet.

In the meantime, is there a good workaround to the above? Specifically, is there a good way to gracefully fail the entire test suite when using xdist?

I did spend some time now thinking about a possible workaround, but unfortunately couldn't think of any. 😕

I think the outline in #3144 (comment) is what would solve this correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: xdist related to the xdist external plugin type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

5 participants