Reviewing Tests

In order to encourage a higher level of quality in the CSS test suites, tests that have been individually reviewed for correctness and usability are assigned an Approved status and placed in the top-level approved/ directory. Additions and changes to the Approved section of the test suite must be reviewed before they can be checked in.

Anyone with the ability to read and understand the spec and a thorough understanding of the test suite guidelines can review additions and changes to the Approved collection. However a reviewer cannot review his/her own tests and changes.

Tests pending approval are linked from the pending review page.

1. The reviewer must check each test submitted for approval for conformance to the test suite format and guidelines and for correctness with respect to the specification. For proposed changes to existing approved tests, the reviewer must check the changes for correctness.
2. If the test does not pass review, the reviewer must tell the submitter what is wrong with the test and what steps should be taken to correct any problems. Typically the submitter is responsible for fixing the tests, but the reviewer may make the changes directly and then explain to the submitter what they were (possibly by pointing to the change log) and why they were necessary. If these were not metadata-only changes and they are sufficient for the reviewer to consider the test acceptable, then the reviewer should add himself as reviewer-author, and the submitter (or, in the case of abandoned tests, another reviewer) needs to review and accept these changes.
   • New tests and changes that have been declined by one reviewer cannot be accepted by someone else until the problems are corrected or have been demonstrated to be invalid. In case of a conflict, an Owner or Peer's opinion carries more weight. In all cases the CSS Working Group has final say in whether a test is valid, and may be consulted if deemed necessary.
3. Once test/changes pass review, the reviewer must note their acceptance in the test. The test is now “Accepted”. At this point an Owner or Peer needs to approve the test.
4. The Owner or Peer can either approve the reviewer's judgement if the reviewer is known to be competent in this area, or review the test himself. Once the Owner or Peer is satisfied the test is “Approved”. (If the initial reviewer was an Owner or Peer then this step is automatic).
5. New tests and changes that have been accepted and approved can be moved to the approved directory by anyone with write access, and that person is not required to perform any further review. The checkin comment should indicate the approver by either “r=approver” if the approver performed a review or “rs=approver” if the approver rubber-stamped the previous reviewer's judgement.

The review process looks like this:

Changes to the build scripts must be reviewed by a test suite Owner.

To request review, submit proposed tests and changes as described in the contribution guidelines.

If these requirements change, notice will be sent to public-css-testsuite of the changes.

When reviewing a test, you're responsible for making sure the test matches the test format and testing guidelines. In addition to all the format and validity requirements in those documents, make sure you also check the following:

• That the test is testing what it thinks it's testing. (I've run across many tests that think they're testing something, but are actually testing something else.)
• That the test instructions are accurate, precise, simple, and self-explanatory. Your mother/husband/roommate/brother/bus driver 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.
• That the title is both unique and descriptive but not too wordy.
• That the test is as cross-platform as reasonably possible, working across different devices, screen resolutions, paper sizes, etc. If there are limitations (e.g. the test will only work on 96dpi devices, or screens wider than 200 pixels) that these are documented in the instructions.
• That the spec backs up the expected behavior in the test. (I've run into a number of tests that make assumptions I could've sworn were in the spec, but aren't there when I go and check. Since this often means the spec forgot to handle something, you should send a message to www-style about it.)

A more detailed review checklist is available.

The test suite Owner is the person most responsible for the test suite. Previously that was Ian Hickson (Hixie); currently that is Elika J. Etemad (fantasai). A Peer is someone the test suite Owner considers equally capable of evaluating changes to the test suite and responsible enough to have write access to the test suite. A review from a Peer carries equal weight to a review from an Owner.

Currently the following people are Owners or Peers:

• Elika J. Etemad aka fantasai
• L. David Baron
• Ian Hickson aka Hixie
• Anne van Kesteren

An Approver is someone who has demonstrated a very strong understanding of the CSS specs and test methodology and an active commitment to working on the test suite but who hasn't quite racked up enough experience points to become a full test suite Peer. An Approver has write access to the repository and can approve tests but may only review his own company's tests under the Owner's oversight.

Currently the following people are Approvers:

• Arron Eicholz (Microsoft)
• Gérard Talbot

There are three basic qualifications you must satisfy to become a Peer:

• You must demonstrate the ability to read and interpret the spec correctly.
• You must demonstrate a thorough understanding of the test suite guidelines.
• You must demonstrate that you are trustworthy.

The first two qualifications can be met by submitting good-quality tests for the test suite and intelligently critiquing others' tests.

The third qualification is met by someone vouching for you. This could be a test suite Owner or Peer, or it could be someone else trusted by an Owner. For example, since fantasai trusts active members of the CSS Working Group, an employee of Microsoft can ask his CSS Working Group representative to vouch. As another example, since fantasai trusts Mozilla module owners and peers, a contributor to that project could ask one of them to vouch. By vouching, a voucher is putting his or her reputation on the line, and is confirming that the candidate has established enough credibility in the voucher's experience that the voucher would entrust him or her with a similar level of write privileges in their own project.

If the Owner is satisfied that a candidate would make a suitable Peer, s/he will then request write access to any relevant systems for the candidate and add him or her to the Peers list.