Please lets just use the New Name already ... Im sick of magical knobs, they cause so much pain and suffering down the line
Am 5. April 2017 20:44:21 MESZ schrieb holger krekel <[email protected]>: >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
