On Friday 24 May 2013 at 17:07, holger krekel 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. >
I cannot say that the contextmanager was obvious the first time I first saw it. But neither was generators and the yield statement. :-) I do think that using yield contextmanager-style that is a very good fit for fixtures. People that are not familiar/happy with the idea contextmanager, generators and with statements might just want to continue use request.addfinalizer, which is totally fine. About the API/naming: I like and prefer the current trunk behavior (just one API: pytest.fixture). If you find pytest.contextfixture more explicit and clear - go for it. I will be very happy with "context fixtures" whatever they are called. :-) Cheers Andreas _______________________________________________ Pytest-dev mailing list [email protected] http://mail.python.org/mailman/listinfo/pytest-dev
