[pytest-dev] [3.1 feature] "assert not raise exception" helper: opinions about the name

Wed Apr 5 16:48:35 EDT 2017

Hi all,

After this whole thread I'm now starting to lean towards being -1 on the
entire concept of the feature.  It tries to optimise the Don't Repeat
Yourself thing for one rather specific corner case where the cognitive
load on pytest itself seems to become higher then what is saved by doing
things with the existing API.  Even some simple normal python code of
the test allows DRY right now (wrap code in a closure/function + if
statement).

Anyway, just my opinion.

Regards,
Floris

Ronny Pfannschmidt <opensource at ronnypfannschmidt.de> writes:

> 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 <holger at merlinux.eu>:
>>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 <holger at merlinux.eu>
>>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
>><nicoddemus at gmail.com>
>>> > wrote:
>>> > > >>
>>> > > >>> Hi all,
>>> > > >>>
>>> > > >>> On Fri, Mar 31, 2017 at 7:13 AM Vasily Kuznetsov
>><kvas.it at gmail.com>
>>> > > >>> 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
> pytest-dev at python.org
> https://mail.python.org/mailman/listinfo/pytest-dev