Hi Bruno, thanks for going through user-testing, good idea!
On Wed, Apr 05, 2017 at 18:19 +0000, Bruno Oliveira wrote: > Hi Ronny, Holger, all, > > On Wed, Apr 5, 2017 at 1:31 PM holger krekel <[email protected]> wrote: > > > On Wed, Apr 05, 2017 at 17:25 +0200, Ronny Pfannschmidt wrote: > > > while experimenting i learned of the fun fact that None is a "valid > > > exception type" > > > for except clauses on at least python 2.7 > > > however on python3 it is invalid and a type error, > > > > I agree with Florian in that the Python behavior for try/except None should > not really be an argument in favor or against pytest.raises(None), given > that try/except None is clearly an accident of implementation which has > been fixed in Python 3. > > > since the usage patterns of python don't hold such a case, > > > i'd like to use a distinct building block for expressing it in order to > > > match the semantics of the language closer > > > > several people said however that "with pytest.raises(None): ..." reads > > good to them. > > > > I decided to go around the office and do a small poll, asking my colleagues > what they thought "pytest.raises(None)" meant when shown different pieces > of code. > > Example 1: > > def test(): > with pytest.raises(None): > foo() > > Overall answers/thoughts: > > 1. "I expect pytest.raises(None) to assert that an exception of *any* type > is raised" > 2. "I expect pytest.raises(None) to assert that no exception will be raised > by the block" > 3. "Expect that foo() should raise None? Wait, is that possible?" (it isn't) > 4. "I think pytest.raise(None) should always be an error, seems like it > could let an error slip through"" > > When showing this example: > > @pytest.mark.parametrize('v, exc', [ > ('', ValueError), > (1, None), > ]) > def test(v, exc): > with pytest.raises(exc): > validate_int(v) > > Things then made more sense everyone, but most people still thought it > weird/surprising and another person still preferred that > pytest.raises(None) to immediately throw an explicit error about None not > being accepted (and that an exception type should be used instead). Hum, wondering what about a "pytest.NoExceptionRaised" as a special value: @pytest.mark.parametrize('v, exc', [ ('', ValueError), (1, pytest.NoExceptionRaised), ]) def test(v, exc): with pytest.raises(exc): validate_int(v) If it makes some sense to you could you ask your colleagues how they feel about that? It also means that if someone just looks at the pytest.* namespace content there is confusing "pytest.do_not_raise" as a general helper. best, holger > > When shown the "pytest.do_not_raise" alternative most people liked it > because it is explicit, there's no guessing about what it means and it is > easy to find in the documentation, albeit a little more verbose. > > At first I was also convinced that the semantics of pytest.raises(None) > would be obvious, but it seems that's not the case (at least in my small > poll of around the office). > > Given all that I'm now leaning towards going through the safe route of > being more explicit albeit more verbose. > > What do others think? > > Cheers, > Bruno. > > > > > > holger > > > > > - Ronny > > > > > > On 04.04.2017 19:19, holger krekel wrote: > > > > On Tue, Apr 04, 2017 at 15:48 +0000, Bruno Oliveira wrote: > > > >> On Fri, Mar 31, 2017 at 8:24 AM Bruno Oliveira <[email protected]> > > wrote: > > > >> > > > >>> Hi all, > > > >>> > > > >>> On Fri, Mar 31, 2017 at 7:13 AM Vasily Kuznetsov <[email protected]> > > > >>> wrote: > > > >>> > > > >>> Hi Holger and Ronny, > > > >>> > > > >>> I see merit in both of your points. All those "is not None" checks in > > > >>> between other logic and the proposed "raises unless the argument is > > None" > > > >>> semantics of pytest.raises do increase complexity (I'm not qualified > > to > > > >>> predict whether or not this increase is significant in terms of > > further > > > >>> maintenance though) but at the same time "pytest.raises(None)" API is > > > >>> convenient in some cases. What if we do something like this: > > > >>> > > > >>> ... > > > >>> > > > >>> The "is not None" checks are gone from the main logic and both APIs > > are > > > >>> available. Or if we would rather not have more than one way to do > > it, we > > > >>> can drop "does_not_raise" but otherwise still keep it a separate > > context. > > > >>> > > > >>> Seems like if we can agree on the API, a not-complexity-increasing > > option > > > >>> of implementation is possible. > > > >>> > > > >>> > > > >>> Now for the specific case of pytest.raises(None), I believe we can > > strike > > > >>> a good balance between a nice user interface and minimal internal > > impact > > > >>> like Vasily proposes, unless Ronny or others feel that > > pytest.raises(None) > > > >>> is not a good interface for this functionality. > > > >>> > > > >> Guys, anything else to add here? I would like to move the > > implementation > > > >> forward, either in its current form or changing it to > > pytest.raises(None). > > > > i was and am fine with your suggestion! > > > > > > > > IMO compared to markers or fixtures ... "pytest.raises" is relatively > > > > self-contained side-effect-free code so i don't think it's neccessary > > > > to get scared about maintanability too much in this case. > > > > > > > > cheers, holger > > > > > > > >> Ronny, after the discussion here are you still against using > > > >> ptyest.raises(None), given that we can implement it easily by Vasily's > > > >> suggested implementation? > > > >> > > > >> Cheers, > > > >> Bruno. > > > > > > > > _______________________________________________ pytest-dev mailing list [email protected] https://mail.python.org/mailman/listinfo/pytest-dev
