Patchwork [1,of,6,events,v3] util: add event handling mechanism

login
register
mail settings
Submitter Gregory Szorc
Date Sept. 28, 2014, 9:01 p.m.
Message ID <d513c232aeba16137b29.1411938099@vm-ubuntu-main.gateway.sonic.net>
Download mbox | patch
Permalink /patch/6015/
State Changes Requested
Headers show

Comments

Gregory Szorc - Sept. 28, 2014, 9:01 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1411933407 25200
#      Sun Sep 28 12:43:27 2014 -0700
# Node ID d513c232aeba16137b291456536adee0d88a040f
# Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
util: add event handling mechanism

The patch adds a generic event handling mechanism to Mercurial. From a
high level, you create a class with methods corresponding to event
names. You can then register functions/callbacks against events that
get called when that event fires.

As will be demonstrated in subsequent patches, event handling can be
considered an internal hooks mechanism and will provide a better
alternative to wrapping or monkeypatching.

The intent of the events system is to give extensions a more
well-defined point for code insertion. Currently, extension authors
have a limited set of hooks and a giant pile of functions to choose
from. Hooks often don't satisfy your requirements and you need to dig
through a pile of code to find an appropriate function to intercept.
Then you need to replace/monkeypatch this function. This is an inexact
science and is difficult to do robustly. The result are extensions that
do live dangerously.

Events will provide a better mechanism for code insertion. Events solve
the discovery problem by providing a well-defined (like hooks) set of
places for supported code insertion. Events are also easier to code,
as extension authors don't need to worry about monkeypatching: just
write a function and register it.

Events have another advantage over monkeypatching in that they can be
instance specific. Monkeypatching often results in changing symbols
on modules or class types as opposed to individual methods on
individual object instances. Oftentimes you only want to apply
customization to a single instance of an object if that object meets
certain criteria. In the current world, you often have to globally
replace and filter out invocations at call time that aren't appropriate.
This is prone to failure due to monkeypatched functions not taking all
uses into consideration. Furthermore, monkeypatching can be difficult
for module-level symbols. If a module-level function is imported by
another module (from foo import bar), you'll need to monkeypatch that
imported symbol as well. It is time consuming for module authors to
keep up with Mercurial changes and to ensure that imported uses are
always monkeypatched. Again, events solve this challenge.
Gregory Szorc - Sept. 28, 2014, 9:14 p.m.
On 9/28/2014 2:01 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1411933407 25200
> #      Sun Sep 28 12:43:27 2014 -0700
> # Node ID d513c232aeba16137b291456536adee0d88a040f
> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
> util: add event handling mechanism
> 
> The patch adds a generic event handling mechanism to Mercurial. From a
> high level, you create a class with methods corresponding to event
> names. You can then register functions/callbacks against events that
> get called when that event fires.

I talked with mpm about this feature at the Munich sprint and I believe
this patch series incorporates the improvements he was looking for.
Namely, the .register() bit was replaced by standalone classes holding
event definitions in a more Pythonic way. mpm and I discussed using
decorators. However, I believe __new__ (as implemented) fits the bill
just fine.

