07.08.09

Public service announcement: name test files descriptively, not just with bug numbers

Mozilla’s tests are spread through a large number of directories. One of these directories, layout/reftests/bugs, contains a thousand or so reftests. Each reftest file is named by the number of the corresponding bug: for example, 169749-1.html and 169749-1-ref.html. This name-the-test-by-the-bug pattern is not specific to this particular directory, and if you skim through the directories in layout/reftests you’ll find numerous other tests named this way.

The problem? These names are effectively useless for determining the extent of test coverage. (Assume arguendo that all these files were properly located in directories specific to a feature being tested, for example in a hypothetical layout/reftests/list-item directory for the two aforementioned tests, to make it reasonable to scan the coverage of a particular feature.) If I want to know what corner cases are being tested, I have to read each test and corresponding bug individually, a tedious task.

The solution? Name your test files descriptively. For example, in the above case, a better name might perhaps be marker-position-for-list-item-containing-block-with-margins.html.

Okay, this will make it easier for random readers at unknown points in the future, but is there anything in it for you, the test author? There is: you may endear yourself to reviewers. For example, dbaron decided, based upon the names I gave to test files in the recent patch I wrote to implement background-size (and upon a handful of comments in the list of tests itself), that my patch’s tests were adequate and didn’t need a long, tedious review of each one:

The reftest.list looks good based on the names of the tests and the
comments; I don't see the need to review the tests themselves.

In conclusion: DON’T name your test files based upon bug numbers, and DO name them based upon the particular scenario that they test. Readers will and reviewers may thank you.

8 Comments »

  1. Is anything wrong with names combining both ways? For example:
    169749-1-marker-position-for-list-item-containing-block-with-margins.html

    [That seems reasonable.]

    Comment by Adam Hauner — 07.08.09 @ 23:17

  2. I respectfully disagree with your suggestion. The test case functionality has to be annotated in the test case in the form of a comment instead of trying to cram it into the test case name itself. Consider testing scenarios where we end up with very deep nested directory structures coupled with long test names and end up failing running tests because of a long path problem. A test case name having a bug in it serves a specific purpose. It acts as a regression test case. If a bug number named test case fails , we know that it is causing a regression just by looking at the list of failed tests log.

    Naming test to indicate the purpose and annotating the bug for which the test case is written inside or vice-versa serves same functional purpose if suitable parsers are written to grep for annotations or test case names. However, bug numbered tests provide shorter path lengths while executing tests for fennec, just for example.

    [I’m not suggesting it be described exclusively by the test name; a comment in the test is also well worth doing. Further, as Adam notes, description and number are not exclusive. By the way, you can’t assume a test is a regression just because it’s a number; I’m sure I could point to many testcases named with numbers which are not themselves regressions.

    I don’t buy shorter path lengths as a valid goal. I don’t think we should hack around deficient platforms that don’t support long paths by handicapping ourselves in the source tree. If that’s really a concern, there could be a renaming step included in the test-execution process to specifically address that platform.]

    Comment by mnandigama — 07.08.09 @ 23:21

  3. I concur with your reply. But at the end of the day, one way or the other, we should be able to understand the purpose of the test with out reading the entire test code for each test case. Be it annotations or descriptive names.

    Comment by mnandigama — 07.08.09 @ 23:32

  4. I’ve seen long-named tests in the tree and they’ve bugged me everytime because it is harder to identify the test in the list (because frequently in the directory view the end is chopped off) and even then they tend to be pretty obscure because they try to keep the length shorter.

    Also I really hope we aren’t judging whether we have good test coverage on things based on the naming alone.

    Instead I’d be more inclined to have a README or something in the directory that includes a short description of what each test covers.

    [No serious judgment, but it’s great for a birds-eye view. A README could work, particularly since MXR might display it (or only some of it?), but that requires some duplication of information. (Comments in the test itself, I expect, would be less duplicative because they are likely to be more of an explanatory flavor than a descriptive flavor.)]

    Comment by Mossop — 08.08.09 @ 02:27

  5. I agree with Adam that retaining the bug number in the test file name is good practice. Coming up with a common format for bug number and test description in the test file name is also important. Consider the use case of taking a list of tests and determining the list of bugs where the tests were created. As long as you keep that simple, adding more descriptive text to the file name is fine.

    Comment by bc — 08.08.09 @ 02:33

  6. A descriptive name is helpful also in case of determining what a failure might be about, but a bug number is _very_ helpful in speedily finding the history of what a test was introduced for – IMHO, both should be in there, but too long filenames should also be avoided as I think they still give headaches with some filesystems (e.g. possibly FAT32).

    [Last I recall, FAT32 didn’t have problems with pretty generous path lengths, but I might be mistaken.]

    Comment by Robert Kaiser — 09.08.09 @ 06:24

  7. Having “what the test tests” in the name doesn’t help that much with coverage either, necessarily… and it not feasible anyway for tests that test more than one thing (most of them, actually). Using a non-numbered name also means you have to figure out _where_ to put the test, unless you think we should dump random non-numbered names into bugs/.

    Having the bug# is nice just in terms of figuring out whether a particular bug has a test (since some people aren’t so good at setting in-testsuite flags).

    It’s also nice in terms of figuring out what the test is _trying_ to test, if you discover that the test is bogus in the course of working on some patch (happens reasonably often, sadly).

    In the end, neither one provides all the metadata one wants for a test, but putting all that metadata into the test increases the test-writing burden tremendously. Not sure there are any good solutions here.

    Comment by Boris — 09.08.09 @ 10:12

  8. I think one practices is right in some cases, and the other in others.

    In particular, if you’re trying to write a thorough set of tests for a feature, use descriptive names.

    But if you’re landing a testcase for a particular bug that’s not part of a set of thorough tests, just to make sure that one particular case doesn’t regress, use the bug number, since a “named” test could mislead people into thinking whatever was described by that test name was actually being tested adequately.

    [In the latter case it seems to me an appropriate amount of vagueness is in order. 🙂 Enough of a hint from the name will point the way for someone interested in determining whether something’s been tested, even if it doesn’t identify exactly what’s being tested. Having a handful of files with plausible names is better than having the same handful with just numbers to go by.]

    Comment by David Baron — 09.08.09 @ 10:43

RSS feed for comments on this post. TrackBack URI

Leave a comment

HTML: You can use these tags: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>