Repository: pytest-dev/pytest
Pull request: Support forwardslash characters in identifiers
Closes #8377.
<!--
Thanks for submitting a PR, your contribution is really appreciated!

Here is a quick checklist that should be present in PRs.

- [X] Include documentation when adding new features.
- [X] Include new tests or update existing tests when applicable.
- [X] Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.

If this change fixes an issue, please:

- [ X] Add text like ``closes #XYZW`` to the PR description and/or commits (where ``XYZW`` is the issue number). See the [github docs](https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) for more information.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

- [ X] Create a new changelog file in the `changelog` folder, with a name like `<ISSUE NUMBER>.<TYPE>.rst`. See [changelog/README.rst](https://github.com/pytest-dev/pytest/blob/main/changelog/README.rst) for details.

  Write sentences in the **past or present tense**, examples:

  * *Improved verbose diff output with sequences.*
  * *Terminal summary statistics now use multiple colors.*

  Also make sure to end the sentence with a `.`.

- [ X] Add yourself to `AUTHORS` in alphabetical order.
-->


Problem statement:
test ids with `/`s cannot be selected with `-k`
By default pytest 6.2.2 parametrize does user arguments to generate IDs, but some of these ids cannot be used with `-k` option because you endup with errors like  `unexpected character "/"` when trying to do so.

The solution for this bug is to assure that auto-generated IDs are sanitized so they can be used with -k option.

Example:
```
@pytest.mark.parametrize(
    ('path', 'kind'),
    (
        ("foo/playbook.yml", "playbook"),
    ),
)
def test_auto_detect(path: str, kind: FileType) -> None:
   ...
```

As you can see the first parameter includes a slash, and for good reasons. It is far from practical to have to add custom "ids" for all of these, as you can have LOTS of them.

There is another annoyance related to the -k selecting for parameterized tests, is the fact that square braces `[]` have special meanings for some shells and in order to use it you must remember to quote the strings. It would be much easier if the display and selecting of parametrized tests would use only shell-safe format, so we can easily copy/paste a failed test in run it. For example I think that using colon would be safe and arguably even easier to read: `test_name:param1:param2`.

Hints from issue discussion:
The test ids are not invalid, keyword expressions are simply not able to express slashes

It's not clear to me if that should be added 
I am not sure either, but I wanted to underline the issue, hoping that we can find a way to improve the UX. The idea is what what we display should also be easily used to run a test or to find the test within the source code.

I was contemplating the idea of using indexes instead of test id.
Actual test ids can be passed as such, no need for keyword expressions 
updated the title to more accurately reflect that the ids aren't invalid, they just can't be selected using `-k`
Let's allow `/` in `-k` expressions. I don't see it being useful as an operator, and if it's allowed in nodeids, then it should be allowed in `-k`.

Here 25c65616f4455ac78be1027c2fb4debef827c4e6 is a commit which allowed `\` - adding `/` would be very similar. PRs welcome!
> There is another annoyance related to the -k selecting for parameterized tests, is the fact that square braces [] have special meanings for some shells and in order to use it you must remember to quote the strings. It would be much easier if the display and selecting of parametrized tests would use only shell-safe format, so we can easily copy/paste a failed test in run it. For example I think that using colon would be safe and arguably even easier to read: test_name:param1:param2.

Agree it's annoying, but that would be a big change and if you want to propose this seriously, I suggest opening a separate issue/discussion.

Commit message: Merge branch 'pytest-dev:main' into main

Patch to review:
diff --git a/changelog/8377.bugfix.rst b/changelog/8377.bugfix.rst
new file mode 100644
index 00000000000..eab4135e93f
--- /dev/null
+++ b/changelog/8377.bugfix.rst
@@ -0,0 +1,2 @@
+The test selection options ``pytest -k`` and ``pytest -m`` now support matching
+names containing forwardslash (``\/``)characters.
diff --git a/src/_pytest/mark/expression.py b/src/_pytest/mark/expression.py
index 836e1ebd46f..5fd5eb2fb25 100644
--- a/src/_pytest/mark/expression.py
+++ b/src/_pytest/mark/expression.py
@@ -6,7 +6,7 @@
 expr:       and_expr ('or' and_expr)*
 and_expr:   not_expr ('and' not_expr)*
 not_expr:   'not' not_expr | '(' expr ')' | ident
-ident:      (\w|:|\+|-|\.|\[|\]|\\)+
+ident:      (\w|:|\+|-|\.|\[|\]|\\|\/)+
 
 The semantics are:
 
@@ -88,7 +88,7 @@ def lex(self, input: str) -> Iterator[Token]:
                 yield Token(TokenType.RPAREN, ")", pos)
                 pos += 1
             else:
-                match = re.match(r"(:?\w|:|\+|-|\.|\[|\]|\\)+", input[pos:])
+                match = re.match(r"(:?\w|:|\+|-|\.|\[|\]|\\|\/)+", input[pos:])
                 if match:
                     value = match.group(0)
                     if value == "or":
diff --git a/testing/test_mark.py b/testing/test_mark.py
index ce65e7d56ca..5f3af52f459 100644
--- a/testing/test_mark.py
+++ b/testing/test_mark.py
@@ -1111,7 +1111,7 @@ def test_pytest_param_id_allows_none_or_string(s) -> None:
     assert pytest.param(id=s)
 
 
-@pytest.mark.parametrize("expr", ("NOT internal_err", "NOT (internal_err)", "bogus/"))
+@pytest.mark.parametrize("expr", ("NOT internal_err", "NOT (internal_err)"))
 def test_marker_expr_eval_failure_handling(pytester: Pytester, expr) -> None:
     foo = pytester.makepyfile(
         """
diff --git a/testing/test_mark_expression.py b/testing/test_mark_expression.py
index c758ee66761..be278535c93 100644
--- a/testing/test_mark_expression.py
+++ b/testing/test_mark_expression.py
@@ -170,7 +170,6 @@ def test_valid_idents(ident: str) -> None:
 @pytest.mark.parametrize(
     "ident",
     (
-        "/",
         "^",
         "*",
         "=",


Code review comments:
diff_hunk: @@ -0,0 +1,2 @@
+The test selection options ``pytest -k`` and ``pytest -m`` now support matching
+names containing forwardslash (``\/``)characters.
path: changelog/8377.bugfix.rst
text: @user1:
The / character doesn't need escaping so can write

```suggestion
names containing forward slash (``/``) characters.
```

diff_hunk: @@ -1111,7 +1111,7 @@ def test_pytest_param_id_allows_none_or_string(s) -> None:
     assert pytest.param(id=s)
 
 
-@pytest.mark.parametrize("expr", ("NOT internal_err", "NOT (internal_err)", "bogus/"))
+@pytest.mark.parametrize("expr", ("NOT internal_err", "NOT (internal_err)"))
path: testing/test_mark.py
text: @user2:
```suggestion
@user1.mark.parametrize("expr", ("NOT internal_err", "NOT (internal_err)", "bogus="))
```

diff_hunk: @@ -6,7 +6,7 @@
 expr:       and_expr ('or' and_expr)*
 and_expr:   not_expr ('and' not_expr)*
 not_expr:   'not' not_expr | '(' expr ')' | ident
-ident:      (\w|:|\+|-|\.|\[|\]|\\)+
+ident:      (\w|:|\+|-|\.|\[|\]|\\|\/)+
path: src/_pytest/mark/expression.py
text: @user1:
```suggestion
ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+
```

diff_hunk: @@ -88,7 +88,7 @@ def lex(self, input: str) -> Iterator[Token]:
                 yield Token(TokenType.RPAREN, ")", pos)
                 pos += 1
             else:
-                match = re.match(r"(:?\w|:|\+|-|\.|\[|\]|\\)+", input[pos:])
+                match = re.match(r"(:?\w|:|\+|-|\.|\[|\]|\\|\/)+", input[pos:])
path: src/_pytest/mark/expression.py
text: @user1:
```suggestion
                match = re.match(r"(:?\w|:|\+|-|\.|\[|\]|\\|/)+", input[pos:])
```