There is one feature notably absent from this patch series:
documentation. I'd like to automatically generate documentation for all
available events so extension authors have something that isn't raw
source code to consult. However, I'm not sure where to put this. I don't
think `hg help` is appropriate since events aren't user facing. `hg
debugevents` perhaps?

If incorporated, this patch series could make refactors to facilitate
monkeypatching a thing of the past, as events can satisfy many of the
use cases where monkeypatching was needed. As a recent example, Mike
Hommey's recent refactorings to facilitate additional bundle2 part
handling could likely have been avoided if bundle2 event support were
present. This patch series should thus likely result in less code churn
and thus increased code comprehension.
Pierre-Yves David - Sept. 29, 2014, 9:58 p.m.
On 09/28/2014 02:14 PM, Gregory Szorc wrote:
> On 9/28/2014 2:01 PM, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1411933407 25200
>> #      Sun Sep 28 12:43:27 2014 -0700
>> # Node ID d513c232aeba16137b291456536adee0d88a040f
>> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
>> util: add event handling mechanism
>>
>> The patch adds a generic event handling mechanism to Mercurial. From a
>> high level, you create a class with methods corresponding to event
>> names. You can then register functions/callbacks against events that
>> get called when that event fires.
>
> I talked with mpm about this feature at the Munich sprint and I believe
> this patch series incorporates the improvements he was looking for.
> Namely, the .register() bit was replaced by standalone classes holding
> event definitions in a more Pythonic way. mpm and I discussed using
> decorators. However, I believe __new__ (as implemented) fits the bill
> just fine.

I've walked on the __new__ path in the past (well with metaclass, but 
this is all the same) and I'm really not eager to go down that road in 
Mercurial. Decorator should be just fine.


> There is one feature notably absent from this patch series:
> documentation. I'd like to automatically generate documentation for all
> available events so extension authors have something that isn't raw
> source code to consult. However, I'm not sure where to put this. I don't
> think `hg help` is appropriate since events aren't user facing. `hg
> debugevents` perhaps?

+1 on forcing event to be declared and documented

> If incorporated, this patch series could make refactors to facilitate
> monkeypatching a thing of the past, as events can satisfy many of the
> use cases where monkeypatching was needed. As a recent example, Mike
> Hommey's recent refactorings to facilitate additional bundle2 part
> handling could likely have been avoided if bundle2 event support were
> present. This patch series should thus likely result in less code churn
> and thus increased code comprehension.

Does not apply for the bundle2 thing Mike did. In this case it is 
actually required to wrap//override existing functon to achieve a goal.
Mads Kiilerich - Sept. 29, 2014, 10:24 p.m.
On 09/28/2014 11:01 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1411933407 25200
> #      Sun Sep 28 12:43:27 2014 -0700
> # Node ID d513c232aeba16137b291456536adee0d88a040f
> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
> util: add event handling mechanism
>
> The patch adds a generic event handling mechanism to Mercurial. From a
> high level, you create a class with methods corresponding to event
> names. You can then register functions/callbacks against events that
> get called when that event fires.
>
> As will be demonstrated in subsequent patches, event handling can be
> considered an internal hooks mechanism and will provide a better
> alternative to wrapping or monkeypatching.
>
> The intent of the events system is to give extensions a more
> well-defined point for code insertion.

That sounds a bit like aspect-oriented programming, while events can be 
many other things. Why is not called AOP and why is it called events?

> Events will provide a better mechanism for code insertion. Events solve
> the discovery problem by providing a well-defined (like hooks) set of
> places for supported code insertion. Events are also easier to code,
> as extension authors don't need to worry about monkeypatching: just
> write a function and register it.

That might be true, but this patch series do not provide any evidence of 
what improvements it actually could bring ... or that it is a good 
mechanism for this kind of improvements. Actual use of this mechanism in 
the built-in extensions would help a lot to validate it. Without actual 
use it just adds quite a bit of complexity and no benefits.

> +class event(object):
> +    '''An event with its handlers.
> +
> +    An ``event`` is essentially a collection of functions that will be invoked
> +    when the event fires. ``event`` instances are typically created by defining
> +    methods on ``eventmanager`` instances.
> +
> +    Handler functions can be registered against an instance via the ``+=``
> +    operator. They can be unregistered via the ``-=`` operator.
> +
> +    Handler functions can be invoked by calling an ``event`` instance like
> +    it is a function.
> +
> +    Handlers are executed in the order they are registered.
> +
> +    The return value of handler functions is ignored.

I guess this imply that it should be "pure" functions that can't have 
side effects (or any effect at all?) unless the functions are curryed 
with some context? But they are allowed to raise exceptions? Is it 
accepted or frowned upon to modify the parameters?

Please consider saying something about this intended use here in the 
docstring. Actual use of it will of course also give some examples.

> +class eventmanager(object):
> +    '''A collection of events.
> +
> +    This class powers the internal events system. Instances of this class are
> +    typically attached to an object, but they can be standalone.
> +
> +    This class is an abstract base class. Actual event containers should
> +    subclass this class. Events are registered by defining methods on these
> +    subclasses. The methods should define the named arguments the event
> +    accepts and a docstring describing the purpose of the event. The custom
> +    __new__ implementation of this class will automagically convert each
> +    method to an ``event`` instance.
> +
> +    Methods lacking docstrings will result in an exception during class
> +    creation. This requirement serves to reinforce the well-defined intent
> +    of event APIs.

-1 to enforcing it just to enforce it. Mandatory docstrings is a policy 
- that should not be hardcoded.

> +    Arguments defined in the event/method definition are parsed for special
> +    meaning. If an argument shares a name with a named argument passed to the
> +    ``eventmanager`` constructor, the value passed to the ``eventmanager``
> +    constructor will be used as the default value for that argument in the
> +    event. In addition, if a default value is defined on the method/event,
> +    that default value will be used when calling events. Method-specific
> +    defaults override the "global" defaults from the ``eventmanager``
> +    constructor.
> +
> +    For example, say numerous events pass a repository instance into events.
> +    You can pass the repo as a named argument to the class constructor to
> +    save some typing at event call time. e.g.
> +
> +    >>> class repoevents(eventmanager):
> +    ...     def myevent(repo):
> +    ...         """Some repo event."""
> +    >>> def handler(repo):
> +    ...     print(repo['name'])
> +    >>> repo = {'name': 'my repo'}

I find the use of 'repo' in this example confusing. This dict is so far 
from what a 'repo' is. The example makes more sense to me if calling the 
dict 'data' or 'config' or having a different kind of 'context'.

/Mads
Gregory Szorc - Sept. 29, 2014, 10:44 p.m.
On 9/29/14 3:24 PM, Mads Kiilerich wrote:
> On 09/28/2014 11:01 PM, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1411933407 25200
>> #      Sun Sep 28 12:43:27 2014 -0700
>> # Node ID d513c232aeba16137b291456536adee0d88a040f
>> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
>> util: add event handling mechanism
>>
>> The patch adds a generic event handling mechanism to Mercurial. From a
>> high level, you create a class with methods corresponding to event
>> names. You can then register functions/callbacks against events that
>> get called when that event fires.
>>
>> As will be demonstrated in subsequent patches, event handling can be
>> considered an internal hooks mechanism and will provide a better
>> alternative to wrapping or monkeypatching.
>>
>> The intent of the events system is to give extensions a more
>> well-defined point for code insertion.
>
> That sounds a bit like aspect-oriented programming, while events can be
> many other things. Why is not called AOP and why is it called events?

I don't care what color we paint the bike shed. Give me a better 
nomenclature and I'll update the patch series.

>> Events will provide a better mechanism for code insertion. Events solve
>> the discovery problem by providing a well-defined (like hooks) set of
>> places for supported code insertion. Events are also easier to code,
>> as extension authors don't need to worry about monkeypatching: just
>> write a function and register it.
>
> That might be true, but this patch series do not provide any evidence of
> what improvements it actually could bring ... or that it is a good
> mechanism for this kind of improvements. Actual use of this mechanism in
> the built-in extensions would help a lot to validate it. Without actual
> use it just adds quite a bit of complexity and no benefits.

The thing is that I wrote this patch series to solve problems that I as 
an extension author have come across. The intended use case for this 
feature is mostly extensions. I thought I had mpm's buy-in at the sprint 
that this was a good idea.

If you insist, I could find use cases inside core to make use of events.

>> +class event(object):
>> +    '''An event with its handlers.
>> +
>> +    An ``event`` is essentially a collection of functions that will
>> be invoked
>> +    when the event fires. ``event`` instances are typically created
>> by defining
>> +    methods on ``eventmanager`` instances.
>> +
>> +    Handler functions can be registered against an instance via the
>> ``+=``
>> +    operator. They can be unregistered via the ``-=`` operator.
>> +
>> +    Handler functions can be invoked by calling an ``event`` instance
>> like
>> +    it is a function.
>> +
>> +    Handlers are executed in the order they are registered.
>> +
>> +    The return value of handler functions is ignored.
>
> I guess this imply that it should be "pure" functions that can't have
> side effects (or any effect at all?) unless the functions are curryed
> with some context? But they are allowed to raise exceptions? Is it
> accepted or frowned upon to modify the parameters?

Behavior depends on the event. The pre-push events could modify the 
pushop, for example.

Currently, we don't do any exception catching anywhere. This could 
presumably be extended so individual call sites catch and react to 
certain exceptions. For example, we could provide events that give 
extensions a well-defined location to abort the current procedure.

>> +class eventmanager(object):
>> +    '''A collection of events.
>> +
>> +    This class powers the internal events system. Instances of this
>> class are
>> +    typically attached to an object, but they can be standalone.
>> +
>> +    This class is an abstract base class. Actual event containers should
>> +    subclass this class. Events are registered by defining methods on
>> these
>> +    subclasses. The methods should define the named arguments the event
>> +    accepts and a docstring describing the purpose of the event. The
>> custom
>> +    __new__ implementation of this class will automagically convert each
>> +    method to an ``event`` instance.
>> +
>> +    Methods lacking docstrings will result in an exception during class
>> +    creation. This requirement serves to reinforce the well-defined
>> intent
>> +    of event APIs.
>
> -1 to enforcing it just to enforce it. Mandatory docstrings is a policy
> - that should not be hardcoded.

You disagree with marmoute :/

Personally, I find machines are better at enforcing policies than 
humans. Hence this code. I suppose I could move this to a test...
Pierre-Yves David - Sept. 29, 2014, 10:57 p.m.
On 09/29/2014 03:44 PM, Gregory Szorc wrote:
> On 9/29/14 3:24 PM, Mads Kiilerich wrote:
>> On 09/28/2014 11:01 PM, Gregory Szorc wrote:
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>> # Date 1411933407 25200
>>> #      Sun Sep 28 12:43:27 2014 -0700
>>> # Node ID d513c232aeba16137b291456536adee0d88a040f
>>> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
>>> util: add event handling mechanism
[…]
> If you insist, I could find use cases inside core to make use of events.

Finding user in core will ensure the code is run and tested and prevent 
regression in the future.

>> -1 to enforcing it just to enforce it. Mandatory docstrings is a policy
>> - that should not be hardcoded.
>
> You disagree with marmoute :/

Yop, this is usually how the two of use interact ;-) Sometimes produce 
good results.
Matt Mackall - Oct. 13, 2014, 8:51 p.m.
On Sun, 2014-09-28 at 14:01 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1411933407 25200
> #      Sun Sep 28 12:43:27 2014 -0700
> # Node ID d513c232aeba16137b291456536adee0d88a040f
> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
> util: add event handling mechanism

I guess this series is waiting on an in-core user to demonstrate its
utility. Going to drop for now.

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -13,9 +13,9 @@  This contains helper routines that are i
 hide platform-specific details from the core.
 """
 
 from i18n import _
-import error, osutil, encoding
+import error, osutil, encoding, inspect
 import errno, shutil, sys, tempfile, traceback
 import re as remod
 import os, time, datetime, calendar, textwrap, signal, collections
 import imp, socket, urllib
@@ -2059,4 +2059,142 @@  def debugstacktrace(msg='stacktrace', sk
     f.flush()
 
 # convenient shortcut
 dst = debugstacktrace
+
+class event(object):
+    '''An event with its handlers.
+
+    An ``event`` is essentially a collection of functions that will be invoked
+    when the event fires. ``event`` instances are typically created by defining
+    methods on ``eventmanager`` instances.
+
+    Handler functions can be registered against an instance via the ``+=``
+    operator. They can be unregistered via the ``-=`` operator.
+
+    Handler functions can be invoked by calling an ``event`` instance like
+    it is a function.
+
+    Handlers are executed in the order they are registered.
+
+    The return value of handler functions is ignored.
+
+    When events are created, they are "bound" to 0 or more values which will
+    be passed to every handler function in addition to the values passed to
+    that event. To reduce potential for confusion and to increase awareness
+    of changes by consumers, all arguments are passed as named (not
+    positional) arguments.
+
+    e.g.
+
+    >>> def handler(foo, bar, baz):
+    ...     print '%s %s %s' % (foo, bar, baz)
+    >>> e = event(foo='foo', bar='bar')
+    >>> e += handler
+    >>> e(baz='baz')
+    foo bar baz
+    '''
+
+    def __init__(self, **kwargs):
+        # Convert to list to facilitate + operator later.
+        self._args = kwargs
+        self._handlers = []
+
+    def __iadd__(self, fn):
+        if fn not in self._handlers:
+            self._handlers.append(fn)
+        return self
+
+    def __isub__(self, fn):
+        self._handlers.remove(fn)
+        return self
+
+    def __len__(self):
+        return len(self._handlers)
+
+    def __call__(self, **kwargs):
+        args = dict(self._args)
+        args.update(kwargs)
+        for fn in self._handlers:
+            fn(**args)
+
+class eventmanager(object):
+    '''A collection of events.
+
+    This class powers the internal events system. Instances of this class are
+    typically attached to an object, but they can be standalone.
+
+    This class is an abstract base class. Actual event containers should
+    subclass this class. Events are registered by defining methods on these
+    subclasses. The methods should define the named arguments the event
+    accepts and a docstring describing the purpose of the event. The custom
+    __new__ implementation of this class will automagically convert each
+    method to an ``event`` instance.
+
+    Methods lacking docstrings will result in an exception during class
+    creation. This requirement serves to reinforce the well-defined intent
+    of event APIs.
+
+    Arguments defined in the event/method definition are parsed for special
+    meaning. If an argument shares a name with a named argument passed to the
+    ``eventmanager`` constructor, the value passed to the ``eventmanager``
+    constructor will be used as the default value for that argument in the
+    event. In addition, if a default value is defined on the method/event,
+    that default value will be used when calling events. Method-specific
+    defaults override the "global" defaults from the ``eventmanager``
+    constructor.
+
+    For example, say numerous events pass a repository instance into events.
+    You can pass the repo as a named argument to the class constructor to
+    save some typing at event call time. e.g.
+
+    >>> class repoevents(eventmanager):
+    ...     def myevent(repo):
+    ...         """Some repo event."""
+    >>> def handler(repo):
+    ...     print(repo['name'])
+    >>> repo = {'name': 'my repo'}
+    >>> e = repoevents(repo=repo)
+    >>> e.myevent += handler
+    >>> e.myevent()
+    my repo
+    '''
+    def __new__(cls, **kwargs):
+        # We start with a new, empty object.
+        o = super(eventmanager, cls).__new__(cls)
+
+        for attr in dir(cls):
+            if attr.startswith('_'):
+                continue
+
+            method = getattr(cls, attr)
+
+            if not getattr(method, '__doc__'):
+                raise ValueError('methods of eventmanager classes must '
+                    'contain a docstring: %s' % attr)
+
+            args, varargs, keywords, defaults = inspect.getargspec(method)
+            # defaults is None or an iterable. defaults corresponds to the last
+            # N arguments from args. This one-liner aligns things properly.
+            methoddefaults = dict(zip(reversed(args), reversed(defaults or ())))
+            eventdefaults = {}
+            for arg in args:
+                # Every argument in the method definition gets turned into
+                # a default argument to the created event.
+                #
+                # If there is a default value in the method signature, it has
+                # the highest priority.
+                #
+                # If the argument exists in the constructor arguments, use it
+                # as a fallback.
+                #
+                # If it is a positional argument (no default value could be
+                # located), we set the default argument value to None. This
+                # ensures that all arguments defined as part of the event
+                # definition actually get passed to the event. This prevents
+                # a disconnect between event definitions and their use.
+                eventdefaults[arg] = methoddefaults.get(arg,
+                    kwargs.get(arg, None))
+
+            setattr(o, attr, event(**eventdefaults))
+
+        return o
diff --git a/tests/test-events.py b/tests/test-events.py
new file mode 100644
--- /dev/null
+++ b/tests/test-events.py
@@ -0,0 +1,168 @@ 
+from mercurial.util import event, eventmanager, safehasattr
+
+import unittest
+import silenttestrunner
+
+class testevents(unittest.TestCase):
+    def testeventsimple(self):
+        e = event()
+        self.assertEqual(len(e), 0)
+
+        calls = {'h1': 0, 'h2': 0}
+
+        def h1():
+            calls['h1'] += 1
+
+        def h2():
+            calls['h2'] += 1
+
+        e += h1
+        self.assertEqual(len(e), 1)
+        e += h2
+        self.assertEqual(len(e), 2)
+        e += h2
+        self.assertEqual(len(e), 2)
+
+        e()
+        self.assertEqual(calls, {'h1': 1, 'h2': 1})
+        e()
+        self.assertEqual(calls, {'h1': 2, 'h2': 2})
+
+        e -= h1
+        e()
+        self.assertEqual(calls, {'h1': 2, 'h2': 3})
+
+    def testeventarguments(self):
+        e = event()
+
+        calls = {'h1': [], 'h2': []}
+
+        def h1(foo, bar, baz=False):
+            calls['h1'].append((foo, bar, baz))
+
+        def h2(foo, bar, baz=True):
+            calls['h2'].append((foo, bar, baz))
+
+        e += h1
+        e += h2
+
+        e(foo=1, bar=2, baz=3)
+        self.assertEqual(calls, {'h1': [(1, 2, 3)], 'h2': [(1, 2, 3)]})
+        e(foo=3, bar=4, baz=None)
+        self.assertEqual(calls, {'h1': [(1, 2, 3), (3, 4, None)],
+                                 'h2': [(1, 2, 3), (3, 4, None)]})
+        e(foo=5, bar=6)
+        self.assertEqual(calls, {'h1': [(1, 2, 3), (3, 4, None), (5, 6, False)],
+                                 'h2': [(1, 2, 3), (3, 4, None), (5, 6, True)]})
+
+    def testeventdefaultargs(self):
+        expected = [1, True]
+        def h(obj, foo, bar=True):
+            self.assertEqual(o, obj)
+            self.assertEqual(foo, expected[0])
+            self.assertEqual(bar, expected[1])
+
+        o = object()
+        e = event(obj=o, foo=1)
+        e += h
+
+        e()
+        expected = [2, False]
+        e(foo=2, bar=False)
+
+    def testeventmanagerbasic(self):
+        # Base class does not do anything special.
+        o = eventmanager()
+        self.assertFalse(safehasattr(o, 'e1'))
+        try:
+            o.e1()
+        except AttributeError:
+            pass
+
+        # Subclass with methods results in event creation.
+        class testevents1(eventmanager):
+            def e1():
+                '''Event e1'''
+
+            def e2():
+                '''Event e2'''
+
+        o = testevents1()
+        self.assertTrue(safehasattr(o, 'e1'))
+        self.assertTrue(safehasattr(o, 'e2'))
+        self.assertEqual(type(getattr(o, 'e1')), event)
+        self.assertEqual(type(getattr(o, 'e2')), event)
+
+        # Verify that registered functions get called properly.
+        calls = {'e1': 0, 'e2': 0}
+
+        def e1h():
+            calls['e1'] += 1
+        def e2h():
+            calls['e2'] += 1
+
+        o.e1 += e1h
+        o.e2 += e2h
+        o.e1()
+        self.assertEqual(calls, {'e1': 1, 'e2': 0})
+        o.e2()
+        self.assertEqual(calls, {'e1': 1, 'e2': 1})
+        o.e1()
+        self.assertEqual(calls, {'e1': 2, 'e2': 1})
+
+    def testeventmanagerbadmethods(self):
+        class nodocstring(eventmanager):
+            def badevent():
+                pass
+
+        self.assertRaisesRegexp(ValueError, 'must contain a docstring',
+            nodocstring)
+
+    def testeventmanagerdefaultargument(self):
+        class defaults(eventmanager):
+            def event1(foo=True, bar=1):
+                '''Event 1'''
+
+            def event2(foo, bar):
+                '''Event 2'''
+
+        expected1 = {'foo': True, 'bar': 1}
+        def h1(**kwargs):
+            self.assertEqual(kwargs, expected1)
+
+        expected2 = {'foo': None, 'bar': None}
+        def h2(**kwargs):
+            self.assertEqual(kwargs, expected2)
+
+        o = defaults()
+        o.event1 += h1
+        o.event2 += h2
+        o.event1()
+        o.event2()
+
+        expected1 = {'foo': False, 'bar': 1}
+        o.event1(foo=False)
+        expected1 = {'foo': True, 'bar': 2}
+        o.event1(foo=True, bar=2)
+
+        expected2 = {'foo': True, 'bar': None}
+        o.event2(foo=True)
+
+    def testeventmanagerkwargsdefaults(self):
+        class defaults(eventmanager):
+            def event1(foo, bar=1):
+                '''Event 1'''
+
+        expected = {'foo': True, 'bar': 1}
+        def h1(**kwargs):
+            self.assertEqual(kwargs, expected)
+
+        o = defaults(foo=True)
+        o.event1 += h1
+        o.event1()
+
+        expected = {'foo': False, 'bar': 1}
+        o.event1(foo=False)
+
+if __name__ == '__main__':
+    silenttestrunner.main(__name__)