Closed Bug 570327 Opened 10 years ago Closed 10 years ago

mochitest framework should show values for is/isnot tests even when they pass

Categories

(Testing :: Mochitest, defect)

defect
Not set

Tracking

(status1.9.2 .13-fixed, status1.9.1 .16-fixed)

VERIFIED FIXED
mozilla1.9.3a5
Tracking Status
status1.9.2 --- .13-fixed
status1.9.1 --- .16-fixed

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: verified1.9.1, verified1.9.2)

Attachments

(1 file)

Attached patch patchSplinter Review
I often find myself reading mochitest logs when I'm developing new tests or trying to understand errors on tinderbox, and wishing I knew more about what was happening for *passing* tests:  in particular, I wanted to know the values for is/isnot tests, which we show when they fail, but not when they pass.

This patch changes that, and it also changes the wording of is/isnot diagnostics to be what I consider a significantly better style:  one where the description of the test makes sense both when it passes and when it fails.  (I think it's very bad form to write a test description that only makes sense as a failure message.)
Attachment #449460 - Flags: review?(ted.mielczarek)
Comment on attachment 449460 [details] [diff] [review]
patch

>From: L. David Baron <dbaron@dbaron.org>
>
>Show value information for passing mochitests too.
>
>diff --git a/testing/mochitest/tests/SimpleTest/SimpleTest.js b/testing/mochitest/tests/SimpleTest/SimpleTest.js
>--- a/testing/mochitest/tests/SimpleTest/SimpleTest.js
>+++ b/testing/mochitest/tests/SimpleTest/SimpleTest.js
>@@ -44,22 +44,22 @@ SimpleTest.ok = function (condition, nam
>     SimpleTest._tests.push(test);
> };
> 
> /**
>  * Roughly equivalent to ok(a==b, name)
> **/
> SimpleTest.is = function (a, b, name) {
>     var repr = MochiKit.Base.repr;
>-    SimpleTest.ok(a == b, name, "got " + repr(a) + ", expected " + repr(b));
>+    SimpleTest.ok(a == b, name, repr(a) + " should equal " + repr(b));
> };
> 
> SimpleTest.isnot = function (a, b, name) {
>     var repr = MochiKit.Base.repr;
>-    SimpleTest.ok(a != b, name, "Didn't expect " + repr(a) + ", but got it.");
>+    SimpleTest.ok(a != b, name, repr(a) + " should not equal " + repr(b));
> };

These are a nice change. That wording always bothered me.

>@@ -72,37 +72,37 @@ SimpleTest._logResult = function(test, p
>   if (parentRunner.currentTestURL)
>     msg += parentRunner.currentTestURL;
>   msg += " | " + test.name;
>   var diag = test.diag ? " - " + test.diag : "";

Does it make more sense to just do:
if (test.diag)
  msg += " - " + test.diag;

here?
Attachment #449460 - Flags: review?(ted.mielczarek) → review+
http://hg.mozilla.org/mozilla-central/rev/396aec61ada2
(with the suggestion in the previous comment)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Depends on: 578301
V.Fixed and verified1.9.2, per bug 612411 (for example).
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
verified1.9.1, per 
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1289885804.1289887750.1466.gz
Linux mozilla-1.9.1 test mochitests on 2010/11/15 21:36:44
Keywords: verified1.9.1
You need to log in before you can comment on or make changes to this bug.