New documentation on writing LayoutTests.

docs/testing/writing_layout_tests.md

Index: docs/testing/writing_layout_tests.md
diff --git a/docs/testing/writing_layout_tests.md b/docs/testing/writing_layout_tests.md
new file mode 100644
index 0000000000000000000000000000000000000000..f143945293884b880330dfb52635197ba434daac
--- /dev/null
+++ b/docs/testing/writing_layout_tests.md
@@ -0,0 +1,483 @@
+# Writing Layout Tests
+
+_Layout tests_ is a bit of a misnomer. This term is
+[a part of our WebKit heritage](https://webkit.org/blog/1452/layout-tests-theory/),
+and we use it to refer to every test that is written as a Web page (HTML, SVG,
+or XHTML) and lives in
+[third_party/WebKit/LayoutTests/](../../third_party/WebKit/LayoutTests).
+
+[TOC]
+
+## Overview
+
+Layout tests should be used to accomplish one of the following goals:
+
+1. The entire surface of Blink that is exposed to the Web should be covered by
+   tests that we contribute to the
+   [Web Platform Tests Project](https://github.com/w3c/web-platform-tests)
+   (WPT). This helps us avoid regressions, and gives us confidence that we match

[foolip, 2016/11/16 13:42:38]

To be picky, merely being in WPT doesn't imply much, some tests fail everywhere
or on 3/4 engines. I'd say "and shared tests with other browsers help identify
where interoperability is lacking" or some variation thereof.

[pwnall, 2016/11/18 05:40:43]

Done.
I tried to rephrase. Does this work for you?

[foolip, 2016/11/18 11:30:40]

Wonderful!

+   other browsers' behavior.
+2. When a Blink feature cannot be tested using the Web Platform, and cannot be

[foolip, 2016/11/16 13:42:39]

s/Web Platform/WPT/? There will be many things that are definitely part of the
web platform, but which are currently hard to test using WPT. Input automation
is such a case.

[pwnall, 2016/11/18 05:40:43]

Wouldn't you say these are cases where the Web Platform doesn't provide ways to
test the APIs? For things like input automation, I imagine that WPT might one
day have a WebDriver interface (JS that XHRs to the WPT server, which controls a
WebDriver client running the browser that has the test page open) that resembles
our eventSender, and I imagine that we'll still want to encourage folks to write
tests that stick to Web Platform features (and don't use the Web Driver stuff)
whenever possible.

[foolip, 2016/11/18 11:30:40]

Yes, that's precisely where I think we must go, to standardize enough testing
APIs (as part of WebDriver or whatever makes sense) so that more and more things
can be part of web-platform-tests.

By s/Web Platform/WPT/ I mean that being in WPT isn't synonymous with being
automatable using only on web exposed APIs. The question to ask, I think, is
"can I put this in WPT", and the answer is already yes in a few cases using
wpt_automation.

[pwnall, 2016/11/22 20:32:54]

I was completely unaware of wpt_automation. I added some wording prescribing (1)
Web Platform APIs, (2) WPT testing APIs, (3) Blink-specific APIs. I also added a
stub section with a TODO for WPT APIs, because I couldn't find any
wpt_automation documentation to summarize and link to.

+   easily covered by
+   [C++ unit tests](https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/tests/?q=webframetest&sq=package:chromium&type=cs),
+   the feature must be covered by layout tests, to avoid unexpected regressions.
+   These tests will use Blink-specific testing APIs that are only available in
+   [content_shell](./layout_tests_in_content_shell.md).
+
+### Test Types
+
+There are three broad types of layout tests, listed in the order of preference.

[foolip, 2016/11/16 13:42:38]

There's also the dump render tree kind of test. Not sure where to put it in
order of preference, I guess between reftests and pixel tests?

[pwnall, 2016/11/18 05:40:43]

I have no idea what I'm talking about, because I haven't worked on layout yet...
but isn't it possible to use JavaScript to get (and validate) information that
is equivalent to the DRT output?

[foolip, 2016/11/18 11:30:39]

Not entirely I think, DRT also includes pseudo elements and maybe other things
that aren't web exposed. Whatever the case, there's a lot of these tests. Maybe
ask eae@ for a blurb describing them?

[pwnall, 2016/11/22 20:32:54]

Done.
I did some reading, and wrote a section on DRT tests. Based on my reading (and
experiment, see the code sample), DRT tests produce both an image and a text
render tree dump. This made me decide they're the least desirable tests. 

FWIW, I think that tests that rely on an internal structure would be worse than
pixel tests, even if we could suppress the image output. But we don't need to
have that argument, as it seems like the image output can't be supressed :D

+
+* *JavaScript Tests* are the layout test implementation of
+  [xUnit tests](https://en.wikipedia.org/wiki/XUnit). These tests contain
+  assertions written in JavaScript, and pass if the assertions evaluate to
+  true.
+* *Reference Tests* render a test page and a reference page, and pass if the two
+  renderings are identical, according to a pixel-by-pixel comparison. These
+  tests are less robust, harder to debug, and significantly slower than
+  JavaScript tests, and are only used when JavaScript tests are insufficient,
+  such as when testing layout code.
+* *Pixel Tests* render a test page and compare the result against a pre-rendered
+  image in the repository. Pixel tests are less robust than JavaScript tests and
+  reference tests, because the rendering of a page is influenced by many factors
+  such as the host computer's graphics card and driver, the platform's text
+  rendering system, and various user-configurable operating system settings.
+  For this reason, it is not uncommon for a pixel test to have a different
+  reference image for each platform that Blink is tested on. Pixel tests are
+  least preferred, because the reference images are
+  [quite cumbersome to manage](./layout_test_expectations.md).
+
+## General Principles
+
+The principles below are adapted from
+[Test the Web Forward's Test Format Guidelines](http://testthewebforward.org/docs/test-format-guidelines.html)
+and
+[WebKit's Wiki page on Writing good test cases](https://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree).
+
+* Tests should be as **short** as possible. The page should only include
+  elements that are necessary and relevant to what is being tested.
+
+* Tests should be as **fast** as possible. Blink has several thousand layout
+  tests that are run in parallel, and avoiding unnecessary delays is crucial to
+  keeping our Commit Queue in good shape.
+  * Avoid [window.setTimeout](https://developer.mozilla.org/en-US/docs/Web/API/WindowTimers/setTimeout),
+    as it wastes testing time and can introduce flakiness. Instead, use specific
+    event handlers, such as
+    [window.onload](https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onload).

[foolip, 2016/11/16 13:42:39]

FWIW, when setTimeout is used it's often because there is no event, or you want
to check that an event doesn't fire. Maybe copy/move the "Writing reliable
layout tests" link here? It's a great document I think.

[pwnall, 2016/11/18 05:40:43]

Done.
Ideally, the document should be turned into a Markdown file in the docs/ folder,
or should be absorbed in here. The Google Doc is more difficult to update in a
collaborative fashion. That is definitely future work.

[foolip, 2016/11/18 11:30:40]

Yep, that'd be great, some day.

+
+* Tests should be **self-describing**, so that a project member can recognize
+  whether a test passes or fails without having to read the specification of the
+  feature being tested. `testharness.js` makes a test self-describing when used
+  correctly, but tests that degrade to manual tests
+  [must be carefully designed](http://testthewebforward.org/docs/test-style-guidelines.html)
+  to be self-describing.
+
+* Tests should use the **minimal** set of platform features needed to express
+  the test scenario efficiently.
+    * Avoid depending on edge case behavior of features that aren't explicitly
+      covered by the test. For example, except where testing parsing, tests
+      should contain valid markup (no parsing errors).
+    * Prefer JavaScript's `===` operator to `==`, so that readers don't have to
+      reason about type conversion.
+
+* Tests should be as **cross-platform** as reasonably possible. Avoid
+  assumptions about device type, screen resolution, etc. Unavoidable assumptions
+  should be documented.
+    * When possible, tests should only use Web platform features, as specified
+      in the relevant standards.
+    * Test pages should use the HTML5 doctype (`<!doctype html>`) unless they
+      are testing the quirks mode.
+    * Tests should be written under the assumption that they will be upstreamed

[foolip, 2016/11/16 13:42:39]

\o/

[pwnall, 2016/11/18 05:40:44]

Agreed!

+      to the WPT project. For example, tests should follow the
+      [WPT guidelines](http://testthewebforward.org/docs/writing-tests.html).
+    * Tests that use Blink-specific testing APIs should feature-test for the

[foolip, 2016/11/16 13:42:38]

This isn't something I've ever asked for in code review and when writing tests
using testharness.js and internals I've just made them fail if run without
internals. To add manual steps might require making a sync test async, and I
personally think the "click here, things should be green" and similar is clutter
in what is actually an automated tests. Perhaps opinions differ here :)

[pwnall, 2016/11/18 05:40:44]

I was definitely asked to do this in (perhaps too old?) code reviews... that's
how I learned the technique :)

I think that the manual test aspect is helpful for debugging and for quickly
checking if we match other browsers. Given my past experience, I think we should
document whatever the outcome of this decision is -- if we decide we don't want
manual tests, I'd like to include notes/promos in the relevant places stating
that manual tests used to be a thing but are no longer mandatory.

[foolip, 2016/11/18 11:30:40]

I guess consult blink-dev on this question? Leaving it to CL author and reviewer
judgement seems like it might work.

Note that there are plenty APIs in Internals.idl where no user action could do
the same thing, input automation might be the exception rather than the rule.

+      presence of the testing APIs and degrade to
+      [manual tests](http://testthewebforward.org/docs/manual-test.html)
+      when the testing APIs are not present.
+
+* Tests must be **self-contained** and not depend on external network resources.
+  Unless used by multiple test files, CSS and JavaScript should be inlined using
+  `<style>` and `<script>` tags. Content shared by multiple tests should be
+  placed in a `resources/` directory near the tests that share it. See below for
+  using multiple origins in a test.
+
+* Test **file names** should describe what is being tested. File names should
+  use `snake-case`, but preserve the case of any embedded Web Platform names.

[foolip, 2016/11/16 13:42:39]

s/Web Platform/API/ perhaps.

[pwnall, 2016/11/18 05:40:44]

Done.

+  For example, prefer `document-createElement.html` to
+  `document-create-element.html`.
+
+* Tests must use the UTF-8 **character encoding**, which should be declared by

[foolip, 2016/11/16 13:42:38]

Most tests are in ASCII, so I actually ask people to remove <meta charset=utf-8>
when it won't affect the test.

Looks like the HTML/CSS Style Guide below actually asks for <meta
charset="utf-8">. Could we say that it's optional for a self-contained test that
is obviously ASCII and doesn't include anything that's non-ASCII? At least
without a tool to complain about it, I'm not so keen on asking people to add it
in reviews.

[pwnall, 2016/11/18 05:40:43]

Firefox Nightly now issues a warning when a page doesn't have the charset
specified. This was my motivation for removing the pure ASCII exception that was
in a previous version of this CL.

[foolip, 2016/11/18 11:30:40]

I think I'd still like the exception, but again perhaps seek wider feedback.

[pwnall, 2016/11/22 20:32:54]

Done.
I put the exception in for now, while I seek wider feedback.

+  `<meta charset=utf-8>`. This does not apply when specifically testing
+  encodings.
+
+* Tests must aim to have a **coding style** that is consistent with
+  [Google's JavaScript Style Guide](https://google.github.io/styleguide/javascriptguide.xml),

[foolip, 2016/11/16 13:42:38]

Is there a lint or format tool for this so that people don't have to learn it
nit-by-nit?

[pwnall, 2016/11/18 05:40:44]

AFAIK that is not yet the case, but I hear that clang-format is getting better
at JavaScript?

I still think there's value to us agreeing on a format before we have the tools
to enforce it. I'd much rather have a guide I can read (and people to help me),
as opposed to loose rules that leave me guessing what my reviewer would like to
see.

[foolip, 2016/11/18 11:30:40]

Acknowledged.

+  and
+  [Google's HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.xml),
+  with the following exceptions.
+    * Rules related to Google Closure and JSDoc do not apply.
+    * Modern Web Platform and JavaScript features should be preferred to legacy

[foolip, 2016/11/16 13:42:39]

Note that this is can be slightly at odds with upstreaming to WPT. We shouldn't
gratuitously use unrelated features that aren't yet in the stable versions of
all browsers, as that could obscure real problems.

Some judgement will be needed here. Asking test writers to always be in touch
with the state of interop is a bit much, but maybe "Use of new features
available in all major engines is OK. For example, ..."?

[pwnall, 2016/11/18 05:40:43]

Done.
Thank you for reminding me about this balancing act! I pulled the idea to its
own principle and populated it a bit more.

[foolip, 2016/11/18 11:30:40]

Great, these and the above changes get the message across clearly I think.

+      constructs that target old browsers. For example, prefer `const` and `let`
+      to `var`, and prefer `class` over other OOP constructs.
+    * Concerns regarding buggy behavior in legacy browsers do not apply. For
+      example, the garbage collection cycle note in the _Closures_ section does
+      not apply.
+    * Per the JavaScript guide, new tests should also follow any per-project
+      style guide, such as the
+      [ServiceWorker Tests Style guide](http://www.chromium.org/blink/serviceworker/testing).
+
+## JavaScript Tests
+
+Whenever possible, the testing criteria should be expressed in JavaScript. The
+alternatives, which will be described in future sections, result in slower and
+less robust tests.
+
+All new JavaScript tests should be written using the
+[testharness.js](https://github.com/w3c/testharness.js/) testing framework. This
+framework is used by the tests in the
+[web-platform-tests](https://github.com/w3c/web-platform-tests) repository,
+which is shared with all the other browser vendors, so `testharness.js` tests
+are more accessible to browser developers.
+
+As a shared framework, `testharness.js` enjoys high-quality documentation, such
+as [a tutorial](http://testthewebforward.org/docs/testharness-tutorial.html) and
+[API documentation](https://github.com/w3c/testharness.js/blob/master/docs/api.md).
+Layout tests should follow the recommendations of the above documents.
+Furthermore, layout tests should include relevant
+[metadata](http://testthewebforward.org/docs/css-metadata.html). The
+specification URL (in `<link rel="help">`) is almost always relevant, and is
+incredibly helpful to a developer who needs to understand the test quickly.
+
+Below is a skeleton for an HTML5 JavaScript test. Note that, in order to follow

[foolip, 2016/11/16 13:42:39]

Drop "HTML5", just "JavaScript test" seems OK. (https://html.spec.whatwg.org/
has no version any longer.)

[pwnall, 2016/11/18 05:40:44]

Done.
I rephrased to JavaScript test embedded in HTML page. I wanted to make it clear
(for readers who care) that this isn't a valid skeleton for an XHTML page.

+the minimality guideline, the test omits the tags `<html>`, `<head>` and
+`<body>`, as they can be inferred by the HTML5 parser.
+
+```html
+<!doctype html>
+<meta charset="utf-8">
+<title>JavaScript: the true literal</title>
+<link rel="help"
+      href="http://www.ecma-international.org/ecma-262/6.0/#sec-terms-and-definitions-boolean-value">

[foolip, 2016/11/16 13:42:39]

https://tc39.github.io/ecma262/#sec-boolean-literals, although however you put
it this'll be a weird test, because testharness.js itself uses the true literal
to test it, so the tests amount to (true === true) === true, but I guess that's
OK.

[pwnall, 2016/11/18 05:40:43]

Done.
Thank you very much for the shorter link!

I agree with the weirdness. FWIW, I upgraded an old example, because it seemed
minimal. Do you have any idea for what other concept would be better here?
Something like 1 + 1 === 2?

[foolip, 2016/11/18 11:30:40]

Let's stick with this, presumably literals are already tested somehow, so people
won't copy this to create real tests for the true literal :)

+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+
+// Synchronous test example.
+test(t => {

[foolip, 2016/11/16 13:42:39]

Suggest `() => { }` as the Test instance isn't generally used for sync tests.

[pwnall, 2016/11/18 05:40:43]

Done.

+  const truthy = true;
+  assert_true(truthy, 'true should be truthy');

[foolip, 2016/11/16 13:42:39]

If it failed, this would say "FAIL JavaScript: the true literal assert_true:
true should be truthy expected true got undefined
" so if you rename the variable to value and say 'value' here, it'd be "FAIL
JavaScript: the true literal assert_true: value expected true got undefined"

In other words, if the description is simply a description of the actual value,
the errors make more sense.

However, for very small tests I'd just not have a description, it makes the test
bigger and doesn't help you track down which assertion failed.

[pwnall, 2016/11/18 05:40:43]

Done.
I changed the tests to demonstrate the rule you suggested (no description for
single-assertion tests, description for multi-assertion tests).

I also added an assert_equal(!value, false) because (1) I wanted to show the
actual/expected argument order, and (2) I wanted to have a test where it makes
sense to have assertion descriptions.

+}, 'The literal true in a synchronous test case');

[foolip, 2016/11/16 13:42:39]

This isn't spelled out above, but I think this is great. IMHO, test titles
should not include "Tests ..." or say anything about the pass condition, but
just say the thing that they are testing. If the spec changes to invert the pass
condition, you'd ideally not have to change the test title.

[pwnall, 2016/11/18 05:40:43]

I created a bulleted list below this block of code, where I tried to spell out
this idea, as well as other things that I think aren't obvious on a first read,
but need to be learned to write tests effectively.

+
+// Asynchronous test example.
+async_test(t => {
+  setTimeout(t.step_func(() => {
+    const truthy = true;

[foolip, 2016/11/16 13:42:38]

Testing a true literal like this becomes rather strained. Maybe the test could
pretend that it's checking that the true literal doesn't change over time, by
setting a const originalTruth = true before the timeout, and assert_equals(true,
originalTruth) here?

[pwnall, 2016/11/18 05:40:43]

Done.
This does look better. Thank you!

+    assert_true(truthy, 'true should be truthy');
+  }), 0);
+  t.done();

[foolip, 2016/11/16 13:42:39]

This test will always pass because this is run before the timeout runs, and the
first result is what counts. Suggest instead using t.step_func_done.

[pwnall, 2016/11/18 05:40:43]

Done. (pun not intended)
Wow, that error was pretty terrible! Thanks so much for catching it! I know how
to JS, I promise :)
I added a comment that introduces step_func() and done(), so folks don't blindly
build on the example and end up with multiple step_func_done() in a test.

[foolip, 2016/11/18 11:30:40]

I totally believe that your promise resolves to true!

+}, 'The literal true in a setTimeout callback');
+
+// Promise test example.
+promise_test(t => {

[foolip, 2016/11/16 13:42:39]

() here too since t isn't used.

[pwnall, 2016/11/18 05:40:44]

Done.

+  return new Promise((resolve, reject) => {
+    resolve(true);
+  }).then(truthy => {

[foolip, 2016/11/16 13:42:39]

s/truthy/value/

[pwnall, 2016/11/18 05:40:44]

Done.

+    assert_true(truthy, 'true should be truthy');
+  });
+}, 'The literal true in a Promise');
+
+</script>
+```
+
+### Relying on Blink-Specific Testing APIs
+
+Tests that cannot be expressed using the Web Platform APIs rely on
+Blink-specific testing APIs. These APIs are only available in
+[content_shell](./layout_tests_in_content_shell.md).
+
+### Manual Tests
+
+Whenever possible, tests that rely on Blink-specific testing APIs should also be

[foolip, 2016/11/16 13:42:38]

Maybe drop the similar bit above and have only this. I don't enjoy writing or
reviewing those manual steps, I guess opinions may differ though.

[pwnall, 2016/11/18 05:40:43]

Ack.
I will watch out for more opinions on the manual tests requirement.

+usable as [manual tests](http://testthewebforward.org/docs/manual-test.html).
+This makes it easy to debug the test, and to check whether our behavior matches
+other browsers.
+
+Manual tests should minimize the chance of user error. This implies keeping the
+manual steps to a minimum, and having simple and clear instructions that
+describe all the configuration changes and user gestures that match the effect
+of the Blink-specific APIs used by the test.
+
+Below is an example of a fairly minimal test that uses a Blink-Specific API
+(`window.eventSender`), and gracefully degrades  to a manual test.
+
+```html
+<!doctype html>
+<meta charset="utf-8">
+<title>DOM Events: Event.isTrusted for UI events</title>

[foolip, 2016/11/16 13:42:39]

s/DOM Events/DOM/

[pwnall, 2016/11/18 05:40:44]

Done.

+<link rel="help" href="https://dom.spec.whatwg.org/#dom-event-istrusted">
+<link rel="help" href="https://dom.spec.whatwg.org/#constructing-events">
+<meta name="assert" content="Event.isTrusted value under certain situations">
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+
+<p>Please click on the button below.</p>
+<button id="click-me" type="button">Click Me!</button>

[foolip, 2016/11/16 13:42:39]

id="click-me" and type="button" is not needed, can use document.querySelector
when there's just one button.

[pwnall, 2016/11/18 05:40:43]

Done.
FWIW, I'm a bit wary of using querySelector, because I got super-excited when it
shipped and I'm afraid that I'm biased and querySelector ALL THE THINGS :)

[foolip, 2016/11/18 11:30:39]

Well, I have that bias too, I even use querySelectorAll if it means I can avoid
id="button1" and id="button2" :)

+
+<script>
+
+setup({ explicit_timeout: true });
+
+promise_test(() => {
+  return new Promise((resolve, reject) => {
+    const button = document.getElementById('click-me');
+    button.addEventListener('click', (event) => {
+      clickEvent = event;
+      resolve(event);
+    });
+
+    if (window.eventSender) {
+      eventSender.mouseMoveTo(button.offsetLeft, button.offsetTop);
+      eventSender.mouseDown();
+      eventSender.mouseUp();
+    }
+  }).then((clickEvent) => {
+    assert_equals(true, clickEvent.isTrusted,

[foolip, 2016/11/16 13:42:39]

Expected and actual are reversed here, but just assert_true would work. And no
description at all would be better IMHO :)

[pwnall, 2016/11/18 05:40:43]

Done.
Wow, I failed at a point that I was trying to make. I now use the correct order
(I hope) above, and document the order explicitly in a bullet point.

[foolip, 2016/11/18 11:30:40]

It's very good to have clear documentation on this, the historical note at the
end of
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#bi...
is probably why the other order comes naturally to many Chromium developers.

+        'User interaction events should have isTrusted set to true');
+  });
+
+}, 'Click generated by user interaction');

[foolip, 2016/11/16 13:42:39]

Omit test description in single-test files with a <title> that works for the
test title.

[pwnall, 2016/11/18 05:40:43]

I also made this explicit in a new bulleted list above.

+
+</script>
+```
+
+The test exhibits the following desirable features:
+
+* It has a second specification URL (`<link rel="help">`), because the paragraph
+  that documents the tested feature (referenced by the primary URL) is not very
+  informative on its own.
+* It links to the
+  [WHATWG Living Standard](https://wiki.whatwg.org/wiki/FAQ#What_does_.22Living_Standard.22_mean.3F),
+  rather than to a frozen version of the specification.
+* It uses relative paths to point to

[foolip, 2016/11/16 13:42:39]

It's not possible to get this wrong outside of LayoutTests/imported/wpt/ is it?
And there you'll have to use absolute paths, so I'd suggest dropping this point
and figuring out how to make manual testing easier for that case.

[pwnall, 2016/11/18 05:40:43]

Done.
I described relative paths as a limitation of our test runner. I don't have
enough cycles to work on infrastructure improvements, I can only document the
current state. I was advised to write this from the perspective of where we'd
want to be in a year, and I think I overshot by a bit :)

[foolip, 2016/11/18 11:30:40]

:)

+  [/references/testharness.js](../../third_party/WebKit/LayoutTests/references/testharness.js)
+  and
+  [/references/testharnessreport.js](../../third_party/WebKit/LayoutTests/references/testharnessreport.js),
+  so a developer can iterate on the test simply by opening its file in a
+  browser, without having to start a HTTP server. This is only relevant for
+  tests that work from `file://` origins, and tests that require a HTTP server
+  should use absolute paths.
+* It documents its assertions clearly.
+    * The `<meta name="assert">` describes the purpose of the entire file.

[foolip, 2016/11/16 13:42:39]

Can you throw on something like "Only use when it adds information that isn't
already in the title"? I've personally rarely found them useful.

[pwnall, 2016/11/18 05:40:44]

Done.

+    * The `assert_equals` string describes the expected behavior, not the error.

[foolip, 2016/11/16 13:42:39]

The expected value (second argument) describes the expected behavior, so the
description need only name the actual value.

[pwnall, 2016/11/18 05:40:43]

Done.

+    * Each test case describes the circumstance that it tests.
+* It contains clear instructions for manually triggering the test conditions.
+  The test starts with a paragraph (`<p>`) that tells the tester exactly what to
+  do, and the `<button>` that needs to be clicked is clearly labeled.
+* It disables the timeout mechanism built into `testharness.js` by calling
+  `setup({ explicit_timeout: true });`
+* It checks for the presence of the Blink-specific testing APIs
+  (`window.eventSender`) before invoking them. The test does not automatically
+  fail when the APIs are not present.
+* It uses [Promises](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Promise)

[foolip, 2016/11/16 13:42:39]

This test I would argue would be smaller and clearer as an async_test:

async_test(t => {
  const button = document.querySelector('button');
  button.addEventListener('click', t.step_func_done(event => {
    assert_true(event.isTrusted);
  });
  /* stuff with internals */
});

[pwnall, 2016/11/18 05:40:44]

I agree. I still think that it's more valuable to teach how this works with
Promises, because they're easier to compose and reason about.

[foolip, 2016/11/18 11:30:40]

OK, I'll take both styles! One benefit of promise_test is a lot less need for
t.step_func wrapping.

+  to separate the test setup from the assertions. This is particularly helpful
+  for manual tests that depend on a sequence of events to occur, as Promises
+  offer a composable way to express waiting for asynchronous events that avoids
+  [callback hell](http://stackabuse.com/avoiding-callback-hell-in-node-js/).
+
+Notice that the test is pretty heavy compared to a minimal JavaScript test that
+does not rely on testing APIs. Only use Blink-specific testing APIs when the
+desired testing conditions cannot be set up using Web Platform APIs.
+
+#### Using Blink-Specific Testing APIs
+
+A downside of Blink-specific APIs is that they are not as well documented as the
+Web Platform features. Learning to use a Blink-specific feature requires finding
+other tests that use it, or reading its source code.
+
+For example, the most popular Blink-specific API is `testRunner`, which is
+implemented in
+[components/test_runner/test_runner.h](../../components/test_runner/test_runner.h)
+and
+[components/test_runner/test_runner.cpp](../../components/test_runner/test_runner.cpp).
+By skimming the `TestRunnerBindings::Install` method, we learn that the
+testRunner API is presented by the `window.testRunner` and
+`window.layoutTestsController` objects, which are synonyms. Reading the
+`TestRunnerBindings::GetObjectTemplateBuilder` method tells us what properties
+are available on the `window.testRunner` object.
+
+*** aside
+`window.testRunner` is the preferred way to access the `testRunner` APIs.
+`window.layoutTestsController` is still supported because it is used by
+3rd-party tests.
+***
+
+*** note
+`testRunner` is the most popular testing API because it is also used indirectly
+by tests that stick to Web Platform APIs. The `testharnessreport.js` file in
+`testharness.js` is specifically designated to hold glue code that connects
+`testharness.js` to the testing environment. Our implementation is in
+[third_party/WebKit/LayoutTests/resources/testharnessreport.js](../../third_party/WebKit/LayoutTests/references/testharnessreport.js),
+and uses the `testRunner` API.
+***
+
+See the [components/test_runner/](../../components/test_runner/) directory and
+[WebKit's LayoutTests guide](https://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree)
+for other useful APIs. For example, `window.eventSender`
+([components/test_runner/event_sender.h](../../components/test_runner/event_sender.h)
+and
+[components/test_runner/event_sender.cpp](../../components/test_runner/event_sender.cpp))
+has methods that simulate events input such as keyboard / mouse input and
+drag-and-drop.
+
+Here is a UML diagram of how the `testRunner` bindings fit into Chromium.
+
+[![UML of testRunner bindings configuring platform implementation](https://docs.google.com/drawings/u/1/d/1KNRNjlxK0Q3Tp8rKxuuM5mpWf4OJQZmvm9_kpwu_Wwg/export/svg?id=1KNRNjlxK0Q3Tp8rKxuuM5mpWf4OJQZmvm9_kpwu_Wwg&pageid=p)](https://docs.google.com/drawings/d/1KNRNjlxK0Q3Tp8rKxuuM5mpWf4OJQZmvm9_kpwu_Wwg/edit)
+
+### Text Test Baselines
+
+By default, all the test cases in a file that uses `testharness.js` are expected
+to pass. However, in some cases, we prefer to add failing test cases to the
+repository, so that we can be notified when the failure modes change (e.g., we
+want to know if a test starts crashing rather than returning incorrect output).
+In these situations, a test file will be accompanied by a baseline, which is an
+`-expected.txt` file that contains the test's expected output.
+
+The baselines  are generated automatically when appropriate by

[foolip, 2016/11/16 13:42:39]

double space

[pwnall, 2016/11/18 05:40:44]

Done.

+`run-webkit-tests`, which is described [here](./layout_tests.md), and by the
+[rebaselining tools](./layout_test_expectations.md).
+
+Text baselines should be very rare. In general, layout tests should use

[foolip, 2016/11/16 13:42:38]

As noted above, this'll be at odds with the desire to upstream more tests. If
writing with upstreaming in mind, I'd argue that we should test spec behavior
and live with the expected files, taking care to see that failures don't cascade
and hide other problems.

[pwnall, 2016/11/18 05:40:43]

Done.
I rewrote this paragraph. It now advises for text baselines (+crbug.com issues
tracking their removal) for WPT tests, and current behavior tests for non-WPT
tests. 

[foolip, 2016/11/18 11:30:40]

Acknowledged.

+JavaScript to document Blink's current behavior, rather than using JavaScript to
+document desired behavior and a text file to document current behavior.
+
+### The js-test.js Legacy Harness
+
+*** promo
+For historical reasons, older tests are written using the `js-test` harness.
+This harness is **deprecated**, and should not be used for new tests.
+***
+
+If you need to understand old tests, the best `js-test` documentation is its
+implementation at
+[third_party/WebKit/LayoutTests/resources/js-test.js](../../third_party/WebKit/LayoutTests/resources/js-test.js).
+
+`js-test` tests lean heavily on the Blink-specific `testRunner` testing API.
+In a nutshell, the tests call `testRunner.dumpAsText()` to signal that the page
+content should be dumped and compared against a text baseline (an
+`-expected.txt` file). As a consequence, `js-test` tests are always accompanied
+by text baselines. Asynchronous tests also use `testRunner.waitUntilDone()` and
+`testRunner.notifyDone()` to tell the testing tools when they are complete.
+
+### Tests that use an HTTP Server
+
+By default, tests are loaded as if via `file:` URLs. Some web platform features
+require tests served via HTTP or HTTPS, for example absolute paths (`src=/foo`)
+or features restricted to secure protocols.
+
+HTTP tests are those tests that are under `LayoutTests/http/tests` (or virtual
+variants). Use a locally running HTTP server (Apache) to run them. Tests are
+served off of ports 8000 and 8080 for HTTP, and 8443 for HTTPS. If you run the
+tests using `run-webkit-tests`, the server will be started automatically. To run
+the server manually to reproduce or debug a failure:
+
+```bash
+cd src/third_party/WebKit/Tools/Scripts
+run-blink-httpd start
+```
+
+The layout tests will be served from `http://127.0.0.1:8000`. For example, to
+run the test `http/tests/serviceworker/chromium/service-worker-allowed.html`,
+navigate to
+`http://127.0.0.1:8000/serviceworker/chromium/service-worker-allowed.html`. Some
+tests will behave differently if you go to 127.0.0.1 instead of localhost, so
+use 127.0.0.1.
+
+To kill the server, run `run-blink-httpd --server stop`, or just use `taskkill`
+or the Task Manager on Windows, and `killall` or Activity Monitor on MacOS.
+
+The test server sets up an alias to `LayoutTests/resources` directory. In HTTP
+tests, you can access the testing framework at e.g.
+`src="/js-test-resources/js-test.js"`.
+
+TODO: Document [wptserve](http://wptserve.readthedocs.io/) when we are in a
+position to use it to run layout tests.
+
+## Reference Tests
+
+TODO: Check that we match the Web Platform's
+[reftests](http://testthewebforward.org/docs/reftests.html), link to their
+document and summarize it.
+
+### Legacy Reference Tests
+
+*** promo
+WebKit-style reftests are deprecated. Please use the WPT style for all new

[foolip, 2016/11/16 13:42:39]

Right now this isn't possible, and the WPT style is converted to *-expected.html
on import. So I guess a TODO to update this documentation when we have that
sorted.

[pwnall, 2016/11/18 05:40:43]

Done.
I have been trying to avoid writing this section, because I have zero experience
with reference tests. I rephrased the section to reflect the current reality and
state where we aim to be in the near (< 1 year?) future.

+reference tests that you create.
+***
+
+Blink also has inherited a sizable amount of
+[reftests](https://trac.webkit.org/wiki/Writing%20Reftests) from WebKit. In
+these tests, the reference page file name is based on the test page's file name
+and an `-expected.html` suffix.
+
+## Pixel Tests
+
+`testRunner` APIs such as `window.testRunner.dumpAsTextWithPixelResults()` and
+`window.testRunner.dumpDragImage()` create an image result that is associated
+with the test. The image result is compared against an image baseline, which is
+an `-expected.png` file associated with the test, and the test passes if the
+image result is identical to the baseline, according to a pixel-by-pixel
+comparison. Tests that have image results (and baselines) are called **pixel
+tests**.
+
+Pixel tests should still follow the principles laid out above. Pixel tests pose
+unique challenges to the desire to have *self-describing* and *cross-platform*
+tests. The
+[WPT test style guidelines](http://testthewebforward.org/docs/test-style-guidelines.html)
+contain useful guidance. The most relevant pieces of advice are below.
+
+* use a green paragraph / page / square to indicate success
+* use the red color or the word `FAIL` to highlight errors
+* use the [Ahem font](https://www.w3.org/Style/CSS/Test/Fonts/Ahem/README) to
+  minimize the variance introduced by the platform's text rendering system
+
+The following snippet includes the Ahem font in a layout test.
+
+```html
+<style>
+body {
+  font: 10px Ahem;
+}
+</style>
+<script src="/resources/ahem.js"></script>

[foolip, 2016/11/16 13:42:38]

Looks like this is always included using a relative path.

[pwnall, 2016/11/18 05:40:44]

Done.
I added a note about this.

+```
+
+### Tests that need to paint, raster, or draw a frame of intermediate output
+
+A layout test does not actually draw frames of output until the test exits. If
+it is required to generate a painted frame, then use
+`window.testRunner.displayAsyncThen`, which will run the machinery to put up a
+frame, then call the passed callback. There is also a library at
+`fast/repaint/resources/text-based-repaint.js` to help with writing paint
+invalidation and repaint tests.
+
+## Directory Structure
+
+The [LayoutTests directory](../../third_party/WebKit/LayoutTests) currently
+lacks a strict, formal structure. The following directories have special
+meaning:
+
+* The `http/` directory hosts tests that require a HTTP server (see above).
+* The `resources/` subdirectory in every directory contains binary files, such
+  as media files, and code that is shared by multiple test files.
+
+*** note
+Some layout tests consist of a minimal HTML page that references a JavaScript
+file in `resources/`. Please do not use this pattern for new tests, as it goes
+against the minimality principle. JavaScript and CSS files should only live in
+`resources/` if they are shared by at least two test files.
+***
+
+## See Also
+
+[Writing reliable layout tests](https://docs.google.com/document/d/1Yl4SnTLBWmY1O99_BTtQvuoffP8YM9HZx2YPkEsaduQ/edit)