AbdealiLoKo / pytest-attrib

A pytest plugin that mimics the working of nose-attrib
MIT License
3 stars 1 forks source link

-a vs -A #7

Closed jayvdb closed 8 years ago

jayvdb commented 8 years ago

When using the ! operator in an attribute, an exception occurs:

INTERNALERROR>   File "/home/travis/virtualenvs/pypy-5.0.1/site-packages/pytest_attrib/plugin.py", line 23, in pytest_collection_modifyitems
INTERNALERROR>     if attrexpr and not match_attr(colitem, attrexpr):
INTERNALERROR>   File "/home/travis/virtualenvs/pypy-5.0.1/site-packages/pytest_attrib/plugin.py", line 34, in match_attr
INTERNALERROR>     return eval(expr, {}, AttrMapping(item))
INTERNALERROR>   File "<string>", line 1
INTERNALERROR>     !net
INTERNALERROR>     ^
INTERNALERROR> SyntaxError: Unknown character

Underlying this is that with nose-attrib, -a uses a specialised language, whereas -A is normal python expressions. http://nose.readthedocs.io/en/latest/plugins/attrib.html#simple-syntax

pytest-attrib is currently doing eval on -a arguments. IMO it makes sense to only implement eval style filters, but then the argument should be -A, not -a.

AbdealiLoKo commented 8 years ago

Interesting point, and I was considering implementing both initially in this library.

I decided to keep the method of selection as similar to how pytest -m and -k handle the selection, because otherwise pytest users would get confused, and hence I decided to only use the eval like expression. I also decided to use -a instead of -A because -k and -m use lowercase. (There are no capital short args in pytest) Essentially I was trying to match the style and usage of pytest as much as possible.

If you like we could alias -A and -a ?

PS: I realize now that this may be confusing for nose/nose-attrib users moving to pytest/pytest-attrib, so maybe mentioning this difference in the Readme as a note would be good to avoid confusion?

On Thu, May 26, 2016 at 8:25 PM, John Vandenberg notifications@github.com wrote:

When using the ! operator in an attribute, an exception occurs:

INTERNALERROR> File "/home/travis/virtualenvs/pypy-5.0.1/site-packages/pytest_attrib/plugin.py", line 23, in pytest_collection_modifyitems INTERNALERROR> if attrexpr and not match_attr(colitem, attrexpr): INTERNALERROR> File "/home/travis/virtualenvs/pypy-5.0.1/site-packages/pytest_attrib/plugin.py", line 34, in match_attr INTERNALERROR> return eval(expr, {}, AttrMapping(item)) INTERNALERROR> File "", line 1 INTERNALERROR> !net INTERNALERROR> ^ INTERNALERROR> SyntaxError: Unknown character

Underlying this is that with nose-attrib, -a uses a specialised language, whereas -A is normal python expressions. http://nose.readthedocs.io/en/latest/plugins/attrib.html#simple-syntax

pytest-attrib is currently doing eval on -a arguments. IMO it makes sense to only implement eval style filters, but then the argument should be -A, not -a.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/AbdealiJK/pytest-attrib/issues/7

jayvdb commented 8 years ago

For context, this is used in the Pywikibot Jenkins nose jobs https://github.com/wikimedia/pywikibot-core/blob/master/tox.ini#L71

I've confirmed that not net works as a substitute.

In nose, they are --attr and --eval-attr , with aliases, and I would be very happy to switch to using --eval-attr in both nose and pytest invocations to be more descriptive, if you could provide them. Then, pywikibot doesnt care what you do with -a and -A.

I dont think aliasing -a and -A is a good solution - sometimes they will do what the nose user expects when the syntax overlap, othertimes they will fail. Maybe remove the shortcuts altogether, and only provide long opts.

If you do anything other than implement both --attr and --eval-attr (and have shortcuts -a and -A), documentation will be needed ;-)

jayvdb commented 8 years ago

Another nice addition will be to detect obvious nose -a style values (like OP of this issue) and tell the user to rewrite the value as an expr. Or, catch the exception and present a more useful wrapper message.