Skip to content

ExitCode: support custom codes #6022

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

Closed
wants to merge 1 commit into from
Closed

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 21, 2019

Ref: #5420

This was referenced Oct 21, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Oct 21, 2019

Fails on py35:

Traceback (most recent call last):
  File "d:\a\1\s\.tox\py35-xdist\lib\site-packages\pytest.py", line 17, in <module>
    from _pytest.main import ExitCode
  File "d:\a\1\s\.tox\py35-xdist\lib\site-packages\_pytest\main.py", line 21, in <module>
    class ExitCode(enum.IntEnum):
  File "d:\a\1\s\.tox\py35-xdist\lib\site-packages\_pytest\main.py", line 43, in ExitCode
    @classmethod
  File "C:\hostedtoolcache\windows\Python\3.5.4\x64\Lib\enum.py", line 61, in __setitem__
    raise ValueError('_names_ are reserved for future Enum use')
ValueError: _names_ are reserved for future Enum use

@blueyed
Copy link
Contributor Author

blueyed commented Oct 22, 2019

Do we want to monkeypatch py35, or only have it for py36+ then?
It affects plugins using non-standard exit codes for example.

@RonnyPfannschmidt
Copy link
Member

The refed pr intentionally locked down exit codes

@blueyed
Copy link
Contributor Author

blueyed commented Oct 22, 2019

Update the doc then?
https://github.com/pytest-dev/pytest/pull/6022/files#diff-7191acb1a6aaab2f77b6e53ce837e348L27

But I think we should not prevent custom exit codes, should we?

@RonnyPfannschmidt
Copy link
Member

What actual value do they add that justifies enabling people to break the pytest cli api

@blueyed
Copy link
Contributor Author

blueyed commented Oct 22, 2019

I rather think that we should not break plugins doing it already (for whatever reason).

btw: came to it via #4901.

@bluetech
Copy link
Member

Didn't know _missing_ existed - interesting.

If you decide that ExitCode should be an open enum, then the question is how exit code variables/arguments are intended to be type-annotated.

If annotated as ExitCode, then this is a good approach. Users will need to make sure to wrap custom exit codes with ExitCode(123).

If annotated as int, then this will confuse things somewhat. ExitCode is already an IntEnum, so it will be better to keep it just for well-known codes.

@RonnyPfannschmidt wrote in #5420 that "User defined exit codes are still valid, but should be used with caution.", so it sounds like it should be an int for now, no?

@nicoddemus
Copy link
Member

nicoddemus commented Oct 22, 2019

@RonnyPfannschmidt wrote in #5420 that "User defined exit codes are still valid, but should be used with caution.", so it sounds like it should be an int for now, no?

I agree.

About #4901, we just need to update the representation to use str(retcode):

>>> from enum import IntEnum
>>> class X(IntEnum):
...     A = 1
...     B = 2
...
>>> str(X.A)
'X.A'

?

EDIT: of course that doesn't work because retcode is obtained from the system.

@nicoddemus
Copy link
Member

Submitted a different proposal at #6043

@blueyed
Copy link
Contributor Author

blueyed commented Oct 22, 2019

Cool, closing then.

@blueyed blueyed closed this Oct 22, 2019
@blueyed blueyed deleted the exitcode branch October 22, 2019 23:16
@blueyed
Copy link
Contributor Author

blueyed commented Oct 23, 2019

It's a bit bad though having to use it in try … except ValueError though, e.g. with pytester: https://github.com/pytest-dev/pytest/pull/6025/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants