smarie / python-pytest-cases

Separate test code from test cases in pytest.
https://smarie.github.io/python-pytest-cases/
BSD 3-Clause "New" or "Revised" License
344 stars 40 forks source link

`fixture` gets confused by functions with all default parameters #279

Open vyasr opened 2 years ago

vyasr commented 2 years ago

If all the parameters to a fixture function have default values, pytest.fixture handles them appropriately but pytest_cases.fixture does not.

import pytest
import pytest_cases

# This works
@pytest.fixture
def fix(x=1):
    return 2 * x

# This works again
@pytest_cases.fixture
def fix2(request, x=1):
    return 2 * x

# This fails
@pytest_cases.fixture
def fix3(x=1):
    return 2 * x

My use case is the dynamic generation of fixtures in a loop and using default parameters as a way to avoid late binding issues. I could of course make use of functools.partial or in general find some other way to introduce another stack frame to avoid the value changing within the closure where the function is defined, and for the moment just tossing in the unused request fixture also solves my problem, so this isn't a blocker but certainly surprising behavior.

smarie commented 2 years ago

Thanks for reporting @vyasr ! as for other issues, it would be great to have a stacktrace/explanation rather than "this fails" :) I'll have a look

smarie commented 2 years ago

I am able to reproduce the issue.

(...)
..\src\pytest_cases\fixture_core1_unions.py:223: in ignore_unused
    new_sig = add_signature_parameters(old_sig, last=Parameter('request', kind=Parameter.POSITIONAL_OR_KEYWORD))
..\.nox\tests-3-9-env-pytest-latest\lib\site-packages\makefun\main.py:1089: in add_signature_parameters
    return s.replace(parameters=lst)
..\.nox\tests-3-9-env-pytest-latest\lib\inspect.py:2883: in replace
    return type(self)(parameters,
..\.nox\tests-3-9-env-pytest-latest\lib\inspect.py:2817: in __init__
    raise ValueError(msg)
E   ValueError: non-default argument follows default argument

The issue comes from the fact that in ignore_unused I modify the signature of the wrapper I add the "request" parameter at the end of all parameters. We could modify the code so that request is inserted just before the first argument with a default value if any exists, or last.

Note that the same issue happens at another place in the code, the problem can be reproduced like this:

@pytest_cases.fixture
@pytest_cases.parametrize(foo=[True])
def fix4(foo, x=2):
    return 2 * x

Would you like to try a PR ? Note that the code could be used to improve the underlying add_signature_parameters function in makefun. So the PR could directly be in makefun. Here is a proposal feature description : https://github.com/smarie/python-makefun/issues/82

vyasr commented 2 years ago

Apologies, I spent a lot of time trying to strip these issues down into easily reproducible MWEs and conveniently forgot to post tracebacks :) I'll remember (if there is a) next time.

Yes, that error is that I observe, and your proposed fix is consistent. Thanks for the help in tracing it back! I didn't spend all that much time trying to identify the root cause since I assumed that this issue (and #278) would be much quicker and easier for you to diagnose. I spent most of my time tracing #280 since it was much more convoluted and I had a much harder time creating a simple reproducer.

Yes, I'm happy to try to make a PR. I'll follow up on that issue, I do have one question there. Depending on what you think I guess the fix would involve a minor update to the call to add_signature_parameters in pytest-cases after the fix is in pytest-makefun.

smarie commented 2 years ago

Perfect ! Thanks for the feedback