pytest icon indicating copy to clipboard operation
pytest copied to clipboard

No API to request fixtures from pytest.Item

Open cjw296 opened this issue 8 years ago • 26 comments

It made me a bit sad that I had to c'n'p this piece of code, rather than call a helper, to request fixtures in an Item subclass:

https://github.com/cjw296/sybil/blob/552d3ff2d6e8639ee5d3c8efa26f5e535e064ae8/sybil/integration/pytest.py#L41

cjw296 avatar Jun 05 '17 14:06 cjw296

That's a little clumsy I agree. Unfortunately I'm not sure we want to expose the current fixture API as official right now, given that time and time again we mention that this part of the code could use some refactoring...

nicoddemus avatar Jun 06 '17 01:06 nicoddemus

@RonnyPfannschmidt asked me to log this on IRC, so doing as requested :-)

cjw296 avatar Jun 06 '17 05:06 cjw296

@nicoddemus it helps to get an understanding of the severity of design problems if we get those that suffer them log them here ^^

RonnyPfannschmidt avatar Jun 06 '17 05:06 RonnyPfannschmidt

I'm not complaining about @cjw296 posting the issue at all (sorry if it looked like that way), I was just replying to it. 😁

nicoddemus avatar Jun 06 '17 11:06 nicoddemus

Closing this issue as the proposal has been inactive for over a year.

Zac-HD avatar Feb 25 '19 10:02 Zac-HD

This is kind of annoying. Closing an issue because no-one has had a chance to work on it doesn't make the issue go away. @nicoddemus / @RonnyPfannschmidt - is this some new way of trying to clean up the tracker? It's not a great experience...

cjw296 avatar Feb 25 '19 10:02 cjw296

@Zac-HD i'm under the impression that your cleanups are more zealous than necessary

RonnyPfannschmidt avatar Feb 25 '19 11:02 RonnyPfannschmidt

Sorry - I'm still finding the right balance for that.

Zac-HD avatar Feb 25 '19 11:02 Zac-HD

Would it be possible to re-open this issue? (I mean, unless the problem hasn't actually been fixed, in which case, if you could point me at what I need to do instead of https://github.com/cjw296/sybil/blob/master/sybil/integration/pytest.py#L42-L57)

cjw296 avatar Feb 25 '19 11:02 cjw296

@Zac-HD - unfortunately, the "right way" to do this is to actually look at each issue, do the digging, and see if the problem has been fixed or is no longer applicable. If that's true, then ping your findings on the ticket and close it...

...if not, by all means ask if the OP still cares, and if they don't respond within a week or two, then it's a safe bet it can be closed.

cjw296 avatar Feb 25 '19 11:02 cjw296

Sorry - I'm still finding the right balance for that.

No worries! I for one definitely appreciate your periodic triage of the issue tracker. We also try to find the right balance, don't worry. And we can always re-open an issue if we find that's the correct course of action.

nicoddemus avatar Feb 25 '19 21:02 nicoddemus

I just ran into this issue. I want to use some functionality that's only available in fixtures (namely, capture logs) inside a test implemented as pytest.Item.runtest being generated by pytest_collect_file.

oprypin avatar Oct 23 '20 00:10 oprypin

Some fallout from #11868:

  • https://github.com/smarie/python-pytest-cases/issues/330
  • https://github.com/simplistix/sybil/issues/107

What's the current state on:

Unfortunately I'm not sure we want to expose the current fixture API as official right now, given that time and time again we mention that this part of the code could use some refactoring...

Given that IIRC we had a lot of refactorings to fixture implementations recently? @bluetech @sadra-barikbin

The-Compiler avatar Jan 29 '24 08:01 The-Compiler

the large, awesome and numerous refactorings @bluetech and @sadra-barikbin poured in over the last few months, are but the tip of the iceberg

i'm still very terrified of opening the api up, as i'm aware of a lot of the backlogs and pains ever since the 2016 sprint i wish there was more time and attention that i could pour into the topic as well - the progressions that @sadra-barikbin and @bluetech brought in are very uplifting to say the least

RonnyPfannschmidt avatar Jan 29 '24 09:01 RonnyPfannschmidt

i'm still very terrified of opening the api up

While I share that fear, we probably don't need/should not just open the API, we can probably start small with functions/methods which are based on pytest's external design, rather than exposing implementation details.

Just an example from the top of my head, we could introduce a method to pytest.Item which returns a tree-like structure with all the fixture values active on it. This does not reflect the internal state of things, but seems like it would be stable enough regardless of changes in implementation: pytest will always have the notion of fixtures active on an item, and fixture closures.

nicoddemus avatar Jan 30 '24 00:01 nicoddemus

@cjw296 Can you describe what "request fixtures from pytest.Item" means? It is not very clear. I assume it's not literally requesting a fixture, since request.getfixturevalue() exists.

bluetech avatar Jan 30 '24 11:01 bluetech

@bluetech i believe it relates to the initially linked code dealing with non-python items

as far as fixture configuration, parameterization and state management is concerned, a lot of it is has so far primarily stuck on python details

RonnyPfannschmidt avatar Jan 30 '24 13:01 RonnyPfannschmidt

as far as fixture configuration, parameterization and state management is concerned, a lot of it is has so far primarily stuck on python details

Right, I'm mostly interested to know which fixture functionality these non-python items want to use (i.e. use cases), and what they currently implement using the pytest internals.

bluetech avatar Jan 30 '24 17:01 bluetech

The initially linked code shows what they do

RonnyPfannschmidt avatar Jan 30 '24 18:01 RonnyPfannschmidt

@bluetech - ultimately, I need to do this:

class SybilItem(pytest.Item):

...

    def setup(self) -> None:
        for name, fixture in ...:
            self.example.namespace[name] = fixture

...but to do that, I currently need to do this:

https://github.com/simplistix/sybil/blob/0fc6f9cca420d46481e0e4bf3668b40f38034abc/sybil/integration/pytest.py#L51-L70

...and then this:

https://github.com/simplistix/sybil/blob/0fc6f9cca420d46481e0e4bf3668b40f38034abc/sybil/integration/pytest.py#L84-L87

If there's a better, more supported way to do this, I would love a PR to show me how!

cjw296 avatar Jan 31 '24 07:01 cjw296

What is fixture in this example?

bluetech avatar Jan 31 '24 19:01 bluetech

What is fixture in this example?

The instance of whatever the fixture is.

For example, if this were a normal pytest function:


def test_foo(bar: Baz) -> None:
    ...

...then name='bar' and fixture is an instance of type Baz.

cjw296 avatar Jan 31 '24 19:01 cjw296

A note: while supporting basic fixtures is one thing and probably possible, supporting parametrization will be quite a bit more difficult. Even for pytest's own doctests it's broken -- the following errors out:

import pytest

@pytest.fixture(autouse=True, params=[1, 2])
def number(request, doctest_namespace):
    doctest_namespace["number"] = request.param

def func():
    """
    >>> number < 10
    True
    """

bluetech avatar Feb 20 '24 23:02 bluetech

A note: while supporting basic fixtures is one thing and probably possible, supporting parametrization will be quite a bit more difficult.

I think it is OK to only support basic fixtures initially, with an error if a parametrized fixture is requested. This will probably already cover the majority of cases.

nicoddemus avatar Mar 08 '24 12:03 nicoddemus

+1 for adding this API

gryznar avatar Mar 08 '24 12:03 gryznar

The function definition type needs to be hoisted into nodes

RonnyPfannschmidt avatar Mar 08 '24 16:03 RonnyPfannschmidt