Bug 1497052 [wpt PR 13408] - Improve warnings of command left in queue in TestRunnerManager cleanup, a=testonly
authorPhilip Jägenstedt <philip@foolip.org>
Thu, 11 Oct 2018 09:31:37 +0000
changeset 489260 a0482419c5d35ee353c0ffb288c30f8ba6ee6423
parent 489259 ddb8b080f0b435ea622b4a01bee5cc0c6e12058d
child 489261 97dedc2880cb23bf939a685ea1cfd41cb1a32806
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerstestonly
bugs1497052, 13408, 13407
milestone64.0a1
Bug 1497052 [wpt PR 13408] - Improve warnings of command left in queue in TestRunnerManager cleanup, a=testonly Automatic update from web-platform-testsImprove warnings of command left in queue in TestRunnerManager cleanup Part of https://github.com/web-platform-tests/wpt/issues/13407. -- Rename TestManager to TestRunnerManager in documentation/logging (There is no TestManager class.) -- Normalize messages around TestRunner/Manager more -- Fix Thread-TestRunner misnomer -- Drop the Thread-/Process- prefixes for thread/process names -- wpt-commits: a772507179527a9b2837b5f6798b41a81f9e0969, 2f50699d3942b302bd64e2ec6ce0fc9e25f62dd5, 5e4669247907398d8017e7b6539bab1041c3e4f2, d30010ae297f1057e168f360f292bb809f288bec, af795eb39d35dfb1f50c36618732995c500ef80b wpt-pr: 13408
testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py
@@ -47,17 +47,17 @@ class TestRunner(object):
 
         This class delegates the job of actually running a test to the executor
         that is passed in.
 
         :param logger: Structured logger
         :param command_queue: subprocess.Queue used to send commands to the
                               process
         :param result_queue: subprocess.Queue used to send results to the
-                             parent TestManager process
+                             parent TestRunnerManager process
         :param executor: TestExecutor object that will actually run a test.
         """
         self.command_queue = command_queue
         self.result_queue = result_queue
 
         self.executor = executor
         self.name = current_process().name
         self.logger = logger
@@ -299,36 +299,36 @@ class TestRunnerManager(threading.Thread
 
         self.manager_number = next_manager_number()
 
         self.command_queue = Queue()
         self.remote_queue = Queue()
 
         self.test_runner_proc = None
 
-        threading.Thread.__init__(self, name="Thread-TestrunnerManager-%i" % self.manager_number)
+        threading.Thread.__init__(self, name="TestRunnerManager-%i" % self.manager_number)
         # This is started in the actual new thread
         self.logger = None
 
         self.test_count = 0
         self.unexpected_count = 0
 
         # This may not really be what we want
         self.daemon = True
 
         self.max_restarts = 5
 
         self.browser = None
 
         self.capture_stdio = capture_stdio
 
     def run(self):
-        """Main loop for the TestManager.
+        """Main loop for the TestRunnerManager.
 
-        TestManagers generally receive commands from their
+        TestRunnerManagers generally receive commands from their
         TestRunner updating them on the status of a test. They
         may also have a stop flag set by the main thread indicating
         that the manager should shut down the next time the event loop
         spins."""
         self.logger = structuredlog.StructuredLogger(self.suite_name)
         with self.browser_cls(self.logger, **self.browser_kwargs) as browser:
             self.browser = BrowserManager(self.logger,
                                           browser,
@@ -485,17 +485,17 @@ class TestRunnerManager(threading.Thread
                 self.executor_cls,
                 self.executor_kwargs,
                 executor_browser_cls,
                 executor_browser_kwargs,
                 self.capture_stdio,
                 self.child_stop_flag)
         self.test_runner_proc = Process(target=start_runner,
                                         args=args,
-                                        name="Thread-TestRunner-%i" % self.manager_number)
+                                        name="TestRunner-%i" % self.manager_number)
         self.test_runner_proc.start()
         self.logger.debug("Test runner started")
         # Now we wait for either an init_succeeded event or an init_failed event
 
     def init_succeeded(self):
         assert isinstance(self.state, RunnerManagerState.initializing)
         self.browser.after_init()
         return RunnerManagerState.running(self.state.test,
@@ -669,17 +669,17 @@ class TestRunnerManager(threading.Thread
             self.send_message("stop")
         try:
             self.browser.stop(force=force)
             self.ensure_runner_stopped()
         finally:
             self.cleanup()
 
     def teardown(self):
-        self.logger.debug("teardown in testrunnermanager")
+        self.logger.debug("TestRunnerManager teardown")
         self.test_runner_proc = None
         self.command_queue.close()
         self.remote_queue.close()
         self.command_queue = None
         self.remote_queue = None
 
     def ensure_runner_stopped(self):
         self.logger.debug("ensure_runner_stopped")
@@ -690,43 +690,43 @@ class TestRunnerManager(threading.Thread
         self.test_runner_proc.join(10)
         self.logger.debug("After join")
         if self.test_runner_proc.is_alive():
             # This might leak a file handle from the queue
             self.logger.warning("Forcibly terminating runner process")
             self.test_runner_proc.terminate()
             self.test_runner_proc.join(10)
         else:
-            self.logger.debug("Testrunner exited with code %i" % self.test_runner_proc.exitcode)
+            self.logger.debug("Runner process exited with code %i" % self.test_runner_proc.exitcode)
 
     def runner_teardown(self):
         self.ensure_runner_stopped()
         return RunnerManagerState.stop()
 
     def send_message(self, command, *args):
         self.remote_queue.put((command, args))
 
     def cleanup(self):
-        self.logger.debug("TestManager cleanup")
+        self.logger.debug("TestRunnerManager cleanup")
         if self.browser:
             self.browser.cleanup()
         while True:
             try:
                 cmd, data = self.command_queue.get_nowait()
             except Empty:
                 break
             else:
                 if cmd == "log":
                     self.log(*data)
                 else:
-                    self.logger.warning("%r: %r" % (cmd, data))
+                    self.logger.warning("Command left in command_queue during cleanup: %r, %r" % (cmd, data))
         while True:
             try:
                 cmd, data = self.remote_queue.get_nowait()
-                self.logger.warning("%r: %r" % (cmd, data))
+                self.logger.warning("Command left in remote_queue during cleanup: %r, %r" % (cmd, data))
             except Empty:
                 break
 
 
 def make_test_queue(tests, test_source_cls, **test_source_kwargs):
     queue = test_source_cls.make_queue(tests, **test_source_kwargs)
 
     # There is a race condition that means sometimes we continue
@@ -742,17 +742,17 @@ class ManagerGroup(object):
                  browser_cls, browser_kwargs,
                  executor_cls, executor_kwargs,
                  rerun=1,
                  pause_after_test=False,
                  pause_on_unexpected=False,
                  restart_on_unexpected=True,
                  debug_info=None,
                  capture_stdio=True):
-        """Main thread object that owns all the TestManager threads."""
+        """Main thread object that owns all the TestRunnerManager threads."""
         self.suite_name = suite_name
         self.size = size
         self.test_source_cls = test_source_cls
         self.test_source_kwargs = test_source_kwargs
         self.browser_cls = browser_cls
         self.browser_kwargs = browser_kwargs
         self.executor_cls = executor_cls
         self.executor_kwargs = executor_kwargs