Pytest: Not enforcing reason= with skipif/xfail markers

[The-Compiler]

Currently, when e.g. @pytest.mark.skipif(sys.platform != 'win32') is used rather than @pytest.mark.skipif("sys.platform != 'win32'") (condition as a string), pytest enforces that a reason="..." is given.

As far as I can see, the only place that reason is used is with -rs /-rx which isn't used by default anyways. In other places where pytest can show additional information (like docstrings in --fixtures or descriptions in --markers), we show some default value instead of enforcing the user to specify something.

I was wondering: Should pytest do the same for reason= as well? Why not make it optional, and just report the location (without any description) in the skipped test summary for those cases?

[bluetech]

If we remove the requirement for reason, the effect I foresee is that most new users will omit it, since it is not discoverable and laziness wins anyway. So I see an argument for and against:

• For keeping: users wouldn't do it otherwise (mostly because they wouldn't know about it), it is not much work to add and is good for them, requiring makes the world better.
• Against keeping: "educating" users, forcing work they may intentionally don't want.

Personally I am +0.5 for keeping the requirement -0.5 on dropping the requirement.

---

BTW, from git spelunking I think the reasoning was:

1. Having a reason to display in -rs is required.
2. So let's use strings for the condition and eval them, so we can use the string as a condition.
3. The string form causes problems with importing the mark across modules (?), so let's add a boolean/expression form as well.
4. But then we don't have a string to use as the reason, so let's add a reason parameter and require it for non-string.

If there was a way to stringify the expression, that would solve the problem. Well this is Python so there probably is, but I'm not sure.

[The-Compiler]

If there was a way to stringify the expression, that would solve the problem. Well this is Python so there probably is, but I'm not sure.

Probably possible but not pretty. Reminds me of things Hypothesis does. Money quote:

This file can approximately be considered the collection of hypothesis going to really unreasonable lengths to produce pretty output.

and

This is not a good function and I am sorry for it. Forgive me my sins, oh lord

[bluetech]

@CarycaKatarzyna has a PR for this in #7203. I'd be happy to review it if we there is consensus on dropping the requirement for reason in skipif non-string markers. As said in my comment above I'm -0.5 on this, and I guess @The-Compiler and @CarycaKatarzyna are +1 on this. Probably needs one more vote to be sure :)

[RonnyPfannschmidt]

Reasons are intentionality provided for anything where we don't know the expressions, IMHO that should stay like that or produce a warning at least

[nicoddemus]

While I see the points made by @The-Compiler and think they are reasonable, I think reason as a requirement should stay: more than once a colleague of mine wrote a skipif without a reason, then came to me to ask if that was intentional. After I answered it was, they went back and added useful information (such as related ticket number).

So 👎 from me from stop enforcing reason.

[The-Compiler]

@bluetech You probably mean #7204 :wink:

No strong feelings here either way FWIW, this is just something which came up in a pytest training I gave.

[nicoddemus]

Should we close this and the related PR then?

[bluetech]

Closing per discussion.