feat: support keyword arguments in marker expressions

[lovetheguitar]

Fixes #12281

With this change you can restrict a test run to only run tests matching one or multiple marker keyword arguments Only keyword argument matching is supported in marker expressions. Only int, str, bool & None values are supported in marker expressions.

[webknjaz]

@lovetheguitar could you rebase the PR, fill out the PR description/commits, and compose a change note?

[lovetheguitar]

@RonnyPfannschmidt Could you please have a look? :)

[lovetheguitar]

@webknjaz https://readthedocs.org/projects/pytest/builds/24773128/ does the preview fail to link that or did I reference it wrongly?

Edit:

The solution was to define a "title" when referencing since at the anchor there was no explicit headline.

:ref:`marker examples <marker_keyword_expression_example>`

Thanks for the tip @webknjaz.

[RonnyPfannschmidt]

awesome - @lovetheguitar thanks for seeing this tough and @webknjaz thanks for sorting out the last details to land this

[lovetheguitar]

I think this is amazing work, and it's great how many tests there are and corner cases there are handled!

I'm really unhappy how this was merged in a seemingly unfinished state (TODO comments in various places), while we were eating dinner together, 2 hours after the final version was pushed. I found a couple of pretty obvious things from a cursory review, and now I'll either need to do a follow-up PR to pick up the lose ends, or someone else needs to go back to opening a new PR and all that additional work.

Also, @bluetech wrote the parser in the first place if memory serves me right, so I feel like it'd be appropriate to give him a chance to look at this, no?

@The-Compiler Sure thing, I was also thinking about mentioning @bluetech, since, yes he wrote that part and he commented also in the issue.

Feel free to be nitpicky, I'm motivated to learn the customs properly and do the right thing (TM). 😄

[webknjaz]

Too bad that it got auto-merged earlier, than intended. I knew that Ronny didn't want to make the process too difficult for a first-time contributor so I fixed a few concerns myself and hit Approve to replace the blocking review, assuming that Ronny did check the other things. And I addressed Ronny's comment and dismissed his review, communicating with him offline. This was in line with Ronny's plan and the auto-merge that he'd set earlier (it wasn't auto-reset after my commits). I should've instead dismissed my review so it'd not be a blocker and not hit approve, as there were parts I didn't really inspect deeply.

@lovetheguitar since you stated you wanted to do better, here are some action items that you could perform (with an attempt to explain why they are important): 1) Make a follow-up PR or several of them, addressing the comments that Ran and Florian made (I suppose, this one is rather obvious); 2) When you see examples of things similar to what you're adding, it's usually best to follow the surrounding style — this avoids additional mental overhead for the reviewers since the pre-existing pieces have been battle-tested and any customizations would need to be inspected from various angles and might be missed by some reviewers (I missed it, for example, and didn't even know it was significant); 3) I usually recommend not closing/dismissing the separate inline review threads started by someone else for the following reasons:

• only the thread creator knows if you actually addressed their concerns — you can guess but in general, the only way of really knowing is to let them declare the thing solved. A review is a multi-sided conversation and not someone throwing in instructions to be either followed or dismissed silently — be verbose about what's going on and let others get onto the same page with you;
• clicking the Resolve conversation button silently adds mental overhead for the creator since it doesn't generate any notifications and just becomes collapsed; It then requires someone to click on all the collapsed threads, trying to figure out where that old context exists and attempt to judge whether it was solved and guess why it disappeared as the reason is not always obvious and see the previous point for why it might not actually be resolved. It creates more round-trips, confusion, and frustration. I recommend just re-asking if you feel like it might be resolved or waiting;
• in the previous point, I mentioned that no-notification disappearance of threads. To solve this point, if you're like really sure you can decide that it's resolved by yourself (for example, when you followed what was requested to the letter), I suggest writing a summary/notification comment about this action — I typically explain why/how the thing was resolved and add explicit text like "Marking this thread as resolved." This lets other people actually find it by following their email notifications and registering that they don't need to follow that conversation anymore. I've also seen great pull request authors including commit links there — in case they didn't click on the Commit suggestion button and re-implemented it.

That point (3) is something that happened here — I suggested a few correct fixes and extra explanations in some places. It was possible to click one button on the GH UI and get them in. However, they ended up being dismissed for a mysterious reason that I still have no idea of. There was a comment about choosing to use an alternative (worse) syntax option but the motivation was never stated. So I still had to just add those things manually which prolonged the PR cycle — because of lack of communication. And with the other suggestion, it was dismissed with an incorrect perception, forcing me to dig the thread from the pile of others and unresolve it.

To summarize, the PR was good in general, but a few things were missed during the review and the transparency could be better (not just on the PR author's side, though).

[lovetheguitar]

Very nice work, I wouldn't have done it better myself :)

I left some comments.

Thanks for the fast review @bluetech. I tried to address the open points in #12523. Looking forward to some feedback.