Deprecate --new-baseline and --new-test-results

Use --reset-results instead of --new-test-results.

Use "webkit-patch rebaseline-cl" or "--reset-results --add-platform-exceptions"
instead of --new-baseline.

Still keep "--add-platform-exceptions" to create platform-version-specific
baselines. May remove it in the future if no one wants it.

BUG=660231

Change-Id: I9671c69a806e5cba1ccd838944e757a364a947a2
Reviewed-on: https://chromium-review.googlesource.com/523386
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Walter Korman <wkorman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477075}

docs/testing/layout_tests.md

@@ -153,8 +153,7 @@ to see a full list of options. A few of the most useful options are below:
 | `--nocheck-sys-deps`        | Don't check system dependencies; this allows faster iteration. |
 | `--verbose`                 |	Produce more verbose output, including a list of tests that pass. |
 | `--no-pixel-tests`          | Disable the pixel-to-pixel PNG comparisons and image checksums for tests that don't call `testRunner.dumpAsText()` |
-| `--reset-results`           |	Write all generated results directly into the given directory, overwriting what's there. |
-| `--new-baseline`            |	Write all generated results into the most specific platform directory, overwriting what's there. Equivalent to `--reset-results --add-platform-expectations` |
+| `--reset-results`           |	Overwrite the current baselines (`-expected.{png|txt|wav}` files) with actual results, or create new baselines if there are no existing baselines. |
 | `--renderer-startup-dialog` | Bring up a modal dialog before running the test, useful for attaching a debugger. |
 | `--fully-parallel`          | Run tests in parallel using as many child processes as the system has cores. |
 | `--driver-logging`          | Print C++ logs (LOG(WARNING), etc).  |
@@ -431,36 +430,29 @@ machine?
 
 *** promo
 To automatically re-baseline tests across all Chromium platforms, using the
