Chromium Code Reviews| OLD | NEW |
|---|---|
| (Empty) | |
| 1 # Layout Tests Tips | |
| 2 | |
| 3 *** note | |
| 4 This document intentionally uses _should_ a lot more than _must_, as defined in | |
| 5 [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt). Writing layout tests is a | |
| 6 careful act of balancing many concerns, and this humble document cannot possibly | |
| 7 capture the context that rests in the head of an experienced Blink engineer. | |
| 8 *** | |
| 9 | |
| 10 ## General Principles | |
|
jsbell
2017/01/20 19:25:21
I'd add a comment to the effect that blink's layou
pwnall
2017/01/21 01:15:50
Done.
Argh, thank you for catching this! I intend
| |
| 11 | |
| 12 This section contains guidelines adopted from | |
| 13 [Test the Web Forward's Test Format Guidelines](http://testthewebforward.org/doc s/test-format-guidelines.html) | |
| 14 and | |
| 15 [WebKit's Wiki page on Writing good test cases](https://trac.webkit.org/wiki/Wri ting%20Layout%20Tests%20for%20DumpRenderTree), | |
| 16 with Blink-specific flavoring. | |
| 17 | |
| 18 ### Concise | |
| 19 | |
| 20 Tests should be **concise**, without compromising on the principles below. Every | |
| 21 element and piece of code on the page should be necessary and relevant to what | |
| 22 is being tested. For example, don't build a fully functional signup form if you | |
| 23 only need a text field or a button. | |
| 24 | |
| 25 Content needed to satisfy the principles below is considered necessary. For | |
| 26 example, it is acceptable and desirable to add elements that make the test | |
| 27 self-describing (see below), and to add code that makes the test more reliable | |
| 28 (see below). | |
| 29 | |
| 30 Content that makes test failures easier to debug is considered necessary (to | |
| 31 maintaining a good development speed), and is both acceptable and desirable. | |
| 32 | |
| 33 *** promo | |
| 34 Conciseness is particularly important for reference tests and pixel tests, as | |
| 35 the test pages are rendered in an 800x600px viewport. Having content outside the | |
| 36 viewport is undesirable because the outside content does not get compared, and | |
| 37 because the resulting scrollbars are platform-specific UI widgets, making the | |
| 38 test results less reliable. | |
| 39 *** | |
| 40 | |
| 41 ### Fast | |
| 42 | |
| 43 Tests should be as **fast** as possible, without compromising on the principles | |
| 44 below. Blink has several thousand layout tests that are run in parallel, and | |
| 45 avoiding unnecessary delays is crucial to keeping our Commit Queue in good | |
| 46 shape. | |
| 47 | |
| 48 Avoid | |
| 49 [window.setTimeout](https://developer.mozilla.org/en-US/docs/Web/API/WindowTimer s/setTimeout), | |
| 50 as it wastes time on the testing infrastructure. Instead, use specific event | |
| 51 handlers, such as | |
| 52 [window.onload](https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHand lers/onload), | |
| 53 to decide when to advance to the next step in a test. | |
| 54 | |
| 55 ### Reliable | |
| 56 | |
| 57 Tests should be **reliable** and yield consistent results for a given | |
| 58 implementation. Flaky tests slow down your fellow developers' debugging efforts | |
| 59 and the Commit Queue. | |
| 60 | |
| 61 `window.setTimeout` is again a primary offender here. Asides from wasting time | |
| 62 on a fast system, tests that rely on fixed timeouts can fail when on systems | |
| 63 that are slower than expected. | |
| 64 | |
| 65 When adding or significantly modifying a layout test, use the command below to | |
| 66 assess its flakiness. While not foolproof, this approach gives you some | |
| 67 confidence, and giving up CPU cycles for mental energy is a pretty good trade. | |
| 68 | |
| 69 ```bash | |
| 70 third_party/WebKit/Tools/Scripts/run-webkit-tests path/to/test.html --repeat-eac h=100 | |
| 71 ``` | |
| 72 | |
| 73 The | |
| 74 [PSA on writing reliable layout tests](https://docs.google.com/document/d/1Yl4Sn TLBWmY1O99_BTtQvuoffP8YM9HZx2YPkEsaduQ/edit). | |
| 75 also has good guidelines for writing reliable tests. | |
| 76 | |
| 77 ### Self-Describing | |
| 78 | |
| 79 Tests should be **self-describing**, so that a project member can recognize | |
| 80 whether a test passes or fails without having to read the specification of the | |
| 81 feature being tested. | |
| 82 | |
| 83 `testharness.js` makes a test self-describing when used correctly. Other types | |
| 84 of tests, such as reference tests and | |
| 85 [tests with manual fallback](./layout_tests_with_manual_fallback.md), | |
| 86 [must be carefully designed](http://testthewebforward.org/docs/test-style-guidel ines.html) | |
| 87 to be self-describing. | |
| 88 | |
| 89 ### Minimal | |
| 90 | |
| 91 Tests should require a **minimal** amount of cognitive effort to read and | |
| 92 maintain. | |
| 93 | |
| 94 Avoid depending on edge case behavior of features that aren't explicitly covered | |
| 95 by the test. For example, except where testing parsing, tests should contain | |
| 96 valid markup (no parsing errors). | |
| 97 | |
| 98 Tests should provide as much relevant information as possible when failing. | |
| 99 `testharness.js` tests should prefer | |
| 100 [rich assert_ functions](https://github.com/w3c/testharness.js/blob/master/docs/ api.md#list-of-assertions) | |
| 101 to combining `assert_true()` with a boolean operator. Using appropriate | |
| 102 `assert_` functions results in better diagnostic output when the assertion | |
| 103 fails. | |
| 104 | |
| 105 ### Cross-Platform | |
| 106 | |
| 107 Tests should be as **cross-platform** as reasonably possible. Avoid assumptions | |
| 108 about device type, screen resolution, etc. Unavoidable assumptions should be | |
| 109 documented. | |
| 110 | |
| 111 When possible, tests should only use Web platform features, as specified | |
| 112 in the relevant standards. When the Web platform's APIs are insufficient, | |
| 113 tests should prefer to use WPT extended testing APIs, such as | |
| 114 `wpt_automation`, over Blink-specific testing APIs. | |
| 115 | |
| 116 Test pages should use the HTML5 doctype (`<!doctype html>`) unless they | |
| 117 specifically cover | |
| 118 [quirks mode](https://developer.mozilla.org/docs/Quirks_Mode_and_Standards_Mode) | |
| 119 behavior. | |
| 120 | |
| 121 Tests should avoid using features that haven't been shipped by the | |
| 122 actively-developed major rendering engines (Blink, WebKit, Gecko, Edge). When | |
| 123 unsure, check [caniuse.com](http://caniuse.com/). By necessity, this | |
| 124 recommendation does not apply to the feature targeted by the test. | |
| 125 | |
| 126 *** note | |
| 127 It may be tempting have a test for a bleeding-edge feature X depend on feature | |
| 128 Y, which has only shipped in beta / development versions of various browsers. | |
| 129 The reasoning would be that all browsers that implement X will have implemented | |
| 130 Y. Please keep in mind that Chrome has un-shipped features that made it to the | |
| 131 Beta channel in the past. | |
| 132 *** | |
| 133 | |
| 134 *** aside | |
| 135 [ES2015](http://benmccormick.org/2015/09/14/es5-es6-es2016-es-next-whats-going-o n-with-javascript-versioning/) | |
| 136 is shipped by all major browsers under active development (except for modules), | |
| 137 so using ES2015 features is acceptable. | |
| 138 | |
| 139 At the time of this writing, ES2016 is not fully shipped in all major browsers. | |
| 140 *** | |
| 141 | |
| 142 ### Self-Contained | |
| 143 | |
| 144 Tests must be **self-contained** and not depend on external network resources. | |
| 145 | |
| 146 Unless used by multiple test files, CSS and JavaScript should be inlined using | |
| 147 `<style>` and `<script>` tags. Content shared by multiple tests should be | |
| 148 placed in a `resources/` directory near the tests that share it. See below for | |
| 149 using multiple origins in a test. | |
| 150 | |
| 151 ### File Names | |
| 152 | |
| 153 Test **file names** should describe what is being tested. | |
| 154 | |
| 155 File names should use `snake-case`, but preserve the case of any embedded API | |
| 156 names. For example, prefer `document-createElement.html` to | |
| 157 `document-create-element.html`. | |
| 158 | |
| 159 ### Character Encoding | |
| 160 | |
| 161 Tests should use the UTF-8 **character encoding**, which should be declared by | |
| 162 `<meta charset=utf-8>`. A `<meta>` tag is not required (but is acceptable) for | |
| 163 tests that only contain ASCII characters. This guideline does not apply when | |
| 164 specifically testing encodings. | |
| 165 | |
| 166 The `<meta>` tag must be the first child of the document's `<head>` element. In | |
| 167 documents that do not have an explicit `<head>`, the `<meta>` tag must follow | |
| 168 the doctype. | |
| 169 | |
| 170 ## Coding Style | |
| 171 | |
| 172 No coding style is enforced for layout tests. This section highlights coding | |
| 173 style aspects that are not consistent across our layout tests, and suggests some | |
| 174 defaults for unopinionated developers. When writing layout tests for a new part | |
| 175 of the codebase, you can minimize review latency by taking a look at existing | |
| 176 tests, and pay particular attention to these issues. Also beware of per-project | |
| 177 style guides, such as the | |
| 178 [ServiceWorker Tests Style guide](https://www.chromium.org/blink/serviceworker/t esting). | |
| 179 | |
| 180 ### Baseline | |
| 181 | |
| 182 [Google's JavaScript Style Guide](https://google.github.io/styleguide/jsguide.ht ml) | |
| 183 and | |
| 184 [Google's HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide .xml) | |
| 185 are a reasonable baseline for coding style defaults, with the caveat that layout | |
| 186 tests do not use Google Closure or JSDoc. | |
| 187 | |
| 188 ### == vs === | |
| 189 | |
| 190 JavaScript's | |
| 191 [== operator](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Operat ors/Comparison_Operators#Equality_()) | |
| 192 performs some | |
| 193 [type conversion](http://www.ecma-international.org/ecma-262/6.0/#sec-abstract-e quality-comparison). | |
| 194 on its arguments, which might be surprising to readers whose experience centers | |
| 195 around C++ or Java. The | |
| 196 [=== operator](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Opera tors/Comparison_Operators#Identity_strict_equality_()) | |
| 197 is much more similar to `==` in C++. | |
| 198 | |
| 199 Using `===` everywhere is an easy default that saves you, your reviewer, and any | |
| 200 colleague that might have to debug test failures, from having to reason about | |
| 201 [special cases for ==](http://dorey.github.io/JavaScript-Equality-Table/). At | |
| 202 the same time, some developers consider `===` to add unnecessary noise when `==` | |
| 203 would suffice. While `===` should be universally accepted, be flexible if your | |
| 204 reviewer expresses a strong preference for `==`. | |
| 205 | |
| 206 ### Let and Const vs Var | |
| 207 | |
| 208 JavaScript variable declarations introduced by | |
| 209 [var](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Statements/var ) | |
| 210 are hoisted to the beginning of their containing function, which may be | |
| 211 surprising to C++ and Java developers. By contrast, | |
| 212 [const](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Statements/c onst) | |
| 213 and | |
| 214 [let](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Statements/let ) | |
| 215 declarations are block-scoped, just like in C++ and Java, and have the added | |
| 216 benefit of expressing mutability intent. | |
| 217 | |
| 218 For the reasons above, a reasonable default is to prefer `const` and `let` over | |
| 219 `var`, with the same caveat as above. | |
| 220 | |
| 221 ### Strict Mode | |
| 222 | |
| 223 JavaScript's | |
| 224 [strict mode](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Strict _mode), | |
| 225 activated by adding `'use strict';` to the very top of a script, helps catch | |
| 226 some errors, such as mistyping a variable name, forgetting to declare a | |
| 227 variable, or attempting to change a read-only property. | |
| 228 | |
| 229 Given that strict mode gives some of the benefits of using a compiler, adding it | |
| 230 to every test is a good default. This does not apply when specifically testing | |
| 231 sloppy mode behavior. | |
| 232 | |
| 233 Some developers argue that adding the `'use strict';` boilerplate can be | |
| 234 difficult to remember, weighs down smaller tests, and in many cases running a | |
| 235 test case is sufficient to discover any mistyped variable names. | |
| 236 | |
| 237 ### Promises | |
| 238 | |
| 239 [Promises](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Ob jects/Promise) | |
| 240 are a mechanism for structuring asynchronous code. When used correctly, Promises | |
| 241 avoid some of the | |
| 242 [issues of callbacks](http://colintoh.com/blog/staying-sane-with-asynchronous-pr ogramming-promises-and-generators). | |
| 243 For these reasons, a good default is to prefer promises over other asynchronous | |
| 244 code structures. | |
| 245 | |
| 246 When using promises, be aware of the | |
| 247 [execution order subtleties](https://jakearchibald.com/2015/tasks-microtasks-que ues-and-schedules/) | |
| 248 associated with them. Here is a quick summary. | |
| 249 | |
| 250 * The function passed to `Promise.new` is executed synchronously, so it finishes | |
| 251 before the Promise is created and returned. | |
| 252 * The functions passed to `then` and `catch` are executed in | |
| 253 _separate microtasks_, so they will be executed after the code that resolved | |
| 254 or rejected the promise finishes, but before any other event handler. | |
| 255 | |
| 256 ### Classes | |
| 257 | |
| 258 [Classes](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Classes) | |
| 259 are syntactic sugar for JavaScript's | |
| 260 [prototypal inheritance](https://developer.mozilla.org/docs/Web/JavaScript/Inher itance_and_the_prototype_chain). | |
| 261 Compared to manipulating prototypes directly, classes offer a syntax that is | |
| 262 more familiar to developers coming from other programming languages. | |
| 263 | |
| 264 A good default is to prefer classes over other OOP constructs, as they will make | |
| 265 the code easier to read for many of your fellow Chrome developers. At the same | |
| 266 time, most layout tests are simple enough that OOP is not justified. | |
| 267 | |
| 268 ### Character Encoding | |
| 269 | |
| 270 When HTML pages do not explicitly declare a character encoding, browsers | |
| 271 determine the encoding using an | |
| 272 [encoding sniffing algorithm](https://html.spec.whatwg.org/multipage/syntax.html #determining-the-character-encoding) | |
| 273 that will surprise most modern Web developers. Highlights include a default | |
| 274 encoding that depends on the user's locale, and non-standardized | |
| 275 browser-specific heuristics. | |
| 276 | |
| 277 The easiest way to not have to think about any of this is to add | |
| 278 `<meta charset="utf-8">` to all your tests. This is easier to remember if you | |
| 279 use a template for your layout tests, rather than writing them from scratch. | |
| 280 | |
| 281 ## Tests with Manual Feedback | |
| 282 | |
| 283 Tests that rely on the testing APIs exposed by WPT or Blink will not work when | |
| 284 loaded in a standard browser environment. When writing such tests, default to | |
| 285 having the tests gracefully degrade to manual tests in the absence of the | |
| 286 testing APIs. | |
| 287 | |
| 288 The | |
| 289 [document on layout tests with manual feedback](./layout_tests_with_manual_fallb ack.md) | |
| 290 describes the approach in detail and highlights the trade-off between added test | |
| 291 weight and ease of debugging. | |
| OLD | NEW |