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

Fix reporting exit due to (real) signal #1401

Merged
merged 1 commit into from Nov 11, 2019

Conversation

@blueyed
Copy link
Contributor

blueyed commented Aug 20, 2019

subprocess reports a negative exit code for signals (the assumption in
#760 (comment) was
wrong).

TODO:

  • faster test (takes >3s)
@todo

This comment has been minimized.

Copy link

todo bot commented Aug 20, 2019

can this be minified?

# TODO: can this be minified?
"tox.ini": """\
[tox]
skipsdist = true
skips_install = true


This comment was generated by todo based on a TODO comment in 82e3a8b in #1401. cc @blueyed.
@blueyed blueyed mentioned this pull request Aug 20, 2019
6 of 6 tasks complete
Copy link
Member

asottile left a comment

src/tox/exception.py Outdated Show resolved Hide resolved
tests/unit/test_exit_signal.py Outdated Show resolved Hide resolved
tests/unit/test_exit_signal.py Outdated Show resolved Hide resolved
@gaborbernat

This comment has been minimized.

Copy link
Member

gaborbernat commented Aug 20, 2019

Definitely use initproj and cmd instead of this import. Let's not make the project a lot slower for just this test.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Aug 20, 2019

initproj/cmd

ok.
I think I've mainly skipped it since initially I wanted to kill the process from outside - but it can kill itself.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Aug 20, 2019

It fails to install tox therein when not using minversion?! (using pytest directly)

ERROR: Could not find a version that satisfies the requirement tox>=3.13.3.dev18+g3725c93.d20190820

Added test_exit_code_signal_2 - it is as slow, and fails due to installing 3.7.0, not using the dev version apparently?

@blueyed blueyed force-pushed the blueyed:fix-exit-signal branch from 8dc427e to 041f867 Aug 20, 2019
`subprocess` reports a negative exit code for signals.

Still handle "128 + X" for processes indicating that they received a
signal themselves.
@blueyed blueyed force-pushed the blueyed:fix-exit-signal branch from 041f867 to 3cf4adc Aug 20, 2019
@blueyed blueyed changed the title [RFC] Fix reporting exit due to signal Fix reporting exit due to (real) signal Aug 20, 2019
@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Nov 9, 2019

Ping.

@gaborbernat gaborbernat merged commit 8e6f64a into tox-dev:master Nov 11, 2019
1 of 2 checks passed
1 of 2 checks passed
tox ci #tox ci_20190820.11 failed
Details
Timeline protection Timeline protection: Good to go
Details
@blueyed blueyed deleted the blueyed:fix-exit-signal branch Nov 11, 2019
@gaborbernat

This comment has been minimized.

Copy link
Member

gaborbernat commented Nov 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.