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

Misleading error message when parameters are missing in the parametrize but presented in the test function #10113

Closed
3 of 4 tasks
TommyDew42 opened this issue Jul 8, 2022 · 11 comments
Labels
topic: reporting related to terminal output and user-facing messages and errors

Comments

@TommyDew42
Copy link

TommyDew42 commented Jul 8, 2022

  • a detailed description of the bug or problem you are having
  • output of pip list from the virtual environment you are using
  • pytest and operating system versions
  • minimal example if possible

I'm using pytest version 6.2.4.
Say we have the following function and a corresponding test function:

# ./add.py
def add(n1, n2):
  return n1 + n2

# ./test_add.py
@pytest.mark.parametrize(
  "arg1, expected_answer",
  [pytest.param(2, 3, 5)]
)
def test_add(arg1, arg2, expected_answer):
  assert add(arg1, arg2) == expected_answer

Running pytest ./test_add.py::test_add logs the following error message:

ERROR: not found: ./test_add.py::test_add
(no name './test_add.py::test_add' in any of [<Module test_add.py>])

But actually it fails because arg2 is not presented in the parametrize decorator.
Similar issue when the argument name is wrong.

@The-Compiler
Copy link
Member

This is the full output:

============================= test session starts ==============================
collected 0 items / 1 error                                                    

==================================== ERRORS ====================================
_________________________ ERROR collecting test_add.py _________________________
test_add.py::test_add: in "parametrize" the number of names (2):
  ['arg1', 'expected_answer']
must be equal to the number of values (3):
  (2, 3, 5)
=========================== short test summary info ============================
ERROR test_add.py
=============================== 1 error in 0.04s ===============================
ERROR: not found: /home/florian/tmp/test_add.py::test_add
(no name '/home/florian/tmp/test_add.py::test_add' in any of [<Module test_add.py>])

seems quite clear from that what the error is, no?

@The-Compiler The-Compiler added the status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity label Jul 8, 2022
@TommyDew42
Copy link
Author

TommyDew42 commented Jul 8, 2022

Yes. But if we can improve the error message in the end, the DX would be even better.

Plus sometimes there are a huge chunk of other logs in between, we have to scroll up to see the real cause of the failure.

The reason why I put up the issue is that I was misled by the error message in the end. I spent some time to figure out why pytest didn’t get to pick up my test function. But in the end it’s the typo inside parametrize that caused the error.

@Zac-HD
Copy link
Member

Zac-HD commented Jul 11, 2022

How about we report "couldn't collect" instead of "not found"?

Almost as short, but accurate in a wider variety of cases.

@TommyDew42
Copy link
Author

How much complexity will it bring to the codebase/logic if we want to explicitly display to user that test_add was found but it failed to run because there are errors in parametrize?
A simple hint will help a lot, e.g. there are errors in the decorator parametrize. Please refer to the detailed error above

If it is too complex, we can go for a simpler approach: use "couldn't collect" instead of "not found"

@Zac-HD
Copy link
Member

Zac-HD commented Jul 12, 2022

We already report that explicitly - see "errors" in the output example above - so I think it's just fixing the "short test summary info" to avoid claiming that any failure to collect was because the test was not found.

@RonnyPfannschmidt
Copy link
Member

currently we really cant know (as there wont be a collection node with the name after collection) since parameterization failed

@The-Compiler i wonder if we could do a indirect parametrize with something that auto marks as "will fail"

alternatively we could have exceptions in generate_tests yield some node that always fails/indicates collection errors

@The-Compiler
Copy link
Member

I'd be okay with a simple change to the error message, but anything more complex really doesn't seem to be worth the trouble, IMHO. I really don't think the error message is confusing as-is, personally.

@TommyDew42
Copy link
Author

I’ll let the core team make the decision then!

Just trying to propose some rooms for improvement I found (might be very personally and subjectively).

@RonnyPfannschmidt
Copy link
Member

yeah, its a unfortunate edge case, the error itself causes the item not to be "find-able"

what could help is taking a look at collection errors in requested paths and adding a note that a collection error was in the related files

however thats also tricky, so I'd put it in at low prio

@jekwatt
Copy link

jekwatt commented Jul 16, 2022

I was able to duplicate the error. Working to fix the error message.

@Zac-HD Zac-HD added topic: reporting related to terminal output and user-facing messages and errors and removed status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity labels Sep 27, 2022
@The-Compiler
Copy link
Member

As per #10145 (comment) I think a fix for this makes the message far more confusing for a way more common case. Thus, closing this.

@The-Compiler The-Compiler closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: reporting related to terminal output and user-facing messages and errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants