summaryrefslogtreecommitdiffstats
path: root/testing/web-platform/tests/docs/reviewing-tests/checklist.md
blob: be0f4d134e4d3d9ad06e994d96937640ee53246b (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
# Review Checklist

The following checklist is provided as a guideline to assist in reviewing
tests; in case of any contradiction with requirements stated elsewhere in the
documentation it should be ignored
(please [file a bug](https://github.com/web-platform-tests/wpt/issues/new)!).

As noted on the [reviewing tests](./index.md) page, nits need not block PRs
from landing.


## All tests

<label><input type="checkbox">
The CI jobs on the pull request have passed.
</label>

<label><input type="checkbox">
It is obvious what the test is trying to test.
</label>

<label><input type="checkbox">
The test passes when it's supposed to pass.
</label>

<label><input type="checkbox">
The test fails when it's supposed to fail.
</label>

<label><input type="checkbox">
The test is testing what it thinks it's testing.
</label>

<label><input type="checkbox">
The spec backs up the expected behavior in the test.
</label>

<label><input type="checkbox">
The test is automated as either [reftest](../writing-tests/reftests) or
a [script test](../writing-tests/testharness) unless there's a very good reason for it not to be.
</label>

<label><input type="checkbox">
The test does not use external resources.
</label>

<label><input type="checkbox">
The test does not use proprietary features (vendor-prefixed or otherwise).
</label>

<label><input type="checkbox">
The test does not contain commented-out code.
</label>

<label><input type="checkbox">
The test is placed in the relevant directory.
</label>

<label><input type="checkbox">
The test has a reasonable and concise filename.
</label>

<label><input type="checkbox">
If the test needs code running on the server side, the server code must be
written in Python, and the Python code must not do anything potentially unsafe.
</label>

<label><input type="checkbox">
If the test needs to be run in some non-standard configuration or needs user
interaction, it is a manual test.
</label>

<label><input type="checkbox">
**Nit**: The title is descriptive but not too wordy.
</label>


## Reftests Only

<label><input type="checkbox">
The reference file is accurate and will render pixel-perfect
identically to the test on all platforms.
</label>

<label><input type="checkbox">
The reference file uses a different technique that won't fail in
the same way as the test.
</label>

<label><input type="checkbox">
The test and reference render within a 800x600 viewport, only displaying
scrollbars if their presence is being tested.
</label>

<label><input type="checkbox">
**Nit**: The test has a self-describing statement.
</label>

<label><input type="checkbox">
**Nit**: The self-describing statement is accurate, precise, simple, and
self-explanatory. Someone with no technical knowledge should be able to say
whether the test passed or failed within a few seconds, and not need to spend
several minutes thinking or asking questions.
</label>


## Script Tests Only

<label><input type="checkbox">
The number of tests in each file and the test names are consistent
across runs and browsers. It is best to avoid the pattern where there is
a test that asserts that the feature is supported and bails out without
running the rest of the tests in the file if it isn't.
</label>

<label><input type="checkbox">
The test avoids patterns that make it less likely to be stable.
In particular, tests should avoid setting internal timeouts, since the
time taken to run it may vary on different devices; events should be used
instead (if at all possible).
</label>

<label><input type="checkbox">
The test uses the most specific asserts possible (e.g. doesn't use
`assert_true` for everything).
</label>

<label><input type="checkbox">
The test uses `idlharness.js` if it is testing basic IDL-defined behavior.
</label>

<label><input type="checkbox">
**Nit**: Tests in a single file are separated by one empty line.
</label>


## Visual Tests Only

<label><input type="checkbox">
The test has a self-describing statement.
</label>

<label><input type="checkbox">
The self-describing statement is accurate, precise, simple, and
self-explanatory. Someone with no technical knowledge should be able to say
whether the test passed or failed within a few seconds, and not need to spend
several minutes thinking or asking questions.
</label>

<label><input type="checkbox">
The test renders within a 800x600 viewport, only displaying scrollbars if their
presence is being tested.
</label>

<label><input type="checkbox">
The test renders to a fixed, static page with no animation.
</label>