Pytest: Exit code 1 when pytest crashes on loading plugin

[genestack-solomatin]

Steps to reproduce

execute pytest -p not-installed-plugin

Observed result

Exit code is 1

  ...
  File "/usr/local/lib/python2.7/site-packages/_pytest/config.py", line 435, in import_plugin
    six.reraise(new_exc_type, new_exc, sys.exc_info()[2])
  File "/usr/local/lib/python2.7/site-packages/_pytest/config.py", line 429, in import_plugin
    __import__(importspec)
   ImportError: Error importing plugin "not-installed-plugin": No module named not-installed-plugin

Expected result

Exit code 2 or 4 (https://docs.pytest.org/en/latest/usage.html#possible-exit-codes)

System

platform darwin -- Python 2.7.13, pytest-3.3.2, py-1.5.2, pluggy-0.6.0

[pytestbot]

GitMate.io thinks the contributor most likely able to help you is @nicoddemus.

[nicoddemus]

Hi @genestack-solomatin thanks for writing.

I agree 1 is incorrect, but 2 and 4 doesn't ring correct either to me:

Exit code 2: Test execution was interrupted by the user
Exit code 4: pytest command line usage error

Perhaps we need a new error code?

[RonnyPfannschmidt]

i propose we either generalize the exit code 3 or we introduce a new exit code that indicates "pytest crashed due to a plugin or internal error"

im leaning slightly torwards a new error code

[feuillemorte]

I can see this code in config.py (433 line):

  except ImportError as e:`
            new_exc_type = ImportError
            new_exc_message = 'Error importing plugin "%s": %s' % (modname, safe_str(e.args[0]))
            new_exc = new_exc_type(new_exc_message)
            six.reraise(new_exc_type, new_exc, sys.exc_info()[2])

So, the solusion maybe replace six.reraise(new_exc_type, new_exc, sys.exc_info()[2]) to this code:

            import traceback
            traceback.print_exception(new_exc_type, new_exc, sys.exc_info()[2])
            sys.exit(EXIT_MODULE_IMPORT_ERROR)

What do you think about it?

[genestack-solomatin]

Should we return special exit status in case of other errors?

https://github.com/pytest-dev/pytest/blob/621374679b0cc9be4bbb76589fd94f20ecb9e0ec/_pytest/config.py#L437

except Exception as e:
            import pytest
            if not hasattr(pytest, 'skip') or not isinstance(e, pytest.skip.Exception):
                  raise

[nicoddemus]

@feuillemorte I don't think that's a good approach: we should not call sys.exit() from within pytest, we should just return the error code from main() instead because people might be using pytest by calling pytest.main(); in that case users expect pytest to return an error code, not raising SystemExit.

Not sure how easy is to change this behavior, because we let the ImportError propagate upwards on purpose so we don't lose the traceback, and Python returns 1 in that case:

$ python -c "raise RuntimeError"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
RuntimeError

$ echo %errorlevel%
1