From fc51580345c8dac4a3f85c6119f65e4ef17b764a Mon Sep 17 00:00:00 2001
From: James Teh <jamie@nvaccess.org>
Date: Thu, 10 Aug 2017 14:00:53 +1000
Subject: [PATCH 1/3] Add generic framework to enable extensibility at specific
 points in the code by way of actions and filters.

This allows interested parties to register to be notified when some action occurs or to modify a specific kind of data.

There are two types of extension points:

1. An Action might be used to notify interested parties that the configuration profile has been switched.
2. A Filter might be used to allow interested parties to modify spoken messages before they are passed to the synthesizer.

Handlers for extension points are always called in the order they were registered so that the order is determinate, which should make behaviour more predictable and easier to reproduce.

Handlers can be called with keyword arguments.
If a handler doesn't support a particular keyword argument, it will still be called with the keyword arguments that it does support.
This means that additional keyword arguments can be added to actions and filters in future without breaking existing handlers.
---
 source/extensionPoints.py          | 203 +++++++++++++++++++++++
 tests/unit/test_extensionPoints.py | 255 +++++++++++++++++++++++++++++
 2 files changed, 458 insertions(+)
 create mode 100644 source/extensionPoints.py
 create mode 100644 tests/unit/test_extensionPoints.py

diff --git a/source/extensionPoints.py b/source/extensionPoints.py
new file mode 100644
index 00000000000..046a50c859c
--- /dev/null
+++ b/source/extensionPoints.py
@@ -0,0 +1,203 @@
+#extensionPoints.py
+#A part of NonVisual Desktop Access (NVDA)
+#Copyright (C) 2017 NV Access Limited
+#This file is covered by the GNU General Public License.
+#See the file COPYING for more details.
+
+"""Framework to enable extensibility at specific points in the code.
+This allows interested parties to register to be notified when some action occurs
+or to modify a specific kind of data.
+For example, you might wish to notify about a configuration profile switch
+or allow modification of spoken messages before they are passed to the synthesizer.
+See the L{Action} and L{Filter} classes.
+"""
+
+import weakref
+import collections
+import inspect
+from logHandler import log
+
+class AnnotatableWeakref(weakref.ref):
+	"""A weakref.ref which allows annotation with custom attributes.
+	"""
+
+class BoundMethodWeakref(object):
+	"""Weakly references a bound instance method.
+	Instance methods are bound dynamically each time they are fetched.
+	weakref.ref on a bound instance method doesn't work because
+	as soon as you drop the reference, the method object dies.
+	Instead, this class holds weak references to both the instance and the function,
+	which can then be used to bind an instance method.
+	To get the actual method, you call an instance as you would a weakref.ref.
+	"""
+
+	def __init__(self, target, onDelete):
+		def onRefDelete(weak):
+			"""Calls onDelete for our BoundMethodWeakref when one of the individual weakrefs (instance or function) dies.
+			"""
+			onDelete(self)
+		inst = target.__self__
+		func = target.__func__
+		self.weakInst = weakref.ref(inst, onRefDelete)
+		self.weakFunc = weakref.ref(func, onRefDelete)
+
+	def __call__(self):
+		inst = self.weakInst()
+		if not inst:
+			return
+		func = self.weakFunc()
+		assert func, "inst is alive but func is dead"
+		# Get an instancemethod by binding func to inst.
+		return func.__get__(inst)
+
+def _getHandlerKey(handler):
+	"""Get a key which identifies a handler function.
+	This is needed because we store weak references, not the actual functions.
+	We store the key on the weak reference.
+	When the handler dies, we can use the key on the weak reference to remove the handler.
+	"""
+	inst = getattr(handler, "__self__", None)
+	if inst:
+		return (id(inst), id(handler.__func__))
+	return id(handler)
+
+class HandlerRegistrar(object):
+	"""Base class to Facilitate registration and unregistration of handler functions.
+	The handlers are stored using weak references and are automatically unregistered
+	if the handler dies.
+	Both normal functions and instance methods are supported.
+	The handlers are maintained in the order they were registered
+	so that they can be called in a deterministic order across runs.
+	This class doesn't provide any functionality to actually call the handlers.
+	If you want to implement an extension point,
+	you probably want the L{Action} or L{Filter} subclasses instead.
+	"""
+
+	def __init__(self):
+		#: Registered handler functions.
+		#: This is an OrderedDict where the keys are unique identifiers (as returned by _getHandlerKey)
+		#: and the values are weak references.
+		self._handlers = collections.OrderedDict()
+
+	def register(self, handler):
+		if hasattr(handler, "__self__"):
+			weak = BoundMethodWeakref(handler, self.unregister)
+		else:
+			weak = AnnotatableWeakref(handler, self.unregister)
+		key = _getHandlerKey(handler)
+		# Store the key on the weakref so we can remove the handler when it dies.
+		weak.handlerKey = key
+		self._handlers[key] = weak
+
+	def unregister(self, handler):
+		if isinstance(handler, (AnnotatableWeakref, BoundMethodWeakref)):
+			key = handler.handlerKey
+		else:
+			key = _getHandlerKey(handler)
+		try:
+			del self._handlers[key]
+		except KeyError:
+			return False
+		return True
+
+	@property
+	def handlers(self):
+		"""Generator of registered handler functions.
+		This should be used when you want to call the handlers.
+		"""
+		for weak in self._handlers.values():
+			handler = weak()
+			if not handler:
+				continue # Died.
+			yield handler
+
+def callWithSupportedKwargs(func, *args, **kwargs):
+	"""Call a function with only the keyword arguments it supports.
+	For example, if myFunc is defined as:
+		def myFunc(a=None, b=None):
+	and you call:
+		callWithSupportedKwargs(myFunc, a=1, b=2, c=3)
+	Instead of raising a TypeError, myFunc will simply be called like this:
+		myFunc(a=1, b=2)
+	"""
+	spec = inspect.getargspec(func)
+	if spec.keywords:
+		# func has a catch-all for kwargs (**kwargs).
+		return func(*args, **kwargs)
+	# spec.defaults lists all arguments with defaults (keyword arguments).
+	numKwargs = len(spec.defaults) if spec.defaults else 0
+	# spec.args lists all arguments, first positional, then keyword.
+	firstKwarg = len(spec.args) - numKwargs
+	supportedKwargs = set(spec.args[firstKwarg:])
+	for kwarg in kwargs.keys():
+		if kwarg not in supportedKwargs:
+			del kwargs[kwarg]
+	return func(*args, **kwargs)
+
+class Action(HandlerRegistrar):
+	"""Allows interested parties to register to be notified when some action occurs.
+	For example, this might be used to notify that the configuration profile has been switched.
+
+	First, an Action is created:
+
+	>>> somethingHappened = extensionPoints.Action()
+
+	Interested parties then register to be notified about this action:
+
+	>>> def onSomethingHappened(someArg=None):
+		... 	print(someArg)
+	...
+	>>> somethingHappened.register(onSomethingHappened)
+
+	When the action is performed, register handlers are notified:
+
+	>>> somethingHappened.notify(someArg=42)
+	"""
+
+	def notify(self, **kwargs):
+		"""Notify all registered handlers that the action has occurred.
+		@param kwargs: Arguments to pass to the handlers.
+		"""
+		for handler in self.handlers:
+			try:
+				callWithSupportedKwargs(handler, **kwargs)
+			except:
+				log.exception("Error running handler %r for %r" % (handler, self))
+
+class Filter(HandlerRegistrar):
+	"""Allows interested parties to register to modify a specific kind of data.
+	For example, this might be used to allow modification of spoken messages before they are passed to the synthesizer.
+
+	First, a Filter is created:
+
+	>>> messageFilter = extensionPoints.Filter()
+
+	Interested parties then register to filter the data:
+
+	>>> def filterMessage(message, someArg=None):
+	... 	return message + " which has been filtered."
+	...
+	>>> messageFilter.register(filterMessage)
+
+	When filtering is desired, all registered handlers are called to filter the data:
+
+	>>> messageFilter.apply("This is a message", someArg=42)
+	'This is a message which has been filtered'
+	"""
+
+	def apply(self, value, **kwargs):
+		"""Pass a value to be filtered through all registered handlers.
+		The value is passed to the first handler
+		and the return value from that handler is passed to the next handler.
+		This process continues for all handlers until the final handler.
+		The return value from the final handler is returned to the caller.
+		@param value: The value to be filtered.
+		@param kwargs: Arguments to pass to the handlers.
+		@return: The filtered value.
+		"""
+		for handler in self.handlers:
+			try:
+				value = callWithSupportedKwargs(handler, value, **kwargs)
+			except:
+				log.exception("Error running handler %r for %r" % (handler, self))
+		return value
diff --git a/tests/unit/test_extensionPoints.py b/tests/unit/test_extensionPoints.py
new file mode 100644
index 00000000000..ff97949ccf6
--- /dev/null
+++ b/tests/unit/test_extensionPoints.py
@@ -0,0 +1,255 @@
+#tests/unit/test_extensionPoints.py
+#A part of NonVisual Desktop Access (NVDA)
+#This file is covered by the GNU General Public License.
+#See the file COPYING for more details.
+#Copyright (C) 2017 NV Access Limited
+
+"""Unit tests for the extensionPoints module.
+"""
+
+import unittest
+import extensionPoints
+
+class ExampleClass(object):
+	def method(self):
+		return 42
+
+def exampleFunc():
+	return 3.14
+
+class TestBoundMethodWeakref(unittest.TestCase):
+
+	def onDelete(self, weak):
+		self.deleted = weak
+
+	def setUp(self):
+		self.deleted = None
+		self.instance = ExampleClass()
+		self.weak = extensionPoints.BoundMethodWeakref(self.instance.method, self.onDelete)
+
+	def test_get(self):
+		"""Test that we get the right strong reference from the weak reference.
+		"""
+		method = self.weak()
+		self.assertEqual(method, self.instance.method)
+
+	def test_onDelete(self):
+		"""Test that the deletion callback gets called.
+		"""
+		del self.instance
+		self.assertEqual(self.deleted, self.weak)
+
+	def test_isWeak(self):
+		"""Test that this is actually a weak reference;
+		i.e. that it dies when the instance dies.
+		"""
+		del self.instance
+		method = self.weak()
+		self.assertIsNone(method)
+
+class TestCallWithSupportedKwargs(unittest.TestCase):
+
+	def test_supportsNoKwargs(self):
+		called = []
+		def h():
+			called.append(True)
+		extensionPoints.callWithSupportedKwargs(h, a=1)
+		self.assertEqual(called, [True])
+
+	def test_supportsLessKwargs(self):
+		gotKwargs = {}
+		def h(a=None):
+			gotKwargs["a"] = a
+		extensionPoints.callWithSupportedKwargs(h, a=1, b=2)
+		self.assertEqual(gotKwargs, {"a": 1})
+
+	def test_supportsExtraKwargs(self):
+		gotKwargs = {}
+		def h(a=None, b=2):
+			gotKwargs["a"] = a
+			gotKwargs["b"] = b
+		extensionPoints.callWithSupportedKwargs(h, a=1)
+		self.assertEqual(gotKwargs, {"a": 1, "b": 2})
+
+	def test_supportsAllKwargs(self):
+		gotKwargs = {}
+		def h(**kwargs):
+			gotKwargs.update(kwargs)
+		extensionPoints.callWithSupportedKwargs(h, a=1)
+		self.assertEqual(gotKwargs, {"a": 1})
+
+class TestHandlerRegistrar(unittest.TestCase):
+
+	def setUp(self):
+		self.reg = extensionPoints.HandlerRegistrar()
+
+	def test_noHandlers(self):
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [])
+
+	def test_registerFunc(self):
+		self.reg.register(exampleFunc)
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [exampleFunc])
+
+	def test_registerInstanceMethod(self):
+		inst = ExampleClass()
+		self.reg.register(inst.method)
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [inst.method])
+
+	def test_unregisterFunc(self):
+		self.reg.register(exampleFunc)
+		self.reg.unregister(exampleFunc)
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [])
+
+	def test_unregisterInstanceMethod(self):
+		inst = ExampleClass()
+		self.reg.register(inst.method)
+		self.reg.unregister(inst.method)
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [])
+
+	def test_autoUnregisterFunc(self):
+		"""Test that a function gets automatically unregistered when the function dies.
+		"""
+		def tempFunc():
+			return 42
+		self.reg.register(tempFunc)
+		del tempFunc
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [])
+
+	def test_autoUnregisterInstanceMethod(self):
+		"""Test that a method gets automatically unregistered when the instance dies.
+		"""
+		inst = ExampleClass()
+		self.reg.register(inst.method)
+		del inst
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [])
+
+	def test_registerMultiple(self):
+		"""Test that registration of multiple handlers is ordered.
+		"""
+		inst3 = ExampleClass()
+		inst2 = ExampleClass()
+		inst1 = ExampleClass()
+		self.reg.register(inst1.method)
+		self.reg.register(inst2.method)
+		self.reg.register(inst3.method)
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [inst1.method, inst2.method, inst3.method])
+
+	def test_unregisterMiddle(self):
+		"""Test behaviour when unregistering a handler registered between of other handlers.
+		"""
+		inst3 = ExampleClass()
+		inst2 = ExampleClass()
+		inst1 = ExampleClass()
+		self.reg.register(inst1.method)
+		self.reg.register(inst2.method)
+		self.reg.register(inst3.method)
+		self.reg.unregister(inst2.method)
+		actual = list(self.reg.handlers)
+		self.assertEqual(actual, [inst1.method, inst3.method])
+
+class TestAction(unittest.TestCase):
+
+	def setUp(self):
+		self.action = extensionPoints.Action()
+
+	def test_noHandlers(self):
+		# We can only test that this doesn't fail.
+		self.action.notify(a=1)
+
+	def test_oneHandler(self):
+		called = []
+		def h():
+			called.append(h)
+		self.action.register(h)
+		self.action.notify()
+		self.assertEqual(called, [h])
+
+	def test_twoHandlers(self):
+		called = []
+		def h1():
+			called.append(h1)
+		def h2():
+			called.append(h2)
+		self.action.register(h1)
+		self.action.register(h2)
+		self.action.notify()
+		self.assertEqual(called, [h1, h2])
+
+	def test_kwargs(self):
+		"""Test that keyword arguments get passed to handlers.
+		"""
+		calledKwargs = {}
+		def h(**kwargs):
+			calledKwargs.update(kwargs)
+		self.action.register(h)
+		self.action.notify(a=1)
+		self.assertEqual(calledKwargs, {"a": 1})
+
+	def test_handlerException(self):
+		"""Test that a handler which raises an exception doesn't affect later handlers.
+		"""
+		called = []
+		def h1():
+			raise Exception("barf")
+		def h2():
+			called.append(h2)
+		self.action.register(h1)
+		self.action.register(h2)
+		self.action.notify()
+		self.assertEqual(called, [h2])
+
+class TestFilter(unittest.TestCase):
+
+	def setUp(self):
+		self.filter = extensionPoints.Filter()
+
+	def test_noHandlers(self):
+		# We can only test that this doesn't fail.
+		self.filter.apply("value", a=1)
+
+	def test_oneHandler(self):
+		def h(value):
+			return 1
+		self.filter.register(h)
+		filtered = self.filter.apply(0)
+		self.assertEqual(filtered, 1)
+
+	def test_twoHandlers(self):
+		def h1(value):
+			return 1
+		def h2(value):
+			return 2
+		self.filter.register(h1)
+		self.filter.register(h2)
+		filtered = self.filter.apply(0)
+		self.assertEqual(filtered, 2)
+
+	def test_kwargs(self):
+		"""Test that keyword arguments get passed to handlers.
+		"""
+		calledKwargs = {}
+		def h(value, **kwargs):
+			calledKwargs.update(kwargs)
+		self.filter.register(h)
+		self.filter.apply(0, a=1)
+		self.assertEqual(calledKwargs, {"a": 1})
+
+	def test_handlerException(self):
+		"""Test that a handler which raises an exception doesn't affect later handlers.
+		"""
+		def h1(value):
+			raise Exception("barf")
+		def h2(value):
+			return 2
+		self.filter.register(h1)
+		self.filter.register(h2)
+		filtered = self.filter.apply(0)
+		self.assertEqual(filtered, 2)

