Skip to content
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

markers: support new ident '|' #12277

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hfudev
Copy link

@hfudev hfudev commented Apr 30, 2024

Hi pytest team!

While I'm developing a new feature to the pytest-embedded plugin, I realize that | is not allowed in pytest markers. (Inside the pytest-embedded plugin, I'm using | as a separator.)

ERROR: Wrong expression passed to '-m': esp32|esp32p4: at column 6: unexpected character "|"

I'm wondering the reason behind this. so I'm opening this PR.

Thanks!
Hanxi

EDIT

actually use | is better than : (which is supported now)

I didn't find a way to define a marker with : inside, since everything past after the first : will be considered as a optional description.

so pytest -m "a:b" will try to search a marker called a:b, but if I put

[pytest]
  a:b: test

only marker a will be added

if we support |, we could define with

[pytest]
  a|b: test

(tested with --strict-marker)

@hfudev hfudev force-pushed the feat/support_vertical_bar_in_markers branch from 63e9ed1 to 16d317f Compare April 30, 2024 11:15
@hfudev
Copy link
Author

hfudev commented Apr 30, 2024

The test cases are failing, I'll fix them once the idea got approved by you.

@RonnyPfannschmidt
Copy link
Member

It's unclear to me what exactly this solves

Marker names currently are limited to identifiers

The pipe is commonly used as logic operator in python and im under the impression that using those like that can be costly later

@bluetech bluetech added the status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants