Changeset 124240 in webkit

Timestamp:

Jul 31, 2012, 12:42:55 PM (13 years ago)

Author:

dpranke@chromium.org

Message:

nrwt: clean up prepare_lists_and_print_output, run, set_up_run a bit
​https://bugs.webkit.org/show_bug.cgi?id=92781

Reviewed by Ryosuke Niwa.

More refactoring ... rename prepare_lists_and_print_output to
just prepare_lists so that it only has a single purpose, and
clean up the surrounding code a bit as well.

• Scripts/webkitpy/layout_tests/controllers/manager.py:

(Manager._collect_tests):
(Manager._prepare_lists):
(Manager._set_up_run):
(Manager.run):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/controllers/manager.py (modified) (8 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-07-31  Dirk Pranke  <dpranke@chromium.org>
+
+        nrwt: clean up prepare_lists_and_print_output, run, set_up_run a bit
+        https://bugs.webkit.org/show_bug.cgi?id=92781
+
+        Reviewed by Ryosuke Niwa.
+
+        More refactoring ... rename prepare_lists_and_print_output to
+        just prepare_lists so that it only has a single purpose, and
+        clean up the surrounding code a bit as well.
+
+        * Scripts/webkitpy/layout_tests/controllers/manager.py:
+        (Manager._collect_tests):
+        (Manager._prepare_lists):
+        (Manager._set_up_run):
+        (Manager.run):
+
 2012-07-31  Dirk Pranke  <dpranke@chromium.org>
 

trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py

@@ -331 +331 @@
 
     def _collect_tests(self, args):
-        self._paths, self._test_names = self._finder.find_tests(self._options, args)
+        return self._finder.find_tests(self._options, args)
 
     def _is_http_test(self, test):
@@ -345 +345 @@
         return self.PERF_SUBDIR == test or (self.PERF_SUBDIR + self._port.TEST_PATH_SEPARATOR) in test
 
-    # FIXME: This method is way too long and needs to be broken into pieces.
-    def prepare_lists_and_print_output(self):
-        """Create appropriate subsets of test lists and returns a
-        ResultSummary object. Also prints expected test counts.
-        """
-        num_all_test_files = len(self._test_names)
-
+    def _prepare_lists(self):
         tests_to_skip = self._finder.skip_tests(self._paths, self._test_names, self._expectations, self._http_tests())
-        if not self._test_names:
-            _log.critical('No tests to run.')
-            return None
+        self._test_names = list(set(self._test_names) - tests_to_skip)
 
         # Create a sorted list of test files so the subset chunk,
         # if used, contains alphabetically consecutive tests.
-        self._test_names = list(set(self._test_names) - tests_to_skip)
         if self._options.randomize_order:
             random.shuffle(self._test_names)
@@ -379 +370 @@
 
         iterations = self._options.repeat_each * self._options.iterations
-        result_summary = ResultSummary(self._expectations, set(self._test_names), iterations, tests_to_skip)
-
-        self._printer.print_found(num_all_test_files, len(self._test_names), self._options.repeat_each, self._options.iterations)
-        self._printer.print_expected(result_summary, self._expectations.get_tests_with_result_type)
-
-        return result_summary
+        return ResultSummary(self._expectations, set(self._test_names), iterations, tests_to_skip)
 
     def _get_dir_for_test_file(self, test_file):
@@ -664 +650 @@
 
     def _set_up_run(self):
-        """Configures the system to be ready to run tests.
-
-        Returns a ResultSummary object if we should continue to run tests,
-        or None if we should abort.
-
-        """
+        self._printer.write_update("Checking build ...")
+        if not self._port.check_build(self.needs_servers()):
+            _log.error("Build check failed")
+            return False
+
         # This must be started before we check the system dependencies,
         # since the helper may do things to make the setup correct.
@@ -681 +666 @@
             if not self._port.check_sys_deps(self.needs_servers()):
                 self._port.stop_helper()
-                return None
+                return False
 
         if self._options.clobber_old_results:
@@ -690 +675 @@
 
         self._port.setup_test_run()
-
-        self._printer.write_update("Preparing tests ...")
-        result_summary = self.prepare_lists_and_print_output()
-        if not result_summary:
-            return None
-
-        return result_summary
+        return True
 
     def run(self, args):
@@ -702 +681 @@
         self._printer.write_update("Collecting tests ...")
         try:
-            self._collect_tests(args)
-        except IOError as e:
-            # This is raised when the --test-list doesn't exist.
-            return -1
-
-        self._printer.write_update("Checking build ...")
-        if not self._port.check_build(self.needs_servers()):
-            _log.error("Build check failed")
+            self._paths, self._test_names = self._collect_tests(args)
+        except IOError as exception:
+            # This is raised if --test-list doesn't exist
             return -1
 
@@ -715 +689 @@
         self._expectations = test_expectations.TestExpectations(self._port, self._test_names)
 
-        result_summary = self._set_up_run()
-        if not result_summary:
+        num_all_test_files_found = len(self._test_names)
+        result_summary = self._prepare_lists()
+
+        # Check to make sure we're not skipping every test.
+        if not self._test_names:
+            _log.critical('No tests to run.')
             return -1
 
-        assert(self._test_names)
+        self._printer.print_found(num_all_test_files_found, len(self._test_names), self._options.repeat_each, self._options.iterations)
+        self._printer.print_expected(result_summary, self._expectations.get_tests_with_result_type)
+
+        if not self._set_up_run():
+            return -1
 
         start_time = time.time()