-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
should_never_reach_here for particular test case when parametrized test has unmatched brackets or parenthesis #17676
Comments
Same issue here, just started happening for me with an existing project using this parametrize decorator.
Have recreated in a new folder, using a much simpler test case
Like I said test discovery was working last week, and I haven't updated pytest since then. Looking at the code in
EDIT: |
This unfortunately caused a lot of breakage in projects that have an 'unbalanced' Instead, shouldn't the code not just use the provided Node and Function interfaces to 'parse' the name? I know that currently E.g., given: $ pwd
/tmp/mvce
$ cat 'tests/folder-[2]/test_foo.py'
import pytest
@pytest.mark.parametrize("bar", [("]",)])
def test_foo(bar):
pass
$ python -m pdb ~/.vscode/extensions/ms-python.python-2021.10.1317843341/pythonFiles/testing_tools/run_adapter.py discover pytest --no-hide-stdio
# ... PDB session started ... then when I inspect (Pdb) item.nodeid
'tests/folder-[2]/test_foo.py::test_foo[]]'
(Pdb) item.name # Node API
'test_foo[]]'
(Pdb) item.originalname # Function API
'test_foo'
(Pdb) item.fspath
local('/private/tmp/mcve/tests/folder-[2]/test_foo.py')
(Pdb) item.fspath.parts()
[local('/'), local('/private'), local('/private/tmp'), local('/private/tmp/mcve'), local('/private/tmp/mcve/tests'), local('/private/tmp/mcve/tests/folder-[2]'), local('/private/tmp/mcve/tests/folder-[2]/test_foo.py')] I also note that there is also a helpful (Pdb) from _pytest.nodes import iterparentnodeids
(Pdb) print(iterparentnodeids.__doc__)
Return the parent node IDs of a given node ID, inclusive.
For the node ID
"testing/code/test_excinfo.py::TestFormattedExcinfo::test_repr_source"
the result would be
""
"testing"
"testing/code"
"testing/code/test_excinfo.py"
"testing/code/test_excinfo.py::TestFormattedExcinfo"
"testing/code/test_excinfo.py::TestFormattedExcinfo::test_repr_source"
Note that :: parts are only considered at the last / component.
(Pdb) pp list(iterparentnodeids(item.nodeid))
['',
'tests',
'tests/folder-[2]',
'tests/folder-[2]/test_foo.py',
'tests/folder-[2]/test_foo.py::test_foo[]]'] If the expected output for
then using
Then, perhaps use |
I fixed this locally by replacing lines 161-163 in testing_tools/adapter/pytest/_pytest_item.py, with: if kind == "function" and item.nodeid[-1:] == "]":
# work around microsoft/vscode-python#17676
parameterized = item.name[len(item.originalname) :]
(parentid, parents, fileid, testfunc, _) = _parse_node_id(
item.nodeid[: -len(parameterized)], kind
)
nodeid = f"{parentid}{parameterized}"
parents = [(parentid, item.originalname, kind), *parents]
else:
(nodeid, parents, fileid, testfunc, parameterized) = _parse_node_id(
item.nodeid, kind
) On your local system, that's So, if the current item is a function and has been parametrized (sic, pytest's misspelling), then it parses the item without the parameters, to then add back that information based on the parsed result. This avoids having to fully rework the way the rest of the nodeid elements are parsed and normalized. |
@mjpieters Can you make a PR with this change? |
I was thinking of that change as an ugly-ish work-around, not the full solution, to be honest. I'd be happy to turn it into a PR of course, it's your tech debt to pay later ;-) |
@mjpieters We have plans replacing this test adapter entirely, there are several issues with the current test adapter. This will be a temporary solution for this issue. |
Rather than try and parse out the parametrized portion of the nodeid (delimited by square brackets but possibly containing square brackets), use native [pytest item attributes](https://docs.pytest.org/en/6.2.x/reference.html#function) to separate out the decoration. Better solution for microsoft#17357, fixing microsoft#17676.
Rather than try and parse out the parametrized portion of the nodeid (delimited by square brackets but possibly containing square brackets), use native [pytest item attributes](https://docs.pytest.org/en/6.2.x/reference.html#function) to separate out the decoration. Better solution for microsoft#17357, fixing microsoft#17676.
Rather than try and parse out the parametrized portion of the nodeid (delimited by square brackets but possibly containing square brackets), use native [pytest item attributes](https://docs.pytest.org/en/6.2.x/reference.html#function) to separate out the decoration. Better solution for microsoft#17357, fixing microsoft#17676.
As a workaround, you can use import pytest
@pytest.mark.parametrize("invalid_char", [
pytest.param("]", id='closing-bracket'),
])
def test_pytest_fixture(invalid_char):
assert invalid_char == "]" also, FWIW:
Not a misspelling - English just happens to have four valid spellings of that word. pytest-parawtf to the rescue! (no, please don't use it in production...) |
This works around a VS Code bug with weird node IDs: microsoft/vscode-python#17676
* Use pytest data to extract the parametrised decoration Rather than try and parse out the parametrized portion of the nodeid (delimited by square brackets but possibly containing square brackets), use native [pytest item attributes](https://docs.pytest.org/en/6.2.x/reference.html#function) to separate out the decoration. Better solution for #17357, fixing #17676. * Add news entry * Update news/2 Fixes/17676.md Co-authored-by: Karthik Nadig <[email protected]> Co-authored-by: Karthik Nadig <[email protected]>
This works around a VS Code bug with weird node IDs: microsoft/vscode-python#17676
This works around a VS Code bug with weird node IDs: microsoft/vscode-python#17676
Hello! We have just finished our testing rewrite and are beginning the roll out to users. If you are able, it would be very helpful to know if your issue still exists on the rewrite! To try it yourself, add this setting to your users You can confirm you have the rewrite enabled by setting Let me know if the rewrite fixes your issue. Thanks! |
Because we have not heard back with the information we requested, we are closing this issue for now. If you are able to provide the info later on, then we will be happy to re-open this issue to pick up where we left off. Happy Coding! |
Environment data
python.languageServer
setting: PylanceExpected behaviour
Non-fatal test discovery.
Actual behaviour
Runtime error causes test discovery to fail.
Steps to reproduce:
Commenting out the above test case removes the symptoms of the bug.
Logs
The text was updated successfully, but these errors were encountered: