-
-
Couldn't load subscription status.
- Fork 2.9k
[FEAT] Exposing autoused fixture names as global variable for tests #13180
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
Conversation
7f1fea9 to
698f8ea
Compare
|
If there's a way to disable automatic pipeline runs, it'd be great. I didn't get a chance to locally run tests and properly investigate new behaviour yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll stand corrected on the implementation part of things, that's easier than expected, at least on first sight.
edit: Or it might not be, since it completely disregards test methods so far.
However, I'm still a clear -2 on this, pytest should not magically inject globals into functions in ways they would otherwise never work with default Python namespace semantics.
Contrary to what you seem to claim in #13172 (reply in thread) I don't think this is something pytest already does. And it shouldn't.
| async_fail(pyfuncitem.nodeid) | ||
| funcargs = pyfuncitem.funcargs | ||
| testargs = {arg: funcargs[arg] for arg in pyfuncitem._fixtureinfo.argnames} | ||
| autoused = {arg: funcargs[arg] for arg in funcargs if arg not in testargs} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will now define globals for any indirectly used fixture:
import pytest
@pytest.fixture
def inner():
return 42
@pytest.fixture
def outer(inner):
pass
def test_outer(outer):
assert inner == 42There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[...] pytest should not magically inject globals into functions in ways they would otherwise never work with default Python namespace semantics.
I agree that enforcing this as a default behaviour might be a bit much, but offerring a autoparam=True option as suggested in #9955 could be a great compromise.
This will now define globals for any indirectly used fixture
I'm not sure it's a problem, since if outer is autoused, then inner becomes defacto autoused.
Furthermore, with a slightly more advanced implementation introducing autoparam=False as a default behaviour, inner would not get exposed unless wanted. So this should be completely fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contrary to what you seem to claim in #13172 (reply in thread) I don't think this is something pytest already does.
You're right about that, I apologize. pytest operates a lot of magic already, but maybe not this kind as I assumed it did! My bad 😞
…test-dev#13171) Signed-off-by: Élie Goudout <[email protected]>
698f8ea to
45bb8b6
Compare
|
Thanks @eliegoudout for the interest, but magically injecting globals into functions namespaces is not something we want to support in pytest: this makes everything around the tests harder to reason about, not to mention tools like type-checkers and IDEs, even if it would "clean" the tests a bit. |
|
Thanks for your answer. I understand your ground but am interested in getting more of your POV, if I ever whish to push this further as a plugin. I get the point regarding mypy and IDEs. I'll think a bit more about this later.
Could you be a bit more specific please? I'm not sure about your view on tests directly interacting with Just in case since its out of the main thread, consider this answer as part of the answer to "magically injecting is not desired", regarding the potential |
|
As far as I'm concerned, the proposed monkeypatching of globals goes against the zen of python Pytest ought not to recommend or support such a mechanisms |
I can hear that. Then again, I strongly feel like # test_file.py
def test_example(x):
print(x)in it self can represent a test without ever defining I'm having a hard time understanding the positioning of |
|
The big difference is that fixtures as arguments are just normal function call semantics. If you define a Python function: def myfunc(x):
print(x)then you can also proceed to call that function from somewhere else without defining Same thing with tests: pytest is calling your test function, so pytest decides what arguments to pass into your test function. The only arguably magic thing is that pytest inspects the arguments of the test function to decide what to pass, but if you imagine it like keyword arguments: # pseudo-code inside pytest
test_example(x=known_fixtures["x"])then the only magic thing is:
Those things are very far away from global variables randomly appearing without, from a Python syntax point of view, being defined anywhere at all. |
|
In your very example x is defined as input parameter There are valid concerns about the name matching mechanism, which was a reasonable thing to do back when it was invented. If we were to do it today, we'd mirror systems like fastapi or dishka and use type/annotation based dependency injection declaration However, matching defined parameter names to fixtures and filling them in for passover in a function call still has a qualitative difference to magically defining unrefined globals and then magically using said globals in a test function |
|
Thanks a lot for all your inputs!! @The-Compiler I agree that the function in itself may be ran in another environment where I give it to you both @The-Compiler @RonnyPfannschmidt that it may seem like a "next step" in tweaking python behaviour. But in effect, all this PR aims to do is emulate the following. # test_file.py
auto_used_fixture_value = ... # yielded value defined by fixture
def test_mytest():
# Test logic where `auto_used_fixture_value` is usable as global variablewhere the name is only exposed to the test being ran. Honestly, it really does not seem far fetched to me, compared to the fact that pytest emulates a bunch of fixture discovery/imports to the module, as well as, again, the setup and teardown phases that truly hide a lot of machinery compared to default python behaviour.
|
|
Python has a perfect mechanism for values passed in only to the execution of a function Parameters |
If I'm not understanding your properly, my sincere aplogies. But I take this as passive-aggressive "point", which could, again, be extended to the perfect import/raising mechanisms built in Python, that I'm sorry we came to that here, it is not, nor will it ever be, my intention. |
I still think this is the main point you fail to understand despite all our explanations. Just like ruff doesn't complain about def myfunc(x):
print(x)ruff also doesn't complain about fixtures being used in tests, because those local variables are defined, via having them as a parameter. There really isn't anything crazy going on there at all from the Python point of view. |
|
You're totally right here, my point was completely off on this one lol. |
|
After having slept on it a bit more, I'm agreeing more and more with your vision of things regarding the proposed feature, and why it might not be desirable as is. I'm trying to figure out a way to make it more pythonic and compliant with tools somehow, because I do think it could yield a cool feature. Do you think it might somehow be possible to have the following API? from pytest.autouse import autoused_fixture # Importing explicitly exposes as global variable to tests
def test_auto():
# Test logic where `autoused_fixture` is usable as global variableWhere, again, the value of I think this would be a good step towards answering a lot of all your valid concerns, what do you think? In practise though, I don't know an easy way implement this and make this compliant with, say, I precise that I'm not desperately trying to force this feature into making sense, I'm just glad you're open to discussion and wish to explore potential approaches if they are deemed sensible. Again, thank you for your time! |
Would be a potential hack that's implementable Id still strongly advise towards just using parameters |
|
Even the proposed For the fixture to work, you'd need to shadow the dummy-global with the per-test fixture value "global" anyways. Then again, this is just piling more hacks on top of hacks for a gain that's... mediocre at best. I'll once again repeat that I see pretty much a zero chance of this kind of thing getting into pytest. I see no way this can be done without a giant pile of hacks, but I've been saying that since the beginning... |
Closes #13171.
Action list
AUTHORSin alphabetical order.autoparamoption to selectively enable this feature (disabled by default), as suggested in Adding of parameters by default #9955.Exposes autoused fixture names as global variables for tests.
Here, there's no need to add
nullto the test signature! 🤩In many cases, this will clean test files significantly.
Two caveats that will be documented:
autoused_int_fixture += 1without a priorglobal autoused_int_fixturestatement in test definition..__globals__dict. So if the test actually modifiesglobals()(which looks like a terrible idea and is probably discouraged somewhere in pytest docs), it may cause issues.