We have the raises() helper function, look in the tests for it. See for example:

    x = Symbol('x')
    raises(TypeError, "len(x)")

It usually makes sense to have a test that proves your patch actually
works. Here you could for example add a test that would lead without
your patch to an "ambiguous outcome" and now raises an exception
instead. I agree that such a test is not very important, but even just
an example in the commit message would already make review easier.

There is no policy for testing exceptions, you should do it if you
think it makes sense. There is however a policy that every new
functionality has to be tested. In this case I would be fine if there
is no test (I leave this up to you), but your commit message could
explain the "ambiguous outcome".

Thanks!

Vinzent

2010/3/19 Christian Muise <christian.mu...@gmail.com>:
> Not sure what such a test would look like...other oddities are left
> unhandled such as Not(). What's the policy for sympy on testing thrown
> errors?
>
>   Cheers
>
> On Fri, Mar 19, 2010 at 4:53 PM, Vinzent Steinberg
> <vinzent.steinb...@googlemail.com> wrote:
>>
>> 2010/3/17 Christian Muise <christian.mu...@gmail.com>:
>> > ---
>> >  sympy/logic/boolalg.py |    6 ++----
>> >  1 files changed, 2 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/sympy/logic/boolalg.py b/sympy/logic/boolalg.py
>> > index 0a45e42..97c47b7 100755
>> > --- a/sympy/logic/boolalg.py
>> > +++ b/sympy/logic/boolalg.py
>> > @@ -109,10 +109,8 @@ class Implies(BooleanFunction):
>> >     """
>> >     @classmethod
>> >     def eval(cls, *args):
>> > -        if len(args) < 2:
>> > -            raise ValueError, "Only %d operand(s) used for an Implies
>> > (pairs are required): %s" % (len(args), str(args))
>> > -        elif len(args) > 2:
>> > -            return map(cls, args)
>> > +        if len(args) != 2:
>> > +            raise ValueError, "%d operand(s) used for an Implies (pairs
>> > are required): %s" % (len(args), str(args))
>> >         else:
>> >             return Or(Not(args[0]), args[1])
>> >
>> > --
>> > 1.6.3.3
>>
>> Would it make sense to add a test?
>>
>> Vinzent

-- 
You received this message because you are subscribed to the Google Groups 
"sympy-patches" group.
To post to this group, send email to sympy-patc...@googlegroups.com.
To unsubscribe from this group, send email to 
sympy-patches+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/sympy-patches?hl=en.

Reply via email to