Please use `testharness.js` when writing layout tests.

[Mike West]

Hello, people on blink-dev who write layout tests!

Following up on a small piece of the testing discussion at BlinkOn: please do not use `js-test.js` when writing layout tests. Please use `testharness.js` instead. Migrating to this mechanism will make your tests' assertions more explicit, and should make it more easily possible for us to integrate our body of layout tests with the cross-vendor Web Platform Tests project going forward (and you won't have to land `-expected.txt` files anymore!).

I recently landed a presubmit check to throw (overridable, for now) warnings for new `js-test.js` usage, and I've updated the "Writing Tests" wiki bits accordingly.

Thanks!

-mike

[Raymond Toy]

Is there a best-practices document for testharness?

I tried to use the testharness to a new CL, and based on what I did compared to I would have done with the existing js-test, webaudio tests are very much less informative, especially on failure.  For example, https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/webaudio/panner-automation-position-expected.txt?sq=package:chromium produces lots of useful information on pass, and even more useful information on test failure like the actual and expected values, their absolute and relative differences, and where the max diff and relative diff occurred and corresponding values.  This is really useful for adjusting threshold for all the different platforms, and I can use the trybots to help with that instead of manually building on every single platform myself.

Having the expected value printed out also caught a recent change in V8 where the Math.exp change caused the test to fail because the printed values changed. (The test itself still passed because the required threshold was still met.)

If I rewrote this test with testharness, I think the V8 change wouldn't have been caught.  And if the test did fail, the failure message wouldn't be fairly useless in telling me why.

By the way, there's a typo in the presubmit check message  The printed URL for the testharness page is missing a space so the URL looks like it ends in ".with".

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Mike West]

Hey Raymond!

On Sat, Jul 2, 2016 at 12:04 AM, Raymond Toy <rt...@google.com> wrote:

Is there a best-practices document for testharness?

Nothing Chrome/Blink-specific. I usually fall back on https://darobin.github.io/test-harness-tutorial/docs/using-testharness.html and the docs at http://testthewebforward.org/docs/testharness-tutorial.html.

 

I tried to use the testharness to a new CL, and based on what I did compared to I would have done with the existing js-test, webaudio tests are very much less informative, especially on failure.  For example, https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/webaudio/panner-automation-position-expected.txt?sq=package:chromium produces lots of useful information on pass, and even more useful information on test failure like the actual and expected values, their absolute and relative differences, and where the max diff and relative diff occurred and corresponding values.  This is really useful for adjusting threshold for all the different platforms, and I can use the trybots to help with that instead of manually building on every single platform myself.

It seems like you'd be able to write those as assertions. There's a lot going on in https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html?sq=package:chromium, so I'm not sure I'm following it all, but all the values you're looking at are exposed to JavaScript, and could be asserted to be such and such, couldn't they? It's just a matter of replacing the hard-coded list of expected values from the `.txt` file with hard-coded expected values in the `.html` file.

 

Having the expected value printed out also caught a recent change in V8 where the Math.exp change caused the test to fail because the printed values changed. (The test itself still passed because the required threshold was still met.)

If the `-expected.txt` file contains useful data that isn't web-exposed in a way that you can create assertions about, then you can certainly keep the file around. Writing tests with `testharness.js` allows you to remove that file, but if you don't, it will get checked just like it would for any other test.

 

If I rewrote this test with testharness, I think the V8 change wouldn't have been caught.  And if the test did fail, the failure message wouldn't be fairly useless in telling me why.

Why do you think that? Are the messages associated with assertions not enough (e.g. `assert_true(true, "True is true!");`)?

Thanks!

-mike

[Raymond Toy]

Simplified reply:  Yeah, we can make everything work out I think with the new testharness. It will just take a lot of time to do the conversion.  We will use the testharness for new tests, of course.

I do dislike the output where if you did something like

test(function () {

  assert_this();

  assert_that();

  assert_other();

})

the output only says one test passed.  This would have been three separate tests previously. 

But I'll get over it.

[Elliott Sprehn]

Can you write those as separate tests instead with shared setup code?

[Raymond Toy]

Yes, we can add a bunch of globals (ick) so that the tests can be done individually.  Well, maybe. Most of the tests use OfflineAudioContext.startRendering() which returns a promise, and the tests are in the promise resolver.  There's usually a fair amount of state between the setup and the resolver to figure out if the test passes or not.

[Steve Kobes]

On Fri, Jul 8, 2016 at 10:57 AM, 'Raymond Toy' via blink-dev <blin...@chromium.org> wrote:

Yes, we can add a bunch of globals (ick) so that the tests can be done individually.  Well, maybe. Most of the tests use OfflineAudioContext.startRendering() which returns a promise, and the tests are in the promise resolver.  There's usually a fair amount of state between the setup and the resolver to figure out if the test passes or not.

promise_test is nice for this sort of thing.