In light of the examples, IMHO, I agree that fixtures being explicit about using yield as context-managers is preferable.
I like @pytest.contextfixture, it is easy to look-up and understand since it mimics what we already have in contextlib. On Fri, May 24, 2013 at 12:07 PM, holger krekel <[email protected]> wrote: > On Fri, May 24, 2013 at 16:50 +0200, Harro van der Klauw wrote: > > As long as it throws an error hinting that you might need yielding=True > it > > should be obvious on how to fix > > the backwards incompatibility issue as soon as you run your tests. > > We cannot easily throw an error with a hint. Consider this example: > > import pytest > > @pytest.fixture > def fix(): > yield 1 > yield 2 > > def test_fix(fix): > for x in fix: > assert x < 3 > > This runs fine on pytest-2.3.5. On trunk it gives this error: > > ... > > fix = 1 > > def test_fix(fix): > > for x in fix: > assert x < 3 > E TypeError: 'int' object is not iterable > > I've never written or seen somebody writing such a generator fixture, > though. > And what you would need to do is rewrite the fixture: > > @pytest.fixture > def fix(): > def gen(): > yield 1 > yield 2 > return gen() > > Then again, when i first saw the contextlib.contextmanager decorator > i found it not very intuitive. Did anyone? It looks like a hack. > From that angle i'd rather go for requiring "contextyield=True" or > @pytest.contextfixture because that can be looked up in documentation > and thus is easier to read for people not familiar with yields/contextlib. > > best, > holger > > > I don't see a big problem with this, updating of a requirement is > something > > that you should never do automatically. > > > > Cheers, > > Harro > > > > > > > > On 24 May 2013 16:36, Andreas Pelme <[email protected]> wrote: > > > > > On Thursday 9 May 2013 at 15:56, holger krekel wrote: > > > > This is probably used by very few people but to be on the safe side, > > > > we probably should introduce a flag like this: > > > > > > > > @pytest.fixture(ctx=True) # signal this is a context manager style > > > fixture > > > > def fix(): > > > > yield 1 > > > > > > > > What do you think? Any other suggestions for the flag name? > > > > > > > > I'd rather not introduce something like @pytest.contextfixture > > > > because it would be a duplication of the API (scope, params). > > > > But i am open to be convinced otherwise. > > > > > > I agree that another API like contextfixture should be avoided. > > > > > > We had a short discussion on IRC about this, Holger had the idea of > doing > > > the opposite if ctx=True - a new default argument "yielding=False" > which > > > would make it possible to restore the old behavior by putting > yielding=True > > > on fixtures that would be affected by this. > > > > > > A fixture that is a generator that currently looks like this > > > > > > @pytest.fixture > > > def fix(): > > > yield 1 > > > yield 2 > > > > > > Would then have to be changed to > > > > > > @pytest.fixture(yielding=True) > > > def fix(): > > > yield 1 > > > yield 2 > > > > > > This is backward incompatible, but given that it seems questionable if > > > "generator fixtures" useful/is used, with a note in the release notes > and > > > documentation I think this could be a good compromise. > > > > > > I will be happy to give this a try if it is decided this could be a > good > > > approach! > > > > > > Cheers > > > Andreas > > > > > > > > > _______________________________________________ > > > Pytest-dev mailing list > > > [email protected] > > > http://mail.python.org/mailman/listinfo/pytest-dev > > > > > > _______________________________________________ > > Pytest-dev mailing list > > [email protected] > > http://mail.python.org/mailman/listinfo/pytest-dev > > _______________________________________________ > Pytest-dev mailing list > [email protected] > http://mail.python.org/mailman/listinfo/pytest-dev >
_______________________________________________ Pytest-dev mailing list [email protected] http://mail.python.org/mailman/listinfo/pytest-dev
