Support test harness (text-only) tests w/o expected results.

Support test harness (text-only) tests w/o expected results.

A large number of our existing text-only tests follow the convention of having JS assertions that either print "PASS", "FAIL", or "TIMEOUT" at the beginning of a line of text. If the test out therefor does not contain a FAIL or a TIMEOUT and contains at least one or more "PASS"es, it seems reasonable to assume that the test did actually pass (completely), and the -expected.txt is kinda redundant.

The testharness-based tests from the W3C follow this convention as well. Supporting this convention means that we don't need to review and check in thousands of boilerplate -expected.txt files, and we could potentially delete thousands of existing -expected.txt files in the Blink repo.

R=ojan@chromium.org, eseidel@chromium.org
BUG=268711
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159735

[ojan, 2013-08-06 22:54:07]

If we're going to do this for the non-imported tests, then I think it's
important that we be able to add a -expected.txt for the cases where it does
start failing and we want to rebaseline.

I'm not sure we want to do this for our non-imported tests. For example, there
are many tests that print out multiple pass lines. If they got a JS error or
something that didn't get recorded in the text dump and only printed half of the
pass lines, we'd want the test to fail.

Also, I think we need a better name for these than JS tests. JS tests is an
overloaded term. pass_fail_tests? no_expected_result_tests?

Maybe this isn't worth the extra overhead of everyone learning how these tests
work? Maybe we should just check in -expected.txt's for all of these.

I dunno, I'm on the fence.

[Dirk Pranke, 2013-08-07 00:23:40]

On 2013/08/06 22:54:07, ojan wrote:
> If we're going to do this for the non-imported tests, then I think it's
> important that we be able to add a -expected.txt for the cases where it does
> start failing and we want to rebaseline.

This is currently supported (with the caveat we discussed last week about
needing to find someplace to put files for the w3c tests a la platform/chromium
or w3c-baselines or something).

> 
> I'm not sure we want to do this for our non-imported tests. For example, there
> are many tests that print out multiple pass lines. If they got a JS error or
> something that didn't get recorded in the text dump and only printed half of
the
> pass lines, we'd want the test to fail.
> 

That's a good point about not detecting a partial number of PASSes. I'll have to
poke into the W3C test harness to see if I can detect that or not.
I'm fine w/ not deleting any existing -expected.txt files.

> Also, I think we need a better name for these than JS tests. JS tests is an
> overloaded term. pass_fail_tests? no_expected_result_tests?
> 

I think the W3C calls them JS tests, but I could be wrong. What else is also
called a JS test?

> Maybe this isn't worth the extra overhead of everyone learning how these tests
> work? Maybe we should just check in -expected.txt's for all of these.
> 
> I dunno, I'm on the fence.

I really really don't want to check in dummy -expected.txt files for the W3C
tests.

[ojan, 2013-08-07 00:30:33]

> That's a good point about not detecting a partial number of PASSes. I'll have
to
> poke into the W3C test harness to see if I can detect that or not.
> I'm fine w/ not deleting any existing -expected.txt files.
> 
> > Also, I think we need a better name for these than JS tests. JS tests is an
> > overloaded term. pass_fail_tests? no_expected_result_tests?
> > 
> 
> I think the W3C calls them JS tests, but I could be wrong. What else is also
> called a JS test?

The js-test-pre tests. I suppose they're called script-tests, but I still think
it's confusing.

[Dirk Pranke, 2013-10-08 22:52:44]

Post-BlinkOn, there was consensus that we should try to get *something* working
as quickly as we can so that others can start getting involved.

I'd like to dust off and land this patch so that we can handle the w3c's
testharness-based tests accordingly . I've renamed "js test" to "testharness
test"; hopefully that'll be fairly clear.

Note that we still support baselines for tests to handle expected partial passes
of test suites. Otherwise, I have no good solution to the "bailed out halfway"
problem yet. Someone suggested in BlinkOn that we modify testharness to print a
"test ran to completion" message and check for that as well. I'd like to look
into that, but as a separate patch.

WDYT?

[ojan, 2013-10-09 19:53:33]

lgtm

I don't think testharness_tests is especially clear either, but I don't want to
further block your progress here.

[Dirk Pranke, 2013-10-09 20:15:32]

On 2013/10/09 19:53:33, ojan wrote:
> lgtm
> 
> I don't think testharness_tests is especially clear either, but I don't want
to
> further block your progress here.

Thanks. I'm certainly open to other names if someone has a better idea ...

[commit-bot: I haz the power, 2013-10-11 18:20:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/22352002/7001

[commit-bot: I haz the power, 2013-10-11 18:20:18]

Failed to apply patch for
Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
  Hunk #1 FAILED at 217.
  1 out of 1 hunk FAILED -- saving rejects to file
Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py.rej

Patch:      
Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
Index: Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
diff --git
a/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
b/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
index
115428ecf9be8ccaa9eb22b292ff0265f066cfff..b07f5e3c681911d048e0cd2be50935035361a4bc
100644
--- a/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
@@ -217,12 +217,36 @@ class SingleTestRunner(object):
             # In case of timeouts, we continue since we still want to see the
text and image output.
             return TestResult(self._test_name, failures,
driver_output.test_time, driver_output.has_stderr(), pid=driver_output.pid)
 
-        failures.extend(self._compare_text(expected_driver_output.text,
driver_output.text))
-        failures.extend(self._compare_audio(expected_driver_output.audio,
driver_output.audio))
-        if self._should_run_pixel_test:
-            failures.extend(self._compare_image(expected_driver_output,
driver_output))
+        is_testharness_test, testharness_failures =
self._compare_testharness_test(driver_output, expected_driver_output)
+        if is_testharness_test:
+            failures.extend(testharness_failures)
+        else:
+            failures.extend(self._compare_text(expected_driver_output.text,
driver_output.text))
+            failures.extend(self._compare_audio(expected_driver_output.audio,
driver_output.audio))
+            if self._should_run_pixel_test:
+                failures.extend(self._compare_image(expected_driver_output,
driver_output))
         return TestResult(self._test_name, failures, driver_output.test_time,
driver_output.has_stderr(), pid=driver_output.pid)
 
+    def _compare_testharness_test(self, driver_output, expected_driver_output):
+        if expected_driver_output.image or expected_driver_output.audio or
expected_driver_output.text:
+            return False, []
+
+        if driver_output.image or driver_output.audio or
self._is_render_tree(driver_output.text):
+            return False, []
+
+        failures = []
+        found_a_pass = False
+        text = driver_output.text or ''
+        for line in text.splitlines():
+            if line.startswith('FAIL') or line.startswith('TIMEOUT'):
+                return True, [test_failures.FailureTestHarnessAssertion()]
+            if line.startswith('PASS'):
+                found_a_pass = True
+        return found_a_pass, failures
+
+    def _is_render_tree(self, text):
+        return text and "layer at (0,0) size 800x600" in text
+
     def _compare_text(self, expected_text, actual_text):
         failures = []
         if (expected_text and actual_text and

[commit-bot: I haz the power, 2013-10-11 18:34:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/22352002/15001

[commit-bot: I haz the power, 2013-10-11 22:32:41]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[commit-bot: I haz the power, 2013-10-15 22:44:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/22352002/30001

[commit-bot: I haz the power, 2013-10-16 09:36:19]

Change committed as 159735