davidhalter / jedi

Awesome autocompletion, static analysis and refactoring library for python
http://jedi.readthedocs.io
Other
5.78k stars 508 forks source link

Add support for pytest fixtures #791

Closed blueyed closed 4 years ago

blueyed commented 7 years ago

Pytest fixtures are decorated functions that can be dynamically requested in test functions, simply by using them as function arguments.

I could imagine Jedi supporting this:

import pytest

class Obj:
    myattr = 1

@pytest.fixture
def fix():
    return Obj()

def test_fix(fix):
    fix.myattr  # gets not completed

f = fix()
f.myattr  # gets completed

In test_fix it could be known that fix is a pytest.fixture and then that information could be used for completion.

What works currently with Jedi is using type annotations for this (def test_fix(fix: Obj)), but typically that involves importing Obj in / before your test function, since Obj gets imported typically only in the fixture function itself.

Two things make this more difficult:

  1. fixtures are typically defined outside of the test function (and are not imported), in a pytest plugin or conftest module (which is basically a local pytest plugin AFAIK). For this Jedi probably needs support from pytest to get those, based on the current file / location.
  2. pytest.fixture has an optional name kwarg, which allows to use a different name, which adds an additional layer of redirection to this.

I'd be happy to help out with this, of course - also with regard to adding/improving interfaces in pytest.

Related: #257

cpdean commented 7 years ago

would it be possible to implement this in the general case or would exceptions have to be made for pytest's case? nothing about python gives you the hint as to where the fixture is defined, it comes into play as pytest collects tests and finds the fixtures by name as they are defined in the given file, related contest.py, and project dependences that define their own pytest fixtures.

does Jedi have a plugin system where that sort of stuff could be kept? i feel like this would have to be isolated from Jedi in general, as it seems like you'd have to pull quite a bit in scope to find the right fixture

davidhalter commented 7 years ago

This would be awesome. I'm using py.test a lot and think this would be really useful.

Is there always a py.test.ini if it's a py.test project? That would be a pretty easy way to check if it's a py.test project.

does Jedi have a plugin system where that sort of stuff could be kept? i feel like this would have to be isolated from Jedi in general, as it seems like you'd have to pull quite a bit in scope to find the right fixture

I agree. However a plugin system would definitely be right for this. However I haven't really thought about this and it feels like alot of work. I just don't know really what good entry points would be for such a plugin.

I guess I would be ok with supporting py.test for now (within Jedi) and eventually moving it to a separate module. At the same time I'm not even sure. I think it's OK to support certain core-python libraries (like pytest or django) within Jedi. It seems a bit awkward to need to install a lot of plugins (that might not be well maintained).

blueyed commented 4 years ago

There's some discussion about this in https://github.com/pytest-dev/pytest/issues/5981, based on using type annotations.

blueyed commented 4 years ago

How could we handle plugin hooks (like def pytest_addoption(parser):), which are pluggy hooks. In this case I would like it to be handled like if parser was annotated with type _pytest.config.argparsing.Parser already, which could come via the hookspec then:

@hookspec(historic=True)
def pytest_addoption(parser: …):

I could imagine that this works for mypy via a mypy plugin, but that's nothing Jedi uses currently, is it?

Not that this is similar to fixtures in general, but there it would have to discover which fixture is actually used (based on nearest conftest etc - a helper from within pytest would be useful here I guess).

davidhalter commented 4 years ago

What's your use case for this? I'm not sure where it would help.

blueyed commented 4 years ago

@davidhalter Mainly writing pytest plugins / working on pytest itself. Not very common (and not as important for myself as using fixtures in general), but I've figured that those things might be related internally then.

davidhalter commented 4 years ago

I just don't really understand where Jedi could help you (assuming it does magic :)).

blueyed commented 4 years ago

That I get completion for config, parser etc there - i.e. Jedi would know what type the arguments are of - the same as what is the idea with fixtures in general.

davidhalter commented 4 years ago

@blueyed

I just pushed support for pytest fixtures. It looks pretty nice. It should also work with jedi-vim. goto works, inferring works and completions as well.

Are the pytest_* methods still something where you would like to have signature information? If this is the case how many "hooks" like pytest_addoption are regularly used?

blueyed commented 4 years ago

@davidhalter Great. I could not get it to work however, given the following t_jedi.py file:

import pytest

class Foo:
    def bar():
        pass

@pytest.fixture
def fix() -> Foo:
    return Foo()

def test(fix, monkeypatch):
    monkeypatch.
    fix.

