Enabling archiving of test results by default in run-webkit-tests.

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

Index: Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py b/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
index b5d4cce27c8d5b33c1ca1a3a9811b38f2f6aa9d0..72b5da031c15559d806240fa5e720fd1f2d7885c 100644
--- a/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
@@ -167,16 +167,16 @@ class Manager(object):
         archived_path = self._filesystem.join(self._filesystem.dirname(self._results_directory), archived_name)
         self._filesystem.move(self._results_directory, archived_path)
 
-    def _clobber_old_archived_results(self):
+    def _limit_archived_results_count(self):
         results_directory_path = self._filesystem.dirname(self._results_directory)
         file_list = self._filesystem.listdir(results_directory_path)
         results_directories = []
         for dir in file_list:
             file_path = self._filesystem.join(results_directory_path, dir)
-            if self._filesystem.isdir(file_path):
+            if self._filesystem.isdir(file_path) and 'layout-test-results' in dir:

[Dirk Pranke, 2014/08/20 20:09:57]

You should use the basename of self._results_directory, like we do in
rename_results_folder(), above, rather than assuming a hardcoded string.

[patro, 2014/08/21 13:40:39]

Done.

                 results_directories.append(file_path)
         results_directories.sort(key=lambda x: self._filesystem.mtime(x))
-        self._printer.write_update("Clobbering old archived results in %s" % results_directory_path)
+        self._printer.write_update("Clobbering excess archived results in %s" % results_directory_path)
         for dir in results_directories[:-self.ARCHIVED_RESULTS_LIMIT]:
             self._filesystem.rmtree(dir)
 
@@ -202,14 +202,12 @@ class Manager(object):
                 self._port.stop_helper()
                 return exit_code
 
-        if self._options.enable_versioned_results and self._filesystem.exists(self._results_directory):
-            if self._options.clobber_old_results:
-                _log.warning("Flag --enable_versioned_results overrides --clobber-old-results.")
-            self._clobber_old_archived_results()
+        if self._options.clobber_old_results:
+            self._clobber_old_results()
+        elif self._filesystem.exists(self._results_directory):
+            self._limit_archived_results_count()
             # Rename the existing results folder for archiving.
             self._rename_results_folder()
-        elif self._options.clobber_old_results:
-            self._clobber_old_results()
 
         # Create the output directory if it doesn't already exist.
         self._port.host.filesystem.maybe_make_directory(self._results_directory)
@@ -395,16 +393,16 @@ class Manager(object):
                 writer.write_crash_log(crash_log)
 
     def _clobber_old_results(self):
-        # Just clobber the actual test results directories since the other
-        # files in the results directory are explicitly used for cross-run
-        # tracking.
-        self._printer.write_update("Clobbering old results in %s" %
-                                   self._results_directory)
-        layout_tests_dir = self._port.layout_tests_dir()
-        possible_dirs = self._port.test_dirs()
-        for dirname in possible_dirs:
-            if self._filesystem.isdir(self._filesystem.join(layout_tests_dir, dirname)):
-                self._filesystem.rmtree(self._filesystem.join(self._results_directory, dirname))
+        results_directory_path = self._filesystem.dirname(self._results_directory)
+        file_list = self._filesystem.listdir(results_directory_path)
+        results_directories = []
+        for dir in file_list:
+            file_path = self._filesystem.join(results_directory_path, dir)
+            if self._filesystem.isdir(file_path) and 'layout-test-results' in dir:

[Dirk Pranke, 2014/08/20 20:09:57]

same comment.

In fact, you should probably split the logic out into separate functions,
perhaps one that figures out which directories to delete and another that
actually doese the deleting of a list of dirs.

[patro, 2014/08/21 13:40:39]

Done.

[patro, 2014/08/21 13:40:39]

Done.

+                results_directories.append(file_path)
+        self._printer.write_update("Clobbering all archived results in %s" % results_directory_path)
+        for dir in results_directories:
+            self._filesystem.rmtree(dir)
 
         # Port specific clean-up.
         self._port.clobber_old_port_specific_results()