Add PRESUBMIT checks for not required testharness expected files.

Tools/Scripts/webkitpy/layout_tests/testharness/testharness_results_checker.py

+# Copyright 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Utility module for testharness."""
+
+
+# const definitions
+TESTHARNESSREPORT_HEADER = 'This is a testharness.js-based test.'
+TESTHARNESSREPORT_FOOTER = 'Harness: the test ran to completion.'
+
+
+def is_testharness_output(content_text):
+    """Returns whether the content_text in parameter is a testharness output.
+
+    Returns:
+        True if content_text is a valid testharness output. False otherwise.
+    """
+
+    # Leading and trailing white spaces are accepted.
+    lines = content_text.strip().splitlines()
+    lines = [line.strip() for line in lines]
+
+    # A testharness output is defined as containing the header and the footer.
+    found_header = False
+    found_footer = False
+    for line in lines:
+        if line == TESTHARNESSREPORT_HEADER:
+            found_header = True
+        elif line == TESTHARNESSREPORT_FOOTER:
+            found_footer = True
+
+    return found_header and found_footer
+
+
+def is_testharness_output_passing(content_text):

[Dirk Pranke, 2014/06/13 22:27:07]

Can you rewrite this as:

def is_testharness_output_passing(content_text):
    if not is_testharness_output(content_text):
        return False
    lines = [line.strip() for line in content_text.strip().splitlines()]
    return not any(line.startswith('FAIL') or line.startswith('TIMEOUT') or
line.startswith('NOTRUN') or line.startswith('Harness Error')
                           for line in lines)

?

Alternatively, you could refactor both routines to operate on an array of
stripped lines so that you don't have to split and strip them twice.

The basic idea is not to duplicate the overall format checking twice.

[mlamouri (slow - plz ping), 2014/06/15 12:12:41]

The only reason why I'm checking the format here is because we currently are
very conservative in the way we check test output and we expect the test to
contain no unexpected line. I'm not sure  if that's the right way to do it
honestly. I would tend to think that it's too conservative and we should allow
any kind of output (basically, this rule mean that a test would have to cleanup
any HTML elements with a text content).

Your proposal doesn't take that into account and would fail some test cases.

So, the only way to prevent checking the output twice would have to have one
internal method that checks the format and the passing at the same time and both
public methods would use that internal one. However, I don't think that's the
way to go forward. I think we could simply let unexpected output being allowed,
in which case I could have a passing check looking roughly as you suggested but
I would prefer that to be in a separate CL.

WDYT?

+    """Returns whether the content_text in parameter is a passing testharness output.
+
+    Note:
+        It is expected that the |content_text| is a testharness output.
+
+    Returns:
+        True if content_text is a passing testharness output. False otherwise.
+    """
+
+    # Leading and trailing white spaces are accepted.
+    lines = content_text.strip().splitlines()
+    lines = [line.strip() for line in lines]
+
+    # The check is very conservative and reject any unexpected content in the output.
+    for line in lines:
+        # There should be no empty lines.
+        if len(line) == 0:
+            return False
+
+        # Those lines are expected to be exactly equivalent.
+        if line == TESTHARNESSREPORT_HEADER or \
+           line == TESTHARNESSREPORT_FOOTER:
+            continue
+
+        # Those are expected passing output.
+        if line.startswith('CONSOLE') or \
+           line.startswith('PASS'):
+            continue
+
+        # Those are expected failing outpu.
+        if line.startswith('FAIL') or \
+           line.startswith('TIMEOUT') or \
+           line.startswith('NOTRUN') or \
+           line.startswith('Harness Error. harness_status = '):
+            return False
+
+        # Unexpected output should be considered as a failure.
+        return False
+
+    return True