monkeypatch. gets no completions, and fix. only generic dunder ones (I've expected bar to be completed there).

Not sure how https://github.com/davidhalter/jedi/blob/3ec73f1da36ae6d6f623f8012ded4436d6115f03/jedi/plugins/pytest.py#L8-L13 works, but if it is a list of default fixtures to use, it should also include ("_pytest", "pytester") probably (for testdir etc).

davidhalter commented 4 years ago

Looks like I understood test discovery wrong (I only searched for test_ and not test). https://docs.pytest.org/en/latest/goodpractices.html#conventions-for-python-test-discovery

but if it is a list of default fixtures to use, it should also include ("_pytest", "pytester") probably (for testdir etc).

Done.

davidhalter commented 4 years ago

I decided not to pursue stuff like def pytest_addoption(parser: …): .... While this would be nice, I feel like this is too much work for its gain. If somebody is willing to sponsor it, I might do it, but I don't feel like this helps too many people "a lot".

I generally use pytest a lot and I use those method very rarely, so I feel like apart from you @blueyed, and some pytest core devs nobody uses this stuff as intensely :).

blueyed commented 4 years ago

@davidhalter Sounds reasonable.

I've noticed that with the following it will go to the fixture itself (with "goto" on the "caplog" function argument), and not the "parent fixture":

@pytest.fixture
def caplog(caplog):
    yield caplog

Completion however works when using caplog in a test function, but only when it is in the same file.

I've noticed both issues with something along the following in a conftest.py file:

@pytest.fixture
def caplog(caplog):
    # from loguru import logger
    #
    # handler = LoguruPropogateHandler()
    # handler_id = logger.add(handler, format="{message}")
    yield caplog
    # logger.remove(handler_id)

With that "goto" from a test goes there, but completion with caplog. does not work in a test then.

(Feel free to create an issue from this comment - I wasn't sure about it myself.)

davidhalter commented 4 years ago

I have to think if I support this case, but reopening for now so I don't forget it.

How exactly does pytest do inheritance there?

blueyed commented 4 years ago

@davidhalter Thanks, that works better.

But it sill does not go to the original fixture from/via "goto" on the "testdir" arg with the following in a conftest.py file - it goes to the function itself:

@pytest.fixture
def testdir(testdir):
    return testdir

When using it in a test_*.py file it works as expected though (i.e. it goes to the one in the conftest module then).

davidhalter commented 4 years ago

Thanks for bringing it up. I fixed it now. Forgot to test this. Let me know if you find further stuff.

blueyed commented 4 years ago

@davidhalter Awesome, thanks!

I've noticed that it does not work for request, which gets added manually in https://github.com/pytest-dev/pytest/blob/96bc4cc0b97939032442f845f897cca15574b135/src/_pytest/fixtures.py#L275-L294. Would it be good/possible to map this to FixtureRequest automatically (for completion)?

The code around this is also how fixtures get filled in (your previous question), which cannot be answered in a simple way I fear (but the current method appears to work really good already in general).

blueyed commented 4 years ago

The first goto from a test jumps to the argument in the test function - I think it would be better if it would go to the fixture definition directly (but goto-assignment should go to the argument (which it does currently also)).

davidhalter commented 4 years ago

The first goto from a test jumps to the argument in the test function - I think it would be better if it would go to the fixture definition directly (but goto-assignment should go to the argument (which it does currently also)).

At the moment this is entirely intentional. If we want to change that in the future, we can add maybe an additional param go Script.goto(follow_imports=True, follow_pytest_fixtures=True). But since currently this only happens for pytest, I feel like we should at least wait for a bit. At some point I might reconsider if it's really annoying. So feel free to bring it up again in a year or so.

BTW: There's also goto_definitions_command in jedi-vim, which is unassigned by default. That goes directly to the fixture.

Would it be good/possible to map this to FixtureRequest automatically (for completion)?

I'm not against that per se, but it's not a priority at all for me. So I would consider merging it, but I won't do it.

AlexChalk commented 3 years ago

Thanks so much for adding this! Can I expect it to work with a language server that leverages jedi, e.g. https://github.com/pappasam/jedi-language-server or https://github.com/palantir/python-language-server?

I've tried jumping to pytest fixtures from both of these (and jedi-vim actually) but nothing happened, and I wanted to check that goto fixture definitions is still an expected behaviour before digging deeper.

davidhalter commented 3 years ago

I'm pretty sure this works quite well, since I use it very often. The palantir language server will probably not work, because it uses Jedi <0.18.0. The other language server should probably work.

You should probably check that you have jedi >= 0.18.0 in your dependencies.