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

fixture gets confused by functions with all default parameters #279

Open
vyasr opened this issue Jun 15, 2022 · 4 comments
Open

fixture gets confused by functions with all default parameters #279

vyasr opened this issue Jun 15, 2022 · 4 comments

Comments

@vyasr
Copy link
Contributor

vyasr commented Jun 15, 2022

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
Copy link
Owner

smarie commented Jun 19, 2022

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
Copy link
Owner

smarie commented Jun 19, 2022

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 : smarie/python-makefun#82

@vyasr
Copy link
Contributor Author

vyasr commented Jun 19, 2022

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
Copy link
Owner

smarie commented Jun 20, 2022

Perfect ! Thanks for the feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants