2011-04-12 Dirk Pranke <dpranke@chromium.org>

        Reviewed by Tony Chang.

        new-run-webkit-tests: --results-directory is relative to builddir, not $PWD
        https://bugs.webkit.org/show_bug.cgi?id=58272

        NRWT was interpreting the --results-directory cmd line arg as
        relative to the build directory, not the current working
        directory (ORWT uses the latter, which is much more intuitive).

        This patch fixes the base case, but includes an override for
        Chromium that is needed until the bots can be updated.

        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
        * Scripts/webkitpy/layout_tests/port/chromium.py:
        * Scripts/webkitpy/layout_tests/port/port_testcase.py:
        * Scripts/webkitpy/layout_tests/port/test.py:
        * Scripts/webkitpy/layout_tests/port/webkit.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:

Canonical link: https://commits.webkit.org/73406@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83646 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog

@@ -1,3 +1,26 @@
+2011-04-12  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Tony Chang.
+
+        new-run-webkit-tests: --results-directory is relative to builddir, not $PWD
+        https://bugs.webkit.org/show_bug.cgi?id=58272
+
+        NRWT was interpreting the --results-directory cmd line arg as
+        relative to the build directory, not the current working
+        directory (ORWT uses the latter, which is much more intuitive).
+
+        This patch fixes the base case, but includes an override for
+        Chromium that is needed until the bots can be updated.
+
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        * Scripts/webkitpy/layout_tests/port/port_testcase.py:
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
+
 2011-04-12  Philippe Normand  <pnormand@igalia.com>
 
         Unreviewed, roll out r83621 as it broke the GTK build.

Tools/Scripts/webkitpy/layout_tests/port/base.py

