(3) Mochitest already have this. (Though I didn't see where it is done.) RefTest has TinderboxPrint only.

Unrelated nit: *Removes an extra space too. *** [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081122 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4) (http://hg.mozilla.org/mozilla-central/rev/5d37678a2482 +http://hg.mozilla.org/comm-central/rev/892e9f783d9e) Output example: { [...] REFTEST TEST-PASS | file:///[...] | REFTEST INFO | Pass: 2320 REFTEST INFO | Fail: 2 REFTEST INFO | RandomAndKF: 109 REFTEST INFO | LoadOnly: 4 REFTEST INFO | Skip: 2 REFTEST INFO | FailedLoad: 0 REFTEST INFO | Exception: 0 REFTEST FINISHED: Slowest test took 2012ms (file:///[...]) } I'd like to vertically align the numbers (as Mochitest does) but I don't know how to do it (simply). Fwiw, the figures are a little different from the (Windows SeaMonkey) waterfall: { TinderboxPrint: reftest<br/>2326/0/93 } I would guess 2326 = Pass + Fail (which don't on the tindrboxes) + LoadOnly 93 = +/- "Known Fail" (without the "Random only") + Skip

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081122 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4) (http://hg.mozilla.org/mozilla-central/rev/5d37678a2482 +http://hg.mozilla.org/comm-central/rev/892e9f783d9e) Av1, with: *Skip dumping results with no tests; and reorder results accordingly. *Unrelated nit: gSlowestTestURL initialization.

Av1a, with another unrelated extra space removal.

Comment on attachment 349740 [details] [diff] [review] (Av1b) Add dynamic summary to reftest.js >+var gTestResults = {Exception: 0, FailedLoad: 0, Fail: 0, >+ Pass: 0, RandomAndKF : 0, LoadOnly: 0, Skip: 0}; Please write this using the style: var gTestResults = { Exception: 0, ... }; >-var gSlowestTestURL; >+var gSlowestTestURL = null; This isn't needed. >+ ++gTestResults["Exception"]; Please write all increments of this form as ++gTestResults.Exception; >+ for (var result in gTestResults) >+ if (gTestResults[result] != 0) >+ dump("REFTEST INFO | " + result + ": " + gTestResults[result] + "\n"); > dump("REFTEST FINISHED: Slowest test took " + gSlowestTestTime + > "ms (" + gSlowestTestURL + ")\n"); Please put your new information after the "REFTEST FINISHED" message. >- var result = "REFTEST " + outputs[expected][test_passed] + " | "; >- result += gURLs[0].prettyPath + " | "; // the URL being tested >- if (!gURLs[0].equal) { >- result += "(!=) "; >- } >+ var result = "REFTEST " + outputs[expected][test_passed] + " | " + >+ gURLs[0].prettyPath + " |"; // the URL being tested >+ if (!gURLs[0].equal) >+ result += " (!=)"; > dump(result + "\n"); Please don't add unnecessary changes. (Some log parsers might even depend on the trailing space.) > if (!test_passed && expected == EXPECTED_PASS || > test_passed && expected == EXPECTED_FAIL) { >+ ++gTestResults["Fail"]; > if (!equal) { > dump("REFTEST IMAGE 1 (TEST): " + gCanvas1.toDataURL() + "\n"); > dump("REFTEST IMAGE 2 (REFERENCE): " + gCanvas2.toDataURL() + "\n"); > dump("REFTEST number of differing pixels: " + differences + "\n"); > } else { > dump("REFTEST IMAGE: " + gCanvas1.toDataURL() + "\n"); > } >+ } else { >+ if (test_passed && expected == EXPECTED_PASS) >+ ++gTestResults["Pass"]; >+ else >+ ++gTestResults["RandomAndKF"]; > } Please keep the what-to-increment logic separate from the whether-to-dump-images logic. "KF" is unclear, you should call it "KnownFail". And I'm not sure why you want to count Random and Known Fail together. (Or why you want to count Unexpected Fail and Unexpected Pass together and call them "Fail". If you want to lump them together you should call them "Unexpected".)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20081129 Minefield/3.1b3pre] (home, optim default) (W2Ksp4) (http://hg.mozilla.org/mozilla-central/rev/73d9cfe0174c) Av1b, with comment 4 suggestion(s); plus added summary title line.

Comment on attachment 350887 [details] [diff] [review] (Av2) Add dynamic summary to reftest.js [Checkin: Comment 9 & 10] r=dbaron

Nit, while here: Shouldn't { var outputs = {}; const randomMsg = "(EXPECTED RANDOM)"; ... } be global to the file rather than local to the function ? (They are invariant and I guess that should improve/speed things up a little...)

No. They're not needed outside the function. |outputs| could probably be written as a const rather than a var, and in a single expression, though, but that should NOT be done as part of this bug.

Comment on attachment 350887 [details] [diff] [review] (Av2) Add dynamic summary to reftest.js [Checkin: Comment 9 & 10] http://hg.mozilla.org/mozilla-central/rev/0f32649fe11b

Comment on attachment 350887 [details] [diff] [review] (Av2) Add dynamic summary to reftest.js [Checkin: Comment 9 & 10] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d7c76bca7adb

(In reply to comment #1) > Fwiw, the figures are a little different from the (Windows SeaMonkey) > waterfall: I filed bug 468023. (In reply to comment #8) > |outputs| could probably be written as a const rather than a var, and in a single expression, I filed bug 468024.