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

Add support for pytest fixtures #791

Closed
blueyed opened this issue Oct 14, 2016 · 21 comments
Closed

Add support for pytest fixtures #791

blueyed opened this issue Oct 14, 2016 · 21 comments
Labels

Comments

@blueyed
Copy link
Contributor

blueyed commented Oct 14, 2016

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

cpdean commented Oct 15, 2016

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

davidhalter commented Oct 23, 2016

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
Copy link
Contributor Author

blueyed commented Oct 17, 2019

There's some discussion about this in pytest-dev/pytest#5981, based on using type annotations.

@blueyed
Copy link
Contributor Author

blueyed commented Oct 31, 2019

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

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

@blueyed
Copy link
Contributor Author

blueyed commented Nov 8, 2019

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

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

@blueyed
Copy link
Contributor Author

blueyed commented Nov 8, 2019

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

@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
Copy link
Contributor Author

blueyed commented Dec 30, 2019

@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

_PYTEST_FIXTURE_MODULES = [
('_pytest', 'monkeypatch'),
('_pytest', 'capture'),
('_pytest', 'logging'),
('_pytest', 'tmpdir'),
]
works, but if it is a list of default fixtures to use, it should also include ("_pytest", "pytester") probably (for testdir etc).

@davidhalter
Copy link
Owner

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

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
Copy link
Contributor Author

blueyed commented Jan 4, 2020

@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 davidhalter reopened this Jan 4, 2020
@davidhalter
Copy link
Owner

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
Copy link
Contributor Author

blueyed commented Jan 5, 2020

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

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

@blueyed
Copy link
Contributor Author

blueyed commented Jan 7, 2020

@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
Copy link
Contributor Author

blueyed commented Jan 7, 2020

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

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

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

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.

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

4 participants