Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2822)

Unified Diff: docs/testing/layout_test_writing.md

Issue 2492733003: New documentation on writing LayoutTests. (Closed)
Patch Set: Documented test types and legacy reftests. Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | docs/testing/layout_tests.md » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: docs/testing/layout_test_writing.md
diff --git a/docs/testing/layout_test_writing.md b/docs/testing/layout_test_writing.md
new file mode 100644
index 0000000000000000000000000000000000000000..7dd6d0930433ec86722d96171aeb50731a8b2ddf
--- /dev/null
+++ b/docs/testing/layout_test_writing.md
@@ -0,0 +1,453 @@
+# Writing Layout Tests
qyearsley 2016/11/15 18:40:56 Is this file just called layout_test_writing.md so
pwnall 2016/11/16 01:50:39 Done. You guessed my intention correctly :) I thou
+
+_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,
qyearsley 2016/11/15 18:40:56 I notice that Web is capitalized in a few places h
pwnall 2016/11/16 01:50:39 Web is capitalized on Wikipedia and NY Times. This
foolip 2016/11/16 11:59:53 https://www.chromium.org/blink uses lowercase "web
qyearsley 2016/11/16 18:35:45 For some reason I never realized that it was somet
+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
+ other browsers' behavior.
+2. When a Blink feature cannot be tested using the Web Platform, and cannot be
+ 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.
+
+* *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.
qyearsley 2016/11/15 18:40:56 Could note here that there are two different JS li
pwnall 2016/11/16 01:50:39 I'd rather not. js-test.js is part of our WebKit l
foolip 2016/11/16 11:59:53 This section doesn't mention specifics, but below
qyearsley 2016/11/16 18:35:44 Yep, sounds good to me.
+* *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).
+
+* 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
qyearsley 2016/11/15 18:40:56 Nit: "efficiently" may be unnecessary here.
pwnall 2016/11/16 01:50:39 I put it in there because I wanted to say that it'
foolip 2016/11/16 11:59:53 Makes sense to me. If you're waiting for many cond
qyearsley 2016/11/16 18:35:44 Sounds good. This is nitpicky, but I'm still not s
pwnall 2016/11/22 20:32:54 Done. I like concisely. Thank you! FWIW, "efficie
+ features that aren't explicitly covered by the test. For example, except where
+ testing parsing, tests should contain no parse errors.
+
+* 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.
+ * Tests should be written under the assumption that they will be upstreamed
+ 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
+ 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.
+
+* Tests must stick to pure ASCII or use the UTF-8 **character encoding**, which
+ should be declared by `<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),
+ 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
+ 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.
qyearsley 2016/11/15 18:40:56 Also note there's a doc about layout test style gu
pwnall 2016/11/16 01:50:39 Thank you for the link! I either completely forgot
+
+## 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 a JavaScript test. Note that, in order to follow 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">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+
+// Synchronous test example.
+test(t => {
+ const truthy = true;
+ assert_true(truthy, 'true should be truthy');
+}, 'The literal true in a synchronous test case');
+
+// Asynchronous test example.
+async_test(t => {
+ setTimeout(t.step_func(() => {
+ const truthy = true;
+ assert_true(truthy, 'true should be truthy');
+ }), 0);
+ t.done();
+}, 'The literal true in a setTimeout callback');
+
+// Promise test example.
+promise_test(t => {
+ return new Promise((resolve, reject) => {
+ resolve(true);
+ }).then(truthy => {
+ 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
+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>
+<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" />
qyearsley 2016/11/15 18:40:56 I think there's no need to use /> to close meta an
jeffcarp 2016/11/15 19:08:17 Just to back this up, the Chromium style says not
pwnall 2016/11/16 01:50:39 Thanks for pointing me to that! I added a link to
pwnall 2016/11/16 01:50:39 Done. Thanks so much for catching this! Having exa
+<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>
+
+<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,
+ 'User interaction events should have isTrusted set to true');
+ });
+
+}, 'Click generated by user interaction');
+
+</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
+ [/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.
+ * The `assert_equals` string describes the expected behavior, not the error.
+ * 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)
+ 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 Expectations
+
qyearsley 2016/11/15 18:40:56 I've been referring to all -expected.* files as "b
pwnall 2016/11/16 01:50:39 Done. Thank you very much for bringing this up! Co
qyearsley 2016/11/16 18:35:44 Yeah, I think that the current state of affairs is
+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 an `-expected.txt` file,
+which documents the test's expected output.
+
+The `-expected.txt` files are generated automatically when appropriate by
+`run-webkit-tests`, which is described [here](./layout_tests.md), and by the
+[rebaselining tools](./layout_test_expectations.md).
+
+`-expected.txt` files should be very rare. In general, layout tests should
+use JavaScript to document Blink's current behavior, rather than using
+JavaScript to document desired behavior and a text file to document current
+behavior.
qyearsley 2016/11/15 18:40:56 Although, web platform tests test conformance to a
pwnall 2016/11/16 01:50:39 I think this is an exception. WPT tests are import
foolip 2016/11/16 11:59:53 I think it is still worth pointing out somehow. As
pwnall 2016/11/22 20:32:54 Done.
+
+### js-test tests
qyearsley 2016/11/15 18:40:56 Possible alternate section title: "JavaScript Te
pwnall 2016/11/16 01:50:39 I changed the section heading. Thank you for highl
+
+*** promo
qyearsley 2016/11/15 18:40:56 What is "promo" here?
pwnall 2016/11/16 01:50:39 It's a gittiles-specific thing. https://gerrit.goo
qyearsley 2016/11/16 18:35:44 Ah, thanks
+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 an `-expected.txt` file. As a
+consequence, `js-test` tests are always accompanied by an `-expected.txt`.
+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
+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 expectation, turning the
+test into a **pixel test**. These tests have associated `-expected.png` image
+files.
+
+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>
+```
+
+### 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.
+
+## See Also
+
+[Writing reliable layout tests](https://docs.google.com/document/d/1Yl4SnTLBWmY1O99_BTtQvuoffP8YM9HZx2YPkEsaduQ/edit)
« no previous file with comments | « no previous file | docs/testing/layout_tests.md » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698