@@ -140,6 +140,7 @@ def __init__(self, port_name=None, options=None,
             self._options.configuration = self.default_configuration()
         self._test_configuration = None
         self._multiprocessing_is_available = (multiprocessing is not None)
+        self._results_directory = None
 
     def default_child_processes(self):
         """Return the number of DumpRenderTree instances to use for this
@@ -570,8 +571,15 @@ def abspath_for_test(self, test_name):
         return self._filesystem.normpath(self._filesystem.join(self.layout_tests_dir(), test_name))
 
     def results_directory(self):
-        """Absolute path to the place to store the test results."""
-        raise NotImplementedError('Port.results_directory')
+        """Absolute path to the place to store the test results (uses --results-directory)."""
+        if not self._results_directory:
+            option_val = self.get_option('results_directory') or self.default_results_directory()
+            self._results_directory = self._filesystem.abspath(option_val)
+        return self._results_directory
+
+    def default_results_directory(self):
+        """Absolute path to the default place to store the test results."""
+        raise NotImplementedError()
 
     def setup_test_run(self):
         """Perform port-specific work at the beginning of a test run."""

Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py

@@ -274,7 +274,7 @@ def test_virtual_methods(self):
         self.assertVirtual(port.create_driver, 0)
         self.assertVirtual(port.diff_image, None, None)
         self.assertVirtual(port.path_to_test_expectations_file)
-        self.assertVirtual(port.results_directory)
+        self.assertVirtual(port.default_results_directory)
         self.assertVirtual(port.test_expectations)
         self.assertVirtual(port._path_to_apache)
         self.assertVirtual(port._path_to_apache_config_file)

Tools/Scripts/webkitpy/layout_tests/port/chromium.py

@@ -200,14 +200,28 @@ def path_to_test_expectations_file(self):
         return self.path_from_webkit_base('LayoutTests', 'platform',
             'chromium', 'test_expectations.txt')
 
-    def results_directory(self):
+    def default_results_directory(self):
         try:
             return self.path_from_chromium_base('webkit',
                 self.get_option('configuration'),
-                self.get_option('results_directory'))
+                'layout-test-results')
         except AssertionError:
             return self._build_path(self.get_option('configuration'),
-                                    self.get_option('results_directory'))
+                                    'layout-test-results')
+
+    def results_directory(self):
+        # FIXME: This is a hack needed for compatibility with the Chromium
+        # buildbots until their scripts can be updated. Once they have been,
+        # this routine can be deleted and we can use the base class.
+        # See https://bugs.webkit.org/show_bug.cgi?id=58272 for more details.
+        if not self._results_directory:
+            option_val = self.get_option('results_directory')
+            if option_val and not self._filesystem.isabs(option_val):
+                self._results_directory = self._build_path(self.get_option('configuration'),
+                                                           option_val)
+            else:
+                return super(ChromiumPort, self).results_directory(self)
+        return self._results_directory
 
     def setup_test_run(self):
         # Delete the disk cache if any to ensure a clean test run.

Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py

@@ -38,8 +38,7 @@
     multiprocessing = None
 
 from webkitpy.tool import mocktool
-mock_options = mocktool.MockOptions(results_directory='layout-test-results',
-                                    use_apache=True,
+mock_options = mocktool.MockOptions(use_apache=True,
                                     configuration='Release')
 
 # FIXME: This should be used for all ports, not just WebKit Mac. See
@@ -60,10 +59,7 @@ def make_port(self, options=mock_options):
         if not maker:
             return None
 
-        port = maker(options=options)
-        if hasattr(options, "results_directory"):
-            port._options.results_directory = port.results_directory()
-        return port
+        return maker(options=options)
 
     def test_default_worker_model(self):
         port = self.make_port()

Tools/Scripts/webkitpy/layout_tests/port/test.py

@@ -329,11 +329,8 @@ def name(self):
     def _path_to_wdiff(self):
         return None
 
-    def results_directory(self):
-        if not self._results_directory:
-            self._results_directory = self._filesystem.join('/tmp',
-                self.get_option('results_directory'))
-        return self._results_directory
+    def default_results_directory(self):
+        return '/tmp/layout-test-results'
 
     def setup_test_run(self):
         pass

Tools/Scripts/webkitpy/layout_tests/port/webkit.py

@@ -166,10 +166,10 @@ def _diff_image_reply(self, sp, diff_filename):
         sp.stop()
         return result
 
-    def results_directory(self):
+    def default_results_directory(self):
         # Results are store relative to the built products to make it easy
         # to have multiple copies of webkit checked out and built.
-        return self._build_path(self.get_option('results_directory'))
+        return self._build_path('layout-test-results')
 
     def setup_test_run(self):
         # This port doesn't require any specific configuration.

Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

@@ -152,12 +152,6 @@ def _set_up_derived_options(port_obj, options):
     if not options.use_apache:
         options.use_apache = sys.platform in ('darwin', 'linux2')
 
-    if not port_obj._filesystem.isabs(options.results_directory):
-        # This normalizes the path to the build dir.
-        # FIXME: how this happens is not at all obvious; this is a dumb
-        # interface and should be cleaned up.
-        options.results_directory = port_obj.results_directory()
-
     if not options.time_out_ms:
         if options.configuration == "Debug":
             options.time_out_ms = str(2 * test_runner.TestRunner.DEFAULT_TEST_TIMEOUT_MS)
@@ -288,10 +282,7 @@ def parse_args(args=None):
         optparse.make_option("--tolerance",
             help="Ignore image differences less than this percentage (some "
                 "ports may ignore this option)", type="float"),
-        optparse.make_option("--results-directory",
-            default="layout-test-results",
-            help="Output results directory source dir, relative to Debug or "
-                 "Release"),
+        optparse.make_option("--results-directory", help="Location of test results"),
         optparse.make_option("--build-directory",
             help="Path to the directory under which build files are kept (should not include configuration)"),
         optparse.make_option("--new-baseline", action="store_true",

Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py

@@ -35,6 +35,7 @@
 import codecs
 import itertools
 import logging
+import os
 import Queue
 import sys
 import thread
@@ -465,10 +466,12 @@ def test_results_directory_default(self):
     def test_results_directory_relative(self):
         # We run a configuration that should fail, to generate output, then
         # look for what the output results url was.
-
+        fs = port.unit_test_filesystem()
+        fs.maybe_make_directory('/tmp/cwd')
+        fs.chdir('/tmp/cwd')
         res, out, err, user = logging_run(['--results-directory=foo'],
-                                          tests_included=True)
-        self.assertEqual(user.opened_urls, ['/tmp/foo/results.html'])
+                                          tests_included=True, filesystem=fs)
+        self.assertEqual(user.opened_urls, ['/tmp/cwd/foo/results.html'])
 
     # These next tests test that we run the tests in ascending alphabetical
     # order per directory. HTTP tests are sharded separately from other tests,