-buildbot results, see the
-[Rebaselining keywords in TestExpectations](./layout_test_expectations.md)
-and the
-[Rebaselining Tool](https://trac.webkit.org/wiki/Rebaseline).
+buildbot results, see [How to rebaseline](./layout_test_expectations.md#How-to-rebaseline).
 Alternatively, to manually run and test and rebaseline it on your workstation,
 read on.
 ***
 
-By default, text-only tests (ones that call `testRunner.dumpAsText()`) produce
-only text results. Other tests produce both new text results and new image
-results (the image baseline comprises two files, `-expected.png` and
-  `-expected.checksum`). So you'll need either one or three `-expected.\*` files
-in your new baseline, depending on whether you have a text-only test or not. If
-you enable `--no-pixel-tests`, only new text results will be produced, even for
-tests that do image comparisons.
-
 ```bash
 cd src/third_party/WebKit
-Tools/Scripts/run-webkit-tests --new-baseline foo/bar/test.html
+Tools/Scripts/run-webkit-tests --reset-results foo/bar/test.html
 ```
 
-The above command will generate a new baseline for
-`LayoutTests/foo/bar/test.html` and put the output files in the right place,
-e.g.
-`LayoutTests/platform/chromium-win/LayoutTests/foo/bar/test-expected.{txt,png,checksum}`.
+If there are current expectation files for `LayoutTests/foo/bar/test.html`,
+the above command will overwrite the current baselines at their original
+locations with the actual results. The current baseline means the `-expected.*`
+file used to compare the actual result when the test is run locally, i.e. the
+first file found in the [baseline search path]
+(https://cs.chromium.org/search/?q=port/base.py+baseline_search_path).
+
+If there are no current baselines, the above command will create new baselines
+in the platform-independent directory, e.g.
+`LayoutTests/foo/bar/test-expected.{txt,png}`.
 
 When you rebaseline a test, make sure your commit description explains why the
-test is being re-baselined. If this is a special case (i.e., something we've
-decided to be different with upstream), please put a README file next to the new
-expected output explaining the difference.
+test is being re-baselined.
 
 ## web-platform-tests
 

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

@@ -270,9 +270,7 @@ def _prepare_lists(self, paths, test_names):
     def _test_input_for_file(self, test_file):
         return TestInput(test_file,
                          self._options.slow_time_out_ms if self._test_is_slow(test_file) else self._options.time_out_ms,
-                         self._test_requires_lock(test_file),
-                         should_add_missing_baselines=(self._options.new_test_results and
-                                                       not self._test_is_expected_missing(test_file)))
+                         self._test_requires_lock(test_file))
 
     def _test_requires_lock(self, test_file):
         """Return True if the test needs to be locked when running multiple

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py

@@ -57,8 +57,6 @@ def run_single_test(
 
 
 class SingleTestRunner(object):
-    (ALONGSIDE_TEST, PLATFORM_DIR, VERSION_DIR, UPDATE) = ('alongside', 'platform', 'version', 'update')
-
     def __init__(self, port, options, results_directory, worker_name,
                  primary_driver, secondary_driver, test_input, stop_when_done):
         self._port = port
@@ -73,7 +71,6 @@ def __init__(self, port, options, results_directory, worker_name,
         self._should_run_pixel_test = test_input.should_run_pixel_test
         self._should_run_pixel_test_first = test_input.should_run_pixel_test_first
         self._reference_files = test_input.reference_files
-        self._should_add_missing_baselines = test_input.should_add_missing_baselines
         self._stop_when_done = stop_when_done
 
         # If this is a virtual test that uses the default flags instead of the
@@ -105,7 +102,7 @@ def _expected_driver_output(self):
                             self._port.expected_audio(self._test_name))
 
     def _should_fetch_expected_checksum(self):
-        return self._should_run_pixel_test and not (self._options.new_baseline or self._options.reset_results)
+        return self._should_run_pixel_test and not self._options.reset_results
 
     def _driver_input(self):
         # The image hash is used to avoid doing an image dump if the
@@ -166,8 +163,6 @@ def _run_compare_test(self):
         expected_driver_output = self._expected_driver_output()
 
         test_result = self._compare_output(expected_driver_output, driver_output)
-        if self._should_add_missing_baselines:
-            self._add_missing_baselines(test_result, driver_output)
         test_result_writer.write_test_result(self._filesystem, self._port, self._results_directory,
                                              self._test_name, driver_output, expected_driver_output, test_result.failures)
         return test_result
@@ -185,61 +180,32 @@ def _run_rebaseline(self):
 
     _render_tree_dump_pattern = re.compile(r"^layer at \(\d+,\d+\) size \d+x\d+\n")
 
-    def _add_missing_baselines(self, test_result, driver_output):
-        missing_image = test_result.has_failure_matching_types(
-            test_failures.FailureMissingImage, test_failures.FailureMissingImageHash)
-        if test_result.has_failure_matching_types(test_failures.FailureMissingResult):
-            self._save_baseline_data(driver_output.text, '.txt', self._location_for_missing_baseline(driver_output.text, '.txt'))
-        if test_result.has_failure_matching_types(test_failures.FailureMissingAudio):
-            self._save_baseline_data(driver_output.audio, '.wav', self._location_for_missing_baseline(driver_output.audio, '.wav'))
-        if missing_image:
-            self._save_baseline_data(driver_output.image, '.png', self._location_for_missing_baseline(driver_output.image, '.png'))
-
-    def _location_for_missing_baseline(self, data, extension):
-        if self._options.add_platform_exceptions:
-            return self.VERSION_DIR
-        if extension == '.png':
-            return self.PLATFORM_DIR
-        if extension == '.wav':
-            return self.ALONGSIDE_TEST
-        if extension == '.txt' and self._render_tree_dump_pattern.match(data):
-            return self.PLATFORM_DIR
-        return self.ALONGSIDE_TEST
-
     def _update_or_add_new_baselines(self, driver_output):
-        location = self.VERSION_DIR if self._options.add_platform_exceptions else self.UPDATE
-        self._save_baseline_data(driver_output.text, '.txt', location)
-        self._save_baseline_data(driver_output.audio, '.wav', location)
+        self._save_baseline_data(driver_output.text, '.txt')
+        self._save_baseline_data(driver_output.audio, '.wav')
         if self._should_run_pixel_test:
-            self._save_baseline_data(driver_output.image, '.png', location)
+            self._save_baseline_data(driver_output.image, '.png')
 
-    def _save_baseline_data(self, data, extension, location):
+    def _save_baseline_data(self, data, extension):
         if data is None:
             return
         port = self._port
         fs = self._filesystem
 
-        if location == self.ALONGSIDE_TEST:
-            output_dir = fs.dirname(port.abspath_for_test(self._test_name))
-        elif location == self.VERSION_DIR:
+        if self._options.add_platform_exceptions:
             output_dir = fs.join(port.baseline_version_dir(), fs.dirname(self._test_name))
-        elif location == self.PLATFORM_DIR:
-            output_dir = fs.join(port.baseline_platform_dir(), fs.dirname(self._test_name))
-        elif location == self.UPDATE:
-            output_dir = fs.dirname(port.expected_filename(self._test_name, extension))
         else:
-            raise AssertionError('unrecognized baseline location: %s' % location)
+            output_dir = fs.dirname(port.expected_filename(self._test_name, extension))
 
         fs.maybe_make_directory(output_dir)
         output_basename = fs.basename(fs.splitext(self._test_name)[0] + '-expected' + extension)
         output_path = fs.join(output_dir, output_basename)
 
-        if location == self.VERSION_DIR:
-            fallback_path = port.expected_filename(self._test_name, extension)
-            if fallback_path != output_path and fs.sha1(fallback_path) == hashlib.sha1(data).hexdigest():
-                _log.info('Not writing new expected result "%s" because it is the same as "%s"',
-                          port.relative_test_filename(output_path), port.relative_test_filename(fallback_path))
-                return
+        current_expected_path = port.expected_filename(self._test_name, extension)
+        if fs.exists(current_expected_path) and fs.sha1(current_expected_path) == hashlib.sha1(data).hexdigest():
+            _log.info('Not writing new expected result "%s" because it is the same as the current expected result',
+                      port.relative_test_filename(output_path))
+            return
 
         _log.info('Writing new expected result "%s"', port.relative_test_filename(output_path))
         port.update_baseline(output_path, data)

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_input.py

@@ -32,7 +32,7 @@ class TestInput(object):
     """Groups information about a test for easy passing of data."""
 
     def __init__(self, test_name, timeout_ms=None, requires_lock=None, reference_files=None,
-                 should_run_pixel_test=None, should_add_missing_baselines=True):
+                 should_run_pixel_test=None):
         # TestInput objects are normally constructed by the manager and passed
         # to the workers, but these some fields are set lazily in the workers
         # where possible, because they require us to look at the filesystem,
@@ -42,16 +42,13 @@ def __init__(self, test_name, timeout_ms=None, requires_lock=None, reference_fil
         self.requires_lock = requires_lock
         self.reference_files = reference_files
         self.should_run_pixel_test = should_run_pixel_test
-        self.should_add_missing_baselines = should_add_missing_baselines
 
     def __repr__(self):
         return (
             "TestInput('%s', timeout_ms=%s, requires_lock=%s, "
-            'reference_files=%s, should_run_pixel_test=%s, '
-            'should_add_missing_baselines=%s)' % (
+            'reference_files=%s, should_run_pixel_test=%s)' % (
                 self.test_name,
                 self.timeout_ms,
                 self.requires_lock,
                 self.reference_files,
-                self.should_run_pixel_test,
-                self.should_add_missing_baselines))
+                self.should_run_pixel_test))

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

@@ -80,6 +80,10 @@ def main(argv, stdout, stderr):
         return exit_codes.UNEXPECTED_ERROR_EXIT_STATUS
 
 
+def deprecate(option, opt_str, _, parser):
+    parser.error('%s: %s' % (opt_str, option.help))
+
+
 def parse_args(args):
     option_group_definitions = []
 
@@ -174,16 +178,16 @@ def parse_args(args):
                 help='Path to write the JSON test results for only *failing* tests.'),
             optparse.make_option(
                 '--new-baseline',
-                action='store_true',
-                default=False,
-                help=('Save generated results as new baselines into the *most-specific-platform* '
-                      "directory, overwriting whatever's already there. Equivalent to "
-                      '--reset-results --add-platform-exceptions')),
+                action='callback',
+                callback=deprecate,
+                help=('Deprecated. Use "webkit-patch rebaseline-cl" instead, or '
+                      '"--reset-results --add-platform-exceptions" if you do want to create '
+                      'platform-version-specific new baselines locally.')),
             optparse.make_option(
                 '--new-test-results',
-                action='store_true',
-                default=False,
-                help='Create new baselines when no expected results exist'),
+                action='callback',
+                callback=deprecate,
+                help='Deprecated. Use --reset-results instead.'),
             optparse.make_option(
                 '--no-show-results',
                 dest='show_results',
@@ -509,10 +513,6 @@ def _set_up_derived_options(port, options, args):
             additional_platform_directories.append(port.host.filesystem.abspath(path))
         options.additional_platform_directory = additional_platform_directories
 
-    if options.new_baseline:
-        options.reset_results = True
-        options.add_platform_exceptions = True
-
     if options.pixel_test_directories:
         options.pixel_tests = True
         verified_dirs = set()