-
Notifications
You must be signed in to change notification settings - Fork 31
fix issue #24: LazyFixture only resolved when used directly, ... #40
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,10 +49,21 @@ def pytest_fixture_setup(fixturedef, request): | |
|
|
||
|
|
||
| def pytest_runtest_call(item): | ||
| def _rec_part(val): | ||
| if is_lazy_fixture(val): | ||
| return item._request.getfixturevalue(val.name) | ||
| elif type(val) == tuple: | ||
| return tuple(item._request.getfixturevalue(v.name) if is_lazy_fixture(v) else _rec_part(v) for v in val) | ||
| elif type(val) == list: | ||
| return list(item._request.getfixturevalue(v.name) if is_lazy_fixture(v) else _rec_part(v) for v in val) | ||
| elif isinstance(val, dict): | ||
| return {key: item._request.getfixturevalue(v.name) if is_lazy_fixture(v) else _rec_part(v) | ||
| for key, v in val.items()} | ||
| return val | ||
|
|
||
| if hasattr(item, 'funcargs'): | ||
| for arg, val in item.funcargs.items(): | ||
| if is_lazy_fixture(val): | ||
| item.funcargs[arg] = item._request.getfixturevalue(val.name) | ||
| item.funcargs[arg] = _rec_part(val) | ||
|
|
||
|
|
||
| @pytest.hookimpl(hookwrapper=True) | ||
|
|
@@ -165,11 +176,19 @@ def _tree_to_list(trees, leave): | |
| return lst | ||
|
|
||
|
|
||
| def lazy_fixture(names): | ||
| if isinstance(names, string_type): | ||
| def lazy_fixture(names=None, *args, **kwargs): | ||
| if isinstance(names, string_type) and not args and not kwargs: | ||
| return LazyFixture(names) | ||
| else: | ||
| return [LazyFixture(name) for name in names] | ||
| elif not kwargs: | ||
| names = [names] if isinstance(names, string_type) else names | ||
| names, is_tuple = (list(names), True) if isinstance(names, tuple) else (names, False) | ||
| names.extend(args) | ||
| if is_tuple: | ||
| return tuple(LazyFixture(name) for name in names) | ||
| else: | ||
| return [LazyFixture(name) for name in names] | ||
| elif kwargs and not (args or names): | ||
| return {key: LazyFixture(value) for key, value in kwargs.items()} | ||
|
Comment on lines
+179
to
+191
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is an unnecessary to create lazy fixtures this way. I regret adding a way to pass a list of names to create multiple fixtures :)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I find it pretty convenient. Yes it is not so obvious as direct dict or list creating, but at least it helps to make your lines less then 120 symbol lengths. :)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should keep it simple. If you need dict, list or tuple of lazy fixtures just use appropriate python notation to create this objects. If you are doing it in many places just abstract this logic to your own function. |
||
|
|
||
|
|
||
| def is_lazy_fixture(val): | ||
|
|
||
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'm not sure that it is a correct way to solve the problem, because it will try to find lazy fixture inside of every test'
pytest.fixture(params). At least it should be disabled by default and maybe through the marks to enable this behaviour.Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry is a problem with dictionaries only? Or with list too?
Actually the library checks it now, so only some more levels are added :)
But I think decrease amount of checks is a good idea. But how to do it?
Add new attr to pytest.fixture?
or add specific object:
Will it works?
Uh oh!
There was an error while loading. Please reload this page.
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 found the better option:
add an option to pytest.ini lookup for lazy_fixtures (turn off by default).
using decorator @pytest.mark.lookup_for_lazy_fixtures().
If global config is turned on we always look, if it turned off we check for decorator in item.own_markers.
BTW do you concern about slowing down performance, because of lazy_fixtures lookup?
I am not sure it will have a big impact, because usually (at least on project I worked on :) ) test doesn't use big collections with a lot of nested collections inside.
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.
Yeah, I think about using marks too. I will take a look a little bit later though.
Yep :) Also I don't like unknown structure of the param. Maybe it will be a good idea to specify format so we will know where to lookup lazy_fixture?
Uh oh!
There was an error while loading. Please reload this page.
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 am trying to run some benchmark, but something broke:
Do you have any idea what's wrong?
I got your concern, but I think specifying format is a source of many possible issues, the users can muddle up or forget to specify it, also there can be a problem with parsing the format, and it can create bugs in lib, also that can be slower than just checking all collections. BTW we decided to keep it simple and specifying format is not very intuitive and convenient for users. :)