Changeset 51024 in webkit


Ignore:
Timestamp:
Nov 16, 2009 3:04:09 AM (14 years ago)
Author:
eric@webkit.org
Message:

2009-11-16 Eric Seidel <eric@webkit.org>

Reviewed by Adam Barth.

WorkQueue is the only place that should know about special exit codes
https://bugs.webkit.org/show_bug.cgi?id=31534

Move LandPatchesFromBugs.handled_error to WorkQueue.exit_after_handled_error
and add tests for handling exit codes.
I also cleaned up workqueue_unittest.py more.

  • Scripts/bugzilla-tool:
  • Scripts/modules/workqueue.py:
  • Scripts/modules/workqueue_unittest.py:
Location:
trunk/WebKitTools
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r51023 r51024  
     12009-11-16  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        WorkQueue is the only place that should know about special exit codes
     6        https://bugs.webkit.org/show_bug.cgi?id=31534
     7
     8        Move LandPatchesFromBugs.handled_error to WorkQueue.exit_after_handled_error
     9        and add tests for handling exit codes.
     10        I also cleaned up workqueue_unittest.py more.
     11
     12        * Scripts/bugzilla-tool:
     13        * Scripts/modules/workqueue.py:
     14        * Scripts/modules/workqueue_unittest.py:
     15
    1162009-11-16  Eric Seidel  <eric@webkit.org>
    217
  • trunk/WebKitTools/Scripts/bugzilla-tool

    r51022 r51024  
    377377        Command.__init__(self, 'Lands all patches on a bug optionally testing them first', 'BUGID', options=options)
    378378
    379     @staticmethod
    380     def handled_error(error):
    381         log(error)
    382         exit(2) # Exit 2 insted of 1 to indicate to the commit-queue to indicate we handled the error, and that the queue should keep looping.
    383 
    384379    @classmethod
    385380    def _close_bug_if_no_active_patches(cls, bugs, bug_id):
     
    410405            log("Commit failed because the checkout is out of date.  Please update and try again.")
    411406            log("You can pass --no-build to skip building/testing after update if you believe the new commits did not affect the results.")
    412             cls.handled_error(e)
     407            WorkQueue.exit_after_handled_error(e)
    413408        except ScriptError, e:
    414409            # Mark the patch as commit-queue- and comment in the bug.
    415410            tool.bugs.reject_patch_from_commit_queue(patch['id'], e.message_with_output())
    416             cls.handled_error(e)
     411            WorkQueue.exit_after_handled_error(e)
    417412
    418413    @staticmethod
  • trunk/WebKitTools/Scripts/modules/workqueue.py

    r51018 r51024  
    7676    sleep_duration_text = "5 mins"
    7777    seconds_to_sleep = 300
     78    handled_error_code = 2
     79
     80    # Child processes exit with a special code to the parent queue process can detect the error was handled.
     81    @classmethod
     82    def exit_after_handled_error(cls, error):
     83        log(error)
     84        exit(cls.handled_error_code)
    7885
    7986    def run(self):
     
    103110                self._delegate.process_work_item(work_item)
    104111            except ScriptError, e:
    105                 # exit(2) is a special exit code we use to indicate that the error was already
    106                 # handled by and we should keep looping anyway.
    107                 if e.exit_code == 2:
     112                # Use a special exit code to indicate that the error was already
     113                # handled in the child process and we should just keep looping.
     114                if e.exit_code == self.handled_error_code:
    108115                    continue
    109116                message = "Unexpected failure when landing patch!  Please file a bug against bugzilla-tool.\n%s" % e.message_with_output()
  • trunk/WebKitTools/Scripts/modules/workqueue_unittest.py

    r51023 r51024  
    3333import unittest
    3434
     35from modules.scm import ScriptError
    3536from modules.workqueue import WorkQueue, WorkQueueDelegate
    3637
    3738class LoggingDelegate(WorkQueueDelegate):
    3839    def __init__(self, test):
    39         self.test = test
    40         self.callbacks = []
    41         self.run_before = False
     40        self._test = test
     41        self._callbacks = []
     42        self._run_before = False
     43
     44    expected_callbacks = [
     45        'queue_log_path',
     46        'status_host',
     47        'begin_work_queue',
     48        'should_continue_work_queue',
     49        'next_work_item',
     50        'should_proceed_with_work_item',
     51        'work_logs_directory',
     52        'process_work_item',
     53        'should_continue_work_queue'
     54    ]
     55
     56    def record(self, method_name):
     57        self._callbacks.append(method_name)
    4258
    4359    def queue_log_path(self):
    44         self.callbacks.append("queue_log_path")
    45         return os.path.join(self.test.temp_dir, "queue_log_path")
     60        self.record("queue_log_path")
     61        return os.path.join(self._test.temp_dir, "queue_log_path")
    4662
    4763    def work_logs_directory(self):
    48         self.callbacks.append("work_logs_directory")
    49         return os.path.join(self.test.temp_dir, "work_log_path")
     64        self.record("work_logs_directory")
     65        return os.path.join(self._test.temp_dir, "work_log_path")
    5066
    5167    def status_host(self):
    52         self.callbacks.append("status_host")
     68        self.record("status_host")
    5369        return None
    5470
    5571    def begin_work_queue(self):
    56         self.callbacks.append("begin_work_queue")
     72        self.record("begin_work_queue")
    5773
    5874    def should_continue_work_queue(self):
    59         self.callbacks.append("should_continue_work_queue")
    60         if not self.run_before:
    61             self.run_before = True
     75        self.record("should_continue_work_queue")
     76        if not self._run_before:
     77            self._run_before = True
    6278            return True
    6379        return False
    6480
    6581    def next_work_item(self):
    66         self.callbacks.append("next_work_item")
     82        self.record("next_work_item")
    6783        return "work_item"
    6884
    6985    def should_proceed_with_work_item(self, work_item):
    70         self.callbacks.append("should_proceed_with_work_item")
    71         self.test.assertEquals(work_item, "work_item")
     86        self.record("should_proceed_with_work_item")
     87        self._test.assertEquals(work_item, "work_item")
    7288        return (True, "waiting_message", 42)
    7389
    7490    def process_work_item(self, work_item):
    75         self.callbacks.append("process_work_item")
    76         self.test.assertEquals(work_item, "work_item")
     91        self.record("process_work_item")
     92        self._test.assertEquals(work_item, "work_item")
    7793
    7894    def handle_unexpected_error(self, work_item, message):
    79         self.callbacks.append("handle_unexpected_error")
    80         self.test.assertEquals(work_item, "work_item")
     95        self.record("handle_unexpected_error")
     96        self._test.assertEquals(work_item, "work_item")
     97
     98class ThrowErrorDelegate(LoggingDelegate):
     99    def __init__(self, test, error_code):
     100        LoggingDelegate.__init__(self, test)
     101        self.error_code = error_code
     102
     103    def process_work_item(self, work_item):
     104        self.record("process_work_item")
     105        raise ScriptError(exit_code=self.error_code)
    81106
    82107class WorkQueueTest(unittest.TestCase):
     
    85110        work_queue = WorkQueue(delegate)
    86111        work_queue.run()
    87         self.assertEquals(delegate.callbacks, [
    88             'queue_log_path',
    89             'status_host',
    90             'begin_work_queue',
    91             'should_continue_work_queue',
    92             'next_work_item',
    93             'should_proceed_with_work_item',
    94             'work_logs_directory',
    95             'process_work_item',
    96             'should_continue_work_queue'])
     112        self.assertEquals(delegate._callbacks, LoggingDelegate.expected_callbacks)
    97113        self.assertTrue(os.path.exists(delegate.queue_log_path()))
    98114        self.assertTrue(os.path.exists(os.path.join(delegate.work_logs_directory(), "42.log")))
     115
     116    def test_unexpected_error(self):
     117        delegate = ThrowErrorDelegate(self, 3)
     118        work_queue = WorkQueue(delegate)
     119        work_queue.run()
     120        expected_callbacks = LoggingDelegate.expected_callbacks[:]
     121        work_item_index = expected_callbacks.index('process_work_item')
     122        # The unexpected error should be handled right after process_work_item starts
     123        # but before any other callback.  Otherwise callbacks should be normal.
     124        expected_callbacks.insert(work_item_index + 1, 'handle_unexpected_error')
     125        self.assertEquals(delegate._callbacks, expected_callbacks)
     126
     127    def test_handled_error(self):
     128        delegate = ThrowErrorDelegate(self, WorkQueue.handled_error_code)
     129        work_queue = WorkQueue(delegate)
     130        work_queue.run()
     131        self.assertEquals(delegate._callbacks, LoggingDelegate.expected_callbacks)
    99132
    100133    def setUp(self):
Note: See TracChangeset for help on using the changeset viewer.