Tasks now have a may_run_user_code() method, which is used for tasks which

can potentially run unknown code or throw unknown exceptions.  This is used
by the Event.run() method to prevent running unknown logic when the event
as a whole is already known to have failed.

2 files changed, 159 insertions, 31 deletions

diff --git a/mocker.py b/mocker.py
index a453ab5..eee6c64 100644
--- a/mocker.py
+++ b/mocker.py
@@ -381,6 +381,9 @@ class MockerTestCase(unittest.TestCase):
     assertMethodsMatch = failUnlessMethodsMatch
     assertRaises = failUnlessRaises
 
+    # XXX Add assertIsInstance() and assertIsSubclass, and
+    #     extend assertRaises() with new logic from unittest.
+
     # The following are missing in Python < 2.4.
     assertTrue = unittest.TestCase.failUnless
     assertFalse = unittest.TestCase.failIf
@@ -754,7 +757,7 @@ class MockerBase(object):
     def act(self, path):
         """This is called by mock objects whenever something happens to them.
 
-        This method is part of the implementation between the mocker
+        This method is part of the interface between the mocker
         and mock objects.
         """
         if self._recording:
@@ -873,7 +876,7 @@ class MockerBase(object):
         for task in event.get_tasks():
             if isinstance(task, RunCounter):
                 event.remove_task(task)
