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!).

As noted on the reviewing tests page, nits need not block PRs from landing.

All tests

The CI jobs on the pull request have passed.

It is obvious what the test is trying to test.

The test passes when it’s supposed to pass.

The test fails when it’s supposed to fail.

The test is testing what it thinks it’s testing.

The spec backs up the expected behavior in the test.

The test is automated as either reftest or a script test unless there’s a very good reason for it not to be.

The test does not use external resources.

The test does not use proprietary features (vendor-prefixed or otherwise).

The test does not contain commented-out code.

The test is placed in the relevant directory.

The test has a reasonable and concise filename.

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.

If the test needs to be run in some non-standard configuration or needs user interaction, it is a manual test.

Nit: The title is descriptive but not too wordy.

Reftests Only

The reference file is accurate and will render pixel-perfect identically to the test on all platforms.

The reference file uses a different technique that won’t fail in the same way as the test.

The test and reference render within a 800x600 viewport, only displaying scrollbars if their presence is being tested.

Nit: The test has a self-describing statement.

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.

Script Tests Only

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.

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).

The test uses the most specific asserts possible (e.g. doesn’t use assert_true for everything).

The test uses idlharness.js if it is testing basic IDL-defined behavior.

Nit: Tests in a single file are separated by one empty line.

Visual Tests Only

The test has a self-describing statement.

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.

The test renders within a 800x600 viewport, only displaying scrollbars if their presence is being tested.

The test renders to a fixed, static page with no animation.