From 019dc5c86951850f634d32aa0ff3d4e02051ea51 Mon Sep 17 00:00:00 2001
From: James Teh <jamie@nvaccess.org>
Date: Thu, 10 Aug 2017 15:14:49 +1000
Subject: [PATCH 2/3] Add a config.configProfileSwitched action and have
 synthDriverHandler, braille, etc. register for it instead of explicitly
 calling each handler from the config module.

---
 source/audioDucking.py       |  4 ++--
 source/braille.py            |  2 ++
 source/brailleInput.py       |  1 +
 source/config/__init__.py    | 16 ++++++++--------
 source/synthDriverHandler.py |  3 ++-
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/source/audioDucking.py b/source/audioDucking.py
index 11aefe32b4f..b98f4afc81c 100644
--- a/source/audioDucking.py
+++ b/source/audioDucking.py
@@ -117,6 +117,7 @@ def initialize():
 		return
 	_setDuckingState(False)
 	setAudioDuckingMode(config.conf['audio']['audioDuckingMode'])
+	config.configProfileSwitched.register(handleConfigProfileSwitch)
 
 _isAudioDuckingSupported=None
 def isAudioDuckingSupported():
@@ -126,8 +127,7 @@ def isAudioDuckingSupported():
 	return _isAudioDuckingSupported
 
 def handleConfigProfileSwitch():
