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

Discover pytest fixtures through imports #1786

Open
idanarye opened this issue Jul 7, 2021 · 10 comments
Open

Discover pytest fixtures through imports #1786

idanarye opened this issue Jul 7, 2021 · 10 comments
Labels

Comments

@idanarye
Copy link

idanarye commented Jul 7, 2021

On big projects you sometimes want to split organize your fixtures in many files. This means importing the fixtures into either conftest.py or - for better modularity - into the test file itself.

#791 added support for pytest fixtures discovery, but it only looks for fixtures defined directly inside the test file or directly inside conftest.py. Is it possible to make it look for fixtures imported into these files, just like it would do with regular completion?

@davidhalter
Copy link
Owner

This is definitely possible. It's mainly a performance question/issue. I would myself be happy to have this changed. We might just want to follow direct imports, but not other things.

In general it's definitely possible to use it for goto, but for completions, we might need to reconsider it, because of potential speed issues (especially for large projects).

It's a bit unfortunate, but I have been making a lot of these kind of decisions to avoid performance issues. However, I'm currently rewriting Jedi in Rust (with a lot of builtin type caching). I hope that that will eventually fix the issue for people like you. But feel free to play around with this issue.

@davidhalter
Copy link
Owner

But: Feel free to work on this and get it working. Now that I'm reading the code I'm pretty sure that _is_pytest_func is the issue (in jedi/plugins/pytest.py).

@idanarye
Copy link
Author

I looked at the code, and while I don't claim to fully understand it, it does not look like _is_pytest_func is at fault. It's purpose is to detect the test function - a task that does not care about the origin of the fixtures in said test function.

I noticed that:

  1. _iter_pytest_modules does not yield the modules that are star imported in conftest.py or in the test files.
  2. FixtureFilter does not even look at star imports, and does not follow explicit by-name imports.

I'm not sure which of the two should be fixed. I don't think it's a problem in _iter_pytest_modules, because it just lists modules without looking at their context, but then again FixtureFilter is only looking at the module's syntax tree so I'm not sure it alone can provide the solution.

@davidhalter
Copy link
Owner

My idea was to allow _is_pytest_func to check if it is part of an import. If that is the case, it would be possible to infer the underlying function and check again. That imples that is_pytest_func would need additional arguments.

Star imports are a different beast altogether and I'm pretty sure that it's not a good idea to to follow those for performance reasons.

@idanarye
Copy link
Author

I'm not sure I understand.

def complete_param_names(func):
    def wrapper(context, func_name, decorator_nodes):
        module_context = context.get_root_context()
        if _is_pytest_func(func_name, decorator_nodes):
            names = []
            for module_context in _iter_pytest_modules(module_context):
                names += FixtureFilter(module_context).values()
            if names:
                return names
        return func(context, func_name, decorator_nodes)
    return wrapper

(it is also used in infer_anonymous_param and in goto_anonymous_param indirectly through _is_a_pytest_param_and_inherited, but complete_param_names uses it directly and also uses _iter_pytest_modules directly, so let's focus on there)

So let's say we have this:

import pytest

from some_module_that_defines_fixtures import foo


@pytest.fixture
def bar():
    pass


def test_main():
    pass

And we are trying to complete arguments for test_main. complete_param_names will call _is_pytest_func which will return True, and then _iter_pytest_modules and FixtureFilter will be used to populate the names list which will get returned as completion suggestions.

_is_pytest_func is already correctly recognizing test_main as a pytest function. That's why we get bar as a completion suggestion. How would modifying it, even adding more arguments to it, going to help us get foo as a completion suggestion?

@davidhalter
Copy link
Owner

Sorry my bad. You are of course right. _is_pytest_func does a different job than what I imagined.

The thing that needs improvement is the FixtureFilter, as you noted above. There we could check if a name is an import and then act accordingly.

@idanarye
Copy link
Author

How? I tried putting some prints into FixtureFilter._filter, but it does not seem to even look at the star import...

@davidhalter
Copy link
Owner

As I wrote before, star imports are out of scope for this one (for performance reasons). However with normal imports it would work.

@idanarye
Copy link
Author

I still don't get why star imports have to be a performance issue. Isn't Jedi already following them for regular completion and introspection?

@davidhalter
Copy link
Owner

It just means there are even more modules to analyze.

At least for fixture searching we already do a heuristic that avoids a lot of type inference. I guess I'm a bit annoyed that it's already slow for me and I don't want to get it even slower. But you are probably right that it's not as bad as I think. If there are no star imports in a module it's probably almost a no-op and in case there are, people probably want them in their results. So I guess we could add that as well, even if it makes it a bit slower.

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

No branches or pull requests

2 participants