LayoutTest/script tests without expectations: is this the right way to land new tests?

[Adam Klein]

I added a new script test in https://codereview.chromium.org/48633006 and found that there was no need to add an -expected.txt: run-webkit-tests could detect that my test passed or failed based on the text output. This is pretty neat, but surprising for a veteran of script tests. And the new test is now inconsistent with the rest of the tests in that directory.

Anyway, I was wondering if the intention is that new script tests be landed in this fashion, or if I should be adding baselines manually (the other thing I noticed is that run-webkit-tests no longer spits out an expectation when one is missing).

Cheers,

Adam

[Elliott Sprehn]

Dirk had made that change but we decided it was too confusing and be reverted the change I thought. We shouldn't be landing tests without expectations outside external imported suites.

[Dirk Pranke]

Sigh, I thought that change had been reverted, but apparently the CQ defeated me. I'm trying again now ...

That said, I don't quite agree w/ Elliott's response. Let me rephrase things and provide some background

The W3C has many thousands of tests written in a pass/fail assert-style manner using the testharness.js script . They do not have corresponding -expected.txt files for them, and I have no desire to track thousands of -expected.txt baselines that we have to generate and verify (that's almost as bad as pixel tests!).

The change in question was meant to add the following logic:

if a test does not produce any pixel output and is not a ref test and does not produce a render tree as text output

*and* we don't have an -expected.txt baseline

*and* the test contains at least one line starting with "PASS" or "FAIL"

then treat it as a pass/fail test and fail it if it contains any lines containing FAIL.

This works fine for the testharness-based tests, but it turns out that js-test-pre/post always prints at least one PASS line if the scripts were all parsed successfully, even if no other asserts fire. So, the change was reverted until I can change the logic to accomodate that.

I think a better solution would be to modify the output of testharness.js/testharnessreport.js to print a magic string like "THIS IS A TESTHARNESS TEST" that we can also check for to be sure :).

Once we have an appropriately modified harness, I think it's reasonable to not require existing baselines for tests even in our own (non-W3C) tests. Note that this approach can't handle the case where some assertions pass in a test but some are expected to fail (you still need an -expected.txt reference for such situations), and there may be cases where the test will not run some expected assertions and we won't notice, but that seems like an acceptable risk.

Thoughts/feedback welcome,

-- Dirk