-	if isAudioDuckingSupported():
-		setAudioDuckingMode(config.conf['audio']['audioDuckingMode'])
+	setAudioDuckingMode(config.conf['audio']['audioDuckingMode'])
 
 class AudioDucker(object):
 	""" Create one of these objects to manage ducking of background audio. 
diff --git a/source/braille.py b/source/braille.py
index 71cf4751fb9..ed47f87cae9 100644
--- a/source/braille.py
+++ b/source/braille.py
@@ -1426,6 +1426,7 @@ def __init__(self):
 		self._cursorBlinkUp = True
 		self._cells = []
 		self._cursorBlinkTimer = None
+		config.configProfileSwitched.register(self.handleConfigProfileSwitch)
 
 	def terminate(self):
 		if self._messageCallLater:
@@ -1434,6 +1435,7 @@ def terminate(self):
 		if self._cursorBlinkTimer:
 			self._cursorBlinkTimer.Stop()
 			self._cursorBlinkTimer = None
+		config.configProfileSwitched.unregister(self.handleConfigProfileSwitch)
 		if self.display:
 			self.display.terminate()
 			self.display = None
diff --git a/source/brailleInput.py b/source/brailleInput.py
index 699ccebf1fc..2722d53c3e0 100644
--- a/source/brailleInput.py
+++ b/source/brailleInput.py
@@ -88,6 +88,7 @@ def __init__(self):
 		self.untranslatedCursorPos = 0
 		#: The time at which uncontracted characters were sent to the system.
 		self._uncontSentTime = None
+		config.configProfileSwitched.register(self.handleConfigProfileSwitch)
 
 	@property
 	def table(self):
diff --git a/source/config/__init__.py b/source/config/__init__.py
index 1832ad6e677..b897da2a905 100644
--- a/source/config/__init__.py
+++ b/source/config/__init__.py
@@ -27,6 +27,7 @@
 import easeOfAccess
 from fileUtils import FaultTolerantFile
 import winKernel
+import extensionPoints
 import profileUpgrader
 from .configSpec import confspec
 
@@ -34,6 +35,12 @@
 #: @type: ConfigObj
 conf = None
 
+#: Notifies when the configuration profile is switched.
+#: This allows components to apply changes required by the new configuration.
+#: For example, braille switches braille displays if necessary.
+#: Handlers are called with no arguments.
+configProfileSwitched = extensionPoints.Action()
+
 def initialize():
 	global conf
 	conf = ConfigManager()
@@ -333,14 +340,7 @@ def _handleProfileSwitch(self):
 		if init:
 			# We're still initialising, so don't notify anyone about this change.
 			return
-		import synthDriverHandler
-		synthDriverHandler.handleConfigProfileSwitch()
-		import brailleInput
-		brailleInput.handler.handleConfigProfileSwitch()
-		import braille
-		braille.handler.handleConfigProfileSwitch()
-		import audioDucking
-		audioDucking.handleConfigProfileSwitch()
+		configProfileSwitched.notify()
 
 	def _initBaseConf(self, factoryDefaults=False):
 		fn = os.path.join(globalVars.appArgs.configPath, "nvda.ini")
diff --git a/source/synthDriverHandler.py b/source/synthDriverHandler.py
index b8baa044e3d..8347b78b7a9 100644
--- a/source/synthDriverHandler.py
+++ b/source/synthDriverHandler.py
@@ -3,7 +3,7 @@
 #A part of NonVisual Desktop Access (NVDA)
 #This file is covered by the GNU General Public License.
 #See the file COPYING for more details.
-#Copyright (C) 2006-2015 NV Access Limited, Peter Vágner, Aleksey Sadovoy, Joseph Lee
+#Copyright (C) 2006-2017 NV Access Limited, Peter Vágner, Aleksey Sadovoy, Joseph Lee
 
 from copy import deepcopy
 import os
@@ -22,6 +22,7 @@
 
 def initialize():
 	config.addConfigDirsToPythonPackagePath(synthDrivers)
+	config.configProfileSwitched.register(handleConfigProfileSwitch)
 
 def changeVoice(synth, voice):
 	# This function can be called with no voice if the synth doesn't support the voice setting (only has one voice).

From 26df87de1208b65f4a76e944aad3955e7c315764 Mon Sep 17 00:00:00 2001
From: James Teh <jamie@nvaccess.org>
Date: Tue, 15 Aug 2017 22:22:21 +1000
Subject: [PATCH 3/3] Review actions for unit tests: Rename h, h1, etc. to
 handler, handler1, etc. Add tests for positional args for
 callWithSupportedKwargs.

---
 tests/unit/test_extensionPoints.py | 99 ++++++++++++++++++------------
 1 file changed, 60 insertions(+), 39 deletions(-)

diff --git a/tests/unit/test_extensionPoints.py b/tests/unit/test_extensionPoints.py
index ff97949ccf6..5d003afb024 100644
--- a/tests/unit/test_extensionPoints.py
+++ b/tests/unit/test_extensionPoints.py
@@ -51,33 +51,54 @@ class TestCallWithSupportedKwargs(unittest.TestCase):
 
 	def test_supportsNoKwargs(self):
 		called = []
-		def h():
+		def handler():
 			called.append(True)
-		extensionPoints.callWithSupportedKwargs(h, a=1)
+		extensionPoints.callWithSupportedKwargs(handler, a=1)
 		self.assertEqual(called, [True])
 
 	def test_supportsLessKwargs(self):
 		gotKwargs = {}
-		def h(a=None):
+		def handler(a=None):
 			gotKwargs["a"] = a
-		extensionPoints.callWithSupportedKwargs(h, a=1, b=2)
+		extensionPoints.callWithSupportedKwargs(handler, a=1, b=2)
 		self.assertEqual(gotKwargs, {"a": 1})
 
 	def test_supportsExtraKwargs(self):
 		gotKwargs = {}
-		def h(a=None, b=2):
+		def handler(a=None, b=2):
 			gotKwargs["a"] = a
 			gotKwargs["b"] = b
-		extensionPoints.callWithSupportedKwargs(h, a=1)
+		extensionPoints.callWithSupportedKwargs(handler, a=1)
 		self.assertEqual(gotKwargs, {"a": 1, "b": 2})
 
 	def test_supportsAllKwargs(self):
 		gotKwargs = {}
-		def h(**kwargs):
+		def handler(**kwargs):
 			gotKwargs.update(kwargs)
-		extensionPoints.callWithSupportedKwargs(h, a=1)
+		extensionPoints.callWithSupportedKwargs(handler, a=1)
 		self.assertEqual(gotKwargs, {"a": 1})
 
+	def test_positionalsPassedWhenSupportsNoKwargs(self):
+		"""Test that positional arguments are passed untouched.
+		"""
+		gotArgs = []
+		def handler(a, b):
+			gotArgs.append(a)
+			gotArgs.append(b)
+		extensionPoints.callWithSupportedKwargs(handler, 1, 2)
+		self.assertEqual(gotArgs, [1, 2])
+
+	def test_positionalsPassedWhenSupportsAllKwargs(self):
+		"""Test that positional arguments are passed untouched when the function has **kwargs,
+		since **kwargs is a special case early return in the code.
+		"""
+		gotArgs = []
+		def handler(a, b, **kwargs):
+			gotArgs.append(a)
+			gotArgs.append(b)
+		extensionPoints.callWithSupportedKwargs(handler, 1, 2, c=3)
+		self.assertEqual(gotArgs, [1, 2])
+
 class TestHandlerRegistrar(unittest.TestCase):
 
 	def setUp(self):
@@ -166,30 +187,30 @@ def test_noHandlers(self):
 
 	def test_oneHandler(self):
 		called = []
-		def h():
-			called.append(h)
-		self.action.register(h)
+		def handler():
+			called.append(handler)
+		self.action.register(handler)
 		self.action.notify()
-		self.assertEqual(called, [h])
+		self.assertEqual(called, [handler])
 
 	def test_twoHandlers(self):
 		called = []
-		def h1():
-			called.append(h1)
-		def h2():
-			called.append(h2)
-		self.action.register(h1)
-		self.action.register(h2)
+		def handler1():
+			called.append(handler1)
+		def handler2():
+			called.append(handler2)
+		self.action.register(handler1)
+		self.action.register(handler2)
 		self.action.notify()
-		self.assertEqual(called, [h1, h2])
+		self.assertEqual(called, [handler1, handler2])
 
 	def test_kwargs(self):
 		"""Test that keyword arguments get passed to handlers.
 		"""
 		calledKwargs = {}
-		def h(**kwargs):
+		def handler(**kwargs):
 			calledKwargs.update(kwargs)
-		self.action.register(h)
+		self.action.register(handler)
 		self.action.notify(a=1)
 		self.assertEqual(calledKwargs, {"a": 1})
 
@@ -197,14 +218,14 @@ def test_handlerException(self):
 		"""Test that a handler which raises an exception doesn't affect later handlers.
 		"""
 		called = []
-		def h1():
+		def handler1():
 			raise Exception("barf")
-		def h2():
-			called.append(h2)
-		self.action.register(h1)
-		self.action.register(h2)
+		def handler2():
+			called.append(handler2)
+		self.action.register(handler1)
+		self.action.register(handler2)
 		self.action.notify()
-		self.assertEqual(called, [h2])
+		self.assertEqual(called, [handler2])
 
 class TestFilter(unittest.TestCase):
 
@@ -216,19 +237,19 @@ def test_noHandlers(self):
 		self.filter.apply("value", a=1)
 
 	def test_oneHandler(self):
-		def h(value):
+		def handler(value):
 			return 1
-		self.filter.register(h)
+		self.filter.register(handler)
 		filtered = self.filter.apply(0)
 		self.assertEqual(filtered, 1)
 
 	def test_twoHandlers(self):
-		def h1(value):
+		def handler1(value):
 			return 1
-		def h2(value):
+		def handler2(value):
 			return 2
-		self.filter.register(h1)
-		self.filter.register(h2)
+		self.filter.register(handler1)
+		self.filter.register(handler2)
 		filtered = self.filter.apply(0)
 		self.assertEqual(filtered, 2)
 
@@ -236,20 +257,20 @@ def test_kwargs(self):
 		"""Test that keyword arguments get passed to handlers.
 		"""
 		calledKwargs = {}
-		def h(value, **kwargs):
+		def handler(value, **kwargs):
 			calledKwargs.update(kwargs)
-		self.filter.register(h)
+		self.filter.register(handler)
 		self.filter.apply(0, a=1)
 		self.assertEqual(calledKwargs, {"a": 1})
 
 	def test_handlerException(self):
 		"""Test that a handler which raises an exception doesn't affect later handlers.
 		"""
-		def h1(value):
+		def handler1(value):
 			raise Exception("barf")
-		def h2(value):
+		def handler2(value):
 			return 2
-		self.filter.register(h1)
-		self.filter.register(h2)
+		self.filter.register(handler1)
+		self.filter.register(handler2)
 		filtered = self.filter.apply(0)
 		self.assertEqual(filtered, 2)