RFC: addCleanup should take *args and **kwargs?

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Apr 13 21:42:51 BST 2007


>>>>> "john" == John Arbash Meinel <john at arbash-meinel.com> writes:

    john> Aaron Bentley wrote:
    >> Vincent Ladeuil wrote:
    >>>>>>>> "robert" == Robert Collins <robertc at robertcollins.net> writes:
    robert> On Fri, 2007-04-13 at 08:03 +0200, Vincent Ladeuil wrote:
    >>> >> 
    >>> >> +1, as long as we take care of checking the presence of the
    >>> >> callable *and* its (*args, **kwargs) 
    >> 
    robert> What do you mean by this?
    >> 
    >>> addCleanup checks that callable is not already in the cleanup
    >>> list, if we use the same callable for different parameters, it
    >>> should still be added, not rejected as a dupe.
    >> 
    >>> Nit-picking, better safe than sorry, etc :)
    >> 
    >> I don't think you can predict that
    >> 
    >> 1. If a programmer mistakently adds a cleanup twice, that this fix will
    >> catch it
    >> 2. If a programmer adds a cleanup twice, it is a mistake.

I agree, I can't predict that :)

But the *actual* implementation does that. Which demonstrate that
the actual test code never add a cleanup twice.

My point was that by replacing lambdas (unique by definition) by
callables + parameters, the *actual* check will fire for code
that is running (and correct I hope) today. Nothing more.

    >> 
    >> So I don't think this is a good idea.  If double-adding a
    >> cleanup causes an error, the programmer will see this and
    >> fix it.  If it is not an error, it should not be
    >> forbidden.

Then the *actual* check must be removed.

    john> I agree. I was just thinking about potentials for a
    john> "lock/unlock" combo.  Where one of the callers might be
    john> helpful by adding 'unlock()' for you.  But the test
    john> code adds another lock, so you need to add another
    john> unlock.

    john> I would probably rather have addCleanup not check for
    john> duplicates.

I never fired the ValueError exception in addCleanup myself so
I'm not worried about firing it by mistake.

      Vincent




More information about the bazaar mailing list