Should we use testharness or js-test for new layout tests?

23 views
Skip to first unread message

Victor Costan

unread,
Nov 3, 2016, 7:50:32 PM11/3/16
to blink-dev
Hi, everyone!

I'd like to discuss whether new layout tests that will not be upstreamed to web-platform-tests should be written using testharness.js or js-test.

Some context, in case it helps orient the discussion: I'm writing a test for a drag-and-drop bug, which covers both Web-exposed properties of drag-an-drop events and the drag feedback (a.k.a. ghost/preview), which is not Web-exposed. Because I'm testing drag-and-drop, the test uses eventSender, but it can also be used as a manual test on other browsers.

I think that my test, as well as all new layout tests, should by default use testharness.js. My reasons are:
* testharness is used in WPT, so I assume it will become the well-known testing toolkit among browser developers. If this assumption turns out true, testharness-based tests will be easier to read by an average browser developer than another test.
* testharness has well-maintained external documentation.
* testharness already enjoys special treatment in run-webkit-tests, so we don't need to commit text expectations if we expect all the tests to pass; this makes testharness-based tests more robust.
* testharness has reasonable APIs for expressing test expectations and grouping tests; this is not an advantage over js-test, but it's feature parity that I think is worth noting.

The main downside that I can see is that when a browser developer encounters the testharness inclusion tags, the developer may incorrectly assume that the test is intended to be upstreamed to WPT.  I think this is a small cost given the benefits, and it can be mitigated by documentation.

This e-mail originates from the code review discussion at -- http://crrev.com/2477783002. Please read dpranke's comments there to get a full picture. I am not summarizing them to avoid ending up writing text that doesn't accurately represent the original comments.

Thank you very much in advance for your thoughts,
    Victor

Dirk Pranke

unread,
Nov 3, 2016, 8:26:01 PM11/3/16
to Victor Costan, blink-dev
Summarizing for this thread, I basically said that we shouldn't use testharness.js for non-upstreamed tests just because that's the standard we wanted to follow (and it was clear and understandable).

I asked Victor to start this thread because he has good reasons and we should decide what we want to do as a group.

I think agreeing to use testharness.js for non-upstreamed tests would also be fine as long as we were careful to not try and add stuff to the harness itself, and as long as everyone had the same expectations. Like all style questions, it's more important to agree on some set of rules than to pick any given set.

-- Dirk

Victor Costan

unread,
Nov 3, 2016, 8:35:48 PM11/3/16
to Dirk Pranke, blink-dev
On Thu, Nov 3, 2016 at 5:25 PM, Dirk Pranke <dpr...@chromium.org> wrote:
Summarizing for this thread, I basically said that we shouldn't use testharness.js for non-upstreamed tests just because that's the standard we wanted to follow (and it was clear and understandable).

I apologize, I didn't mean to imply otherwise! I didn't want to summarize because I'm worried about _my_ ability to do it well. The comments were very clear.

    Victor

Matt Falkenhagen

unread,
Nov 4, 2016, 4:17:11 AM11/4/16
to Victor Costan, Dirk Pranke, blink-dev
Just a data point: service worker layout tests use testharness.js and those that are not intended to be upstreamed are in a chromium/ subdirectory (these might test crash fixes, error messages, gc behavior, and other implementation details).

We have a README in chromium/ explaining this and in theory each test should have a comment explaining why it's in chromium/.

Of course we were able to organize it this way since we started from scratch with zero tests, so it may not apply well to Blink as a whole.

Takayoshi Kochi

unread,
Nov 4, 2016, 5:19:04 AM11/4/16
to Matt Falkenhagen, Victor Costan, Dirk Pranke, blink-dev
There was a thread on blink-dev "Please use `testharness.js` when writing layout tests.":
https://groups.google.com/a/chromium.org/d/topic/blink-dev/rDKsRjVYsZM/discussion

I read the discussion as "don't use js-test.js any more", but the discussion didn't say about
tests that are clearly not for upstreaming to WPT.

I generally feel comfortable to use testharness.js for any layout tests and
have used it for Chromium/Blink-specific tests these days.

For Shadow DOM v1, we have LayoutTests/shadow-dom and "crash" subdirectory which is
specific to Blink (tests against assertion hit etc.). Custom Elements V1 tests
(LayoutTests/custom-elements), there is a "spec" directory specifically for spec-compliant
tests and could be upstreamed.

It seems ServiceWorker has similar but differently named subdirectory,
so I hope we have one convention that each sub-project can follow for consistency.

--
Takayoshi Kochi

Vincent Scheib

unread,
Nov 4, 2016, 5:23:12 PM11/4/16
to Takayoshi Kochi, Matt Falkenhagen, Victor Costan, Dirk Pranke, blink-dev
I'd prefer to trend towards testharness.js tests, even if needing to expose controls not available today in testharness:
1) Less refactoring should testharness ever gain similar capabilities.
2) External teams will copy tests without chromium developers upstreaming them. (e.g. Web Bluetooth tests copied by Servo)
3) Minimize number of testing frameworks we use over time, trend towards a single future style.


Dirk Pranke

unread,
Nov 14, 2016, 5:56:21 PM11/14/16
to Vincent Scheib, Takayoshi Kochi, Matt Falkenhagen, Victor Costan, blink-dev
I think it's safe to say we've gotten all the feedback we're going to get on this.

From what I see, it looks like there's a leaning to use testharness for new tests regardless of whether they'll be
upstreamed or not. I think that's fine.

Fortunately, it looks like that's also what the docs currently say :):


I'm going to consider this matter closed now. Thanks all!

-- Dirk
Reply all
Reply to author
Forward
0 new messages