-        event.add_task(RunCounter(min, max))
+        event.prepend_task(RunCounter(min, max))
 
     def is_ordering(self):
         """Return true if all events are being ordered.
@@ -1559,13 +1562,25 @@ class Event(object):
         self._has_run = False
 
     def add_task(self, task):
-        """Add a new task to this taks."""
+        """Add a new task to this task."""
         self._tasks.append(task)
         return task
 
+    def prepend_task(self, task):
+        """Add a task at the front of the list."""
+        self._tasks.insert(0, task)
+        return task
+
     def remove_task(self, task):
         self._tasks.remove(task)
 
+    def replace_task(self, old_task, new_task):
+        """Replace old_task with new_task, in the same position."""
+        for i in range(len(self._tasks)):
+            if self._tasks[i] is old_task:
+                self._tasks[i] = new_task
+        return new_task
+
     def get_tasks(self):
         return self._tasks[:]
 
@@ -1606,16 +1621,21 @@ class Event(object):
         result = None
         errors = []
         for task in self._tasks:
-            try:
-                task_result = task.run(path)
-            except AssertionError, e:
-                error = str(e)
-                if not error:
-                    raise RuntimeError("Empty error message from %r" % task)
-                errors.append(error)
-            else:
-                if task_result is not None:
-                    result = task_result
+            if not errors or not task.may_run_user_code():
+                try:
+                    task_result = task.run(path)
+                except AssertionError, e:
+                    error = str(e)
+                    if not error:
+                        raise RuntimeError("Empty error message from %r" % task)
+                    errors.append(error)
+                else:
+                    # XXX That's actually a bit weird.  What if a call() really
+                    # returned None?  This would improperly change the semantic
+                    # of this process without any good reason. Test that with two
+                    # call()s in sequence.
+                    if task_result is not None:
+                        result = task_result
         if errors:
             message = [str(self.path)]
             if str(path) != message[0]:
@@ -1700,6 +1720,15 @@ class Task(object):
         """Return false if running this task would certainly raise an error."""
         return True
 
+    def may_run_user_code(self):
+        """Return true if there's a chance this task may run custom code.
+
+        Whenever errors are detected, running user code should be avoided,
+        because the situation is already known to be incorrect, and any
+        errors in the user code are side effects rather than the cause.
+        """
+        return False
+
     def run(self, path):
         """Perform the task item, considering that the given action happened.
         """
@@ -1796,7 +1825,9 @@ class ImplicitRunCounter(RunCounter):
 def run_counter_recorder(mocker, event):
     """Any event may be repeated once, unless disabled by default."""
     if event.path.root_mock.__mocker_count__:
-        event.add_task(ImplicitRunCounter(1))
+        # Rather than appending the task, we prepend it so that the
+        # issue is raised before any other side-effects happen.
+        event.prepend_task(ImplicitRunCounter(1))
 
 Mocker.add_recorder(run_counter_recorder)
 
@@ -1852,6 +1883,9 @@ class FunctionRunner(Task):
         self._func = func
         self._with_root_object = with_root_object
 
+    def may_run_user_code(self):
+        return True
+
     def run(self, path):
         action = path.actions[-1]
         if self._with_root_object:
@@ -2149,7 +2183,8 @@ class PatchedMethod(object):
         # At least with __getattribute__, Python seems to use *both* the
         # descriptor API and also call the class attribute directly.  It
         # looks like an interpreter bug, or at least an undocumented
-        # inconsistency.
+        # inconsistency.  Coverage tests may show this uncovered, because
+        # it depends on the Python version.
         return self.__get__(obj)(*args, **kwargs)
 
 
diff --git a/test.py b/test.py
index b6caf65..c273ed8 100755
--- a/test.py
+++ b/test.py
@@ -123,6 +123,22 @@ class IntegrationTest(TestCase):
         self.mocker.throw(ValueError)
         self.mocker.replay()
         self.assertRaises(ValueError, obj.x)
+        self.assertRaises(AssertionError, obj.x)
+
+    def test_throw_with_count(self):
+        """
+        This ensures that the count problem is reported correctly even when
+        using an explicit count.  If the ordering of problems isn't handled
+        properly, the thrown error might surface before the count one.
+        """
+        obj = self.mocker.mock()
+        obj.x()
+        self.mocker.throw(ValueError)
+        self.mocker.count(2)
+        self.mocker.replay()
+        self.assertRaises(ValueError, obj.x)
+        self.assertRaises(ValueError, obj.x)
+        self.assertRaises(AssertionError, obj.x)
 
     def test_call(self):
         calls = []
@@ -1650,21 +1666,41 @@ class MockerTest(TestCase):
                           lambda *args: None, with_object=True)
 
     def test_count(self):
+        class MyTask(Task):
+            pass
         event1 = self.mocker.add_event(Event())
         event2 = self.mocker.add_event(Event())
+        event2.add_task(MyTask())
+        event2.add_task(ImplicitRunCounter(1))
         event2.add_task(ImplicitRunCounter(1))
         self.mocker.count(2, 3)
         self.assertEquals(len(event1.get_tasks()), 0)
-        (task,) = event2.get_tasks()
-        self.assertEquals(type(task), RunCounter)
-        self.assertEquals(task.min, 2)
-        self.assertEquals(task.max, 3)
+        (task1, task2,) = event2.get_tasks()
+        self.assertEquals(type(task1), RunCounter)
+        self.assertEquals(type(task2), MyTask)
+        self.assertEquals(task1.min, 2)
+        self.assertEquals(task1.max, 3)
         self.mocker.count(4)
         self.assertEquals(len(event1.get_tasks()), 0)
-        (task,) = event2.get_tasks()
-        self.assertEquals(type(task), RunCounter)
-        self.assertEquals(task.min, 4)
-        self.assertEquals(task.max, 4)
+        (task1, task2) = event2.get_tasks()
+        self.assertEquals(type(task1), RunCounter)
+        self.assertEquals(type(task2), MyTask)
+        self.assertEquals(task1.min, 4)
+        self.assertEquals(task1.max, 4)
+
+    def test_count_without_implicit_counter(self):
+        class MyTask(Task):
+            pass
+        event1 = self.mocker.add_event(Event())
+        event2 = self.mocker.add_event(Event())
+        event2.add_task(MyTask())
+        self.mocker.count(2, 3)
+        self.assertEquals(len(event1.get_tasks()), 0)
+        (task1, task2,) = event2.get_tasks()
+        self.assertEquals(type(task1), RunCounter)
+        self.assertEquals(type(task2), MyTask)
+        self.assertEquals(task1.min, 2)
+        self.assertEquals(task1.max, 3)
 
     def test_order(self):
         mock1 = self.mocker.mock()
@@ -2828,10 +2864,19 @@ class EventTest(TestCase):
         self.assertEquals(event.path, path)
 
     def test_add_and_get_tasks(self):
-        task1 = self.event.add_task(Task())
-        task2 = self.event.add_task(Task())
+        task1 = Task()
+        task2 = Task()
+        self.assertEqual(self.event.add_task(task1), task1)
+        self.assertEqual(self.event.add_task(task2), task2)
         self.assertEquals(self.event.get_tasks(), [task1, task2])
 
+    def test_prepend_tasks(self):
+        task1 = Task()
+        task2 = Task()
+        self.assertEqual(self.event.prepend_task(task1), task1)
+        self.assertEqual(self.event.prepend_task(task2), task2)
+        self.assertEquals(self.event.get_tasks(), [task2, task1])
+
     def test_remove_task(self):
         task1 = self.event.add_task(Task())
         task2 = self.event.add_task(Task())
@@ -2839,6 +2884,15 @@ class EventTest(TestCase):
         self.event.remove_task(task2)
         self.assertEquals(self.event.get_tasks(), [task1, task3])
 
+    def test_replace_task(self):
+        task1 = self.event.add_task(Task())
+        task2 = self.event.add_task(Task())
+        task3 = self.event.add_task(Task())
+        task4 = Task()
+        task5 = self.event.replace_task(task2, task4)
+        self.assertEquals(self.event.get_tasks(), [task1, task4, task3])
+        self.assertTrue(task4 is task5)
+
     def test_default_matches(self):
         self.assertEquals(self.event.matches(None), False)
 
@@ -2892,7 +2946,7 @@ class EventTest(TestCase):
         self.assertEquals(calls, [42, 42, 42])
 
     def test_run_errors(self):
-        class MyTask(object):
+        class MyTask(Task):
             def __init__(self, id, failed):
                 self.id = id
                 self.failed = failed
@@ -2916,7 +2970,7 @@ class EventTest(TestCase):
 
     def test_run_errors_with_different_path_representation(self):
         """When the path representation isn't the same it's shown up."""
-        class MyTask(object):
+        class MyTask(Task):
             def __init__(self, id, failed):
                 self.id = id
                 self.failed = failed
@@ -2946,6 +3000,34 @@ class EventTest(TestCase):
         self.event.add_task(MyTask())
         self.assertRaises(RuntimeError, self.event.run, 42)
 
+    def test_may_run_user_code(self):
+        """
+        Tasks that may run user code should be run if a prior failure has 
+        already been detected.
+        """
+        class MyTask(Task):
+            def __init__(self, id, user_code):
+                self.id = id
+                self.user_code = user_code
+            def may_run_user_code(self):
+                return self.user_code
+            def run(self, path):
+                raise AssertionError("%d failed" % self.id)
+        event = Event("i.am.a.path")
+        event.add_task(MyTask(1, False))
+        event.add_task(MyTask(2, True))
+        event.add_task(MyTask(3, False))
+
+        try:
+            event.run("i.am.a.path")
+        except AssertionError, e:
+            message = os.linesep.join(["i.am.a.path",
+                                       "- 1 failed",
+                                       "- 3 failed"])
+            self.assertEquals(str(e), message)
+        else:
+            self.fail("AssertionError not raised")
+
     def test_has_run(self):
         self.assertFalse(self.event.has_run())
         self.event.run(None)
@@ -3062,6 +3144,9 @@ class TaskTest(TestCase):
     def test_default_may_run(self):
         self.assertEquals(self.task.may_run(None), True)
 
+    def test_default_may_run_user_code(self):
+        self.assertEquals(self.task.may_run_user_code(), False)
+
     def test_default_run(self):
         self.assertEquals(self.task.run(None), None)
 
@@ -3211,11 +3296,15 @@ class RunCounterTest(TestCase):
         self.assertRaises(AssertionError, task.run, self.path)
 
     def test_recorder(self):
+        class MyTask(Task):
+            pass
+        self.event.add_task(MyTask())
         run_counter_recorder(self.mocker, self.event)
-        (task,) = self.event.get_tasks()
-        self.assertEquals(type(task), ImplicitRunCounter)
-        self.assertTrue(task.min == 1)
-        self.assertTrue(task.max == 1)
+        (task1, task2) = self.event.get_tasks()
+        self.assertEquals(type(task1), ImplicitRunCounter)
+        self.assertEquals(type(task2), MyTask)
+        self.assertTrue(task1.min == 1)
+        self.assertTrue(task1.max == 1)
 
     def test_recorder_wont_record_when_count_is_false(self):
         self.mock.__mocker_count__ = False
@@ -3374,6 +3463,10 @@ class FunctionRunnerTest(TestCase):
     def test_is_task(self):
         self.assertTrue(isinstance(FunctionRunner(None), Task))
 
+    def test_may_run_user_code(self):
+        task = FunctionRunner(None)
+        self.assertEquals(task.may_run_user_code(), True)
+
     def test_run(self):
         task = FunctionRunner(lambda *args, **kwargs: repr((args, kwargs)))
         result = task.run(self.path)