Bug 972093 - Detect run_next_test() during add_task(); r=ted, rnewman
authorGregory Szorc <gps@mozilla.com>
Wed, 12 Feb 2014 16:47:29 -0800
changeset 169387 8b0e7e6aa1dffab8a8ee873cd5a6d4536cd052f7
parent 169386 15b7383d57120f2f900180fd18e562a566149fde
child 169388 db98e96431baeac1ce6e5374f5a5fc0c54674cce
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersted, rnewman
bugs972093
milestone30.0a1
Bug 972093 - Detect run_next_test() during add_task(); r=ted, rnewman If an xpcshell test called run_next_test() from inside an add_task(), bad things would happen. This patch detects that behavior and aborts the test immediately with an actionable error message.
services/common/tests/unit/test_utils_convert_string.js
services/crypto/tests/unit/test_utils_pbkdf2.js
services/fxaccounts/tests/xpcshell/test_accounts.js
services/fxaccounts/tests/xpcshell/test_credentials.js
testing/xpcshell/head.js
testing/xpcshell/selftest.py
toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
--- a/services/common/tests/unit/test_utils_convert_string.js
+++ b/services/common/tests/unit/test_utils_convert_string.js
@@ -69,62 +69,54 @@ add_test(function test_bad_argument() {
     do_check_true(failed);
   }
 
   run_next_test();
 });
 
 add_task(function test_stringAsHex() {
   do_check_eq(TEST_HEX, CommonUtils.stringAsHex(TEST_STR));
-  run_next_test();
 });
 
 add_task(function test_hexAsString() {
   do_check_eq(TEST_STR, CommonUtils.hexAsString(TEST_HEX));
-  run_next_test();
 });
 
 add_task(function test_hexToBytes() {
   let bytes = CommonUtils.hexToBytes(TEST_HEX);
   do_check_eq(TEST_BYTES.length, bytes.length);
   // Ensure that the decimal values of each byte are correct
   do_check_true(arraysEqual(TEST_BYTES,
       CommonUtils.stringToByteArray(bytes)));
-  run_next_test();
 });
 
 add_task(function test_bytesToHex() {
   // Create a list of our character bytes from the reference int values
   let bytes = CommonUtils.byteArrayToString(TEST_BYTES);
   do_check_eq(TEST_HEX, CommonUtils.bytesAsHex(bytes));
-  run_next_test();
 });
 
 add_task(function test_stringToBytes() {
   do_check_true(arraysEqual(TEST_BYTES,
       CommonUtils.stringToByteArray(CommonUtils.stringToBytes(TEST_STR))));
-  run_next_test();
 });
 
 add_task(function test_stringRoundTrip() {
   do_check_eq(TEST_STR,
     CommonUtils.hexAsString(CommonUtils.stringAsHex(TEST_STR)));
-  run_next_test();
 });
 
 add_task(function test_hexRoundTrip() {
   do_check_eq(TEST_HEX,
     CommonUtils.stringAsHex(CommonUtils.hexAsString(TEST_HEX)));
-  run_next_test();
 });
 
 add_task(function test_byteArrayRoundTrip() {
   do_check_true(arraysEqual(TEST_BYTES,
     CommonUtils.stringToByteArray(CommonUtils.byteArrayToString(TEST_BYTES))));
-  run_next_test();
 });
 
 // turn formatted test vectors into normal hex strings
 function h(hexStr) {
   return hexStr.replace(/\s+/g, "");
 }
 
 function arraysEqual(a1, a2) {
--- a/services/crypto/tests/unit/test_utils_pbkdf2.js
+++ b/services/crypto/tests/unit/test_utils_pbkdf2.js
@@ -86,18 +86,16 @@ add_task(function test_pbkdf2_hmac_sha1(
      DK: h("56 fa 6a a7 55 48 09 9d"+
            "cc 37 d7 f0 34 25 e0 c3"), // (16 octets)
     },
   ];
 
   for (let v of vectors) {
     do_check_eq(v.DK, b2h(pbkdf2(v.P, v.S, v.c, v.dkLen)));
   }
-
-  run_next_test();
 });
 
 // I can't find any normative ietf test vectors for pbkdf2 hmac-sha256.
 // The following vectors are derived with the same inputs as above (the sha1
 // test).  Results verified by users here:
 // https://stackoverflow.com/questions/5130513/pbkdf2-hmac-sha2-test-vectors
 add_task(function test_pbkdf2_hmac_sha256() {
   let pbkdf2 = CryptoUtils.pbkdf2Generate;
@@ -151,16 +149,14 @@ add_task(function test_pbkdf2_hmac_sha25
            "3c 69 62 26 65 0a 86 87"), // (16 octets)
     },
   ];
 
   for (let v of vectors) {
     do_check_eq(v.DK,
         b2h(pbkdf2(v.P, v.S, v.c, v.dkLen, Ci.nsICryptoHMAC.SHA256, 32)));
   }
-
-  run_next_test();
 });
 
 // turn formatted test vectors into normal hex strings
 function h(hexStr) {
   return hexStr.replace(/\s+/g, "");
 }
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -480,26 +480,22 @@ add_task(function test_getAssertion() {
 add_task(function test_resend_email_not_signed_in() {
   let fxa = new MockFxAccounts();
 
   try {
     yield fxa.resendVerificationEmail();
   } catch(err) {
     do_check_eq(err.message,
       "Cannot resend verification email; no signed-in user");
-    do_test_finished();
-    run_next_test();
     return;
   }
   do_throw("Should not be able to resend email when nobody is signed in");
 });
 
-add_task(function test_resend_email() {
-  do_test_pending();
-
+add_test(function test_resend_email() {
   let fxa = new MockFxAccounts();
   let alice = getTestUser("alice");
 
   do_check_eq(fxa.internal.generationCount, 0);
 
   // Alice is the user signing in; her email is unverified.
   fxa.setSignedInUser(alice).then(() => {
     log.debug("Alice signing in");
@@ -523,17 +519,16 @@ add_task(function test_resend_email() {
         // Timer was not restarted
         do_check_eq(fxa.internal.generationCount, 1);
 
         // Timer is still ticking
         do_check_true(fxa.internal.currentTimer > 0);
 
         // Ok abort polling before we go on to the next test
         fxa.internal.abortExistingFlow();
-        do_test_finished();
         run_next_test();
       });
     });
   });
 });
 
 /*
  * End of tests.
--- a/services/fxaccounts/tests/xpcshell/test_credentials.js
+++ b/services/fxaccounts/tests/xpcshell/test_credentials.js
@@ -59,18 +59,16 @@ add_task(function test_onepw_setup_crede
 
   do_check_eq(b2h(authPW), "4b8dec7f48e7852658163601ff766124c312f9392af6c3d4e1a247eb439be342");
 
   // derive unwrap key
   let unwrapKeyInfo = Credentials.keyWord('unwrapBkey');
   let unwrapKey = hkdf(quickStretchedPW, hkdfSalt, unwrapKeyInfo, hkdfLen);
 
   do_check_eq(b2h(unwrapKey), "8ff58975be391338e4ec5d7138b5ed7b65c7d1bfd1f3a4f93e05aa47d5b72be9");
-
-  run_next_test();
 });
 
 add_task(function test_client_stretch_kdf() {
   let pbkdf2 = CryptoUtils.pbkdf2Generate;
   let hkdf = CryptoUtils.hkdf;
   let expected = vectors["client stretch-KDF"];
 
   let emailUTF8 = h2s(expected.email);
@@ -98,18 +96,16 @@ add_task(function test_client_stretch_kd
   do_check_eq(passwordUTF8, results.passwordUTF8,
       "passwordUTF8 is wrong");
 
   do_check_eq(expected.quickStretchedPW, b2h(results.quickStretchedPW),
       "quickStretchedPW is wrong");
 
   do_check_eq(expected.authPW, b2h(results.authPW),
       "authPW is wrong");
-
-  run_next_test();
 });
 
 // End of tests
 // Utility functions follow
 
 function run_test() {
   run_next_test();
 }
--- a/testing/xpcshell/head.js
+++ b/testing/xpcshell/head.js
@@ -1370,29 +1370,39 @@ function add_task(func) {
   _gTests.push([true, func]);
 }
 
 /**
  * Runs the next test function from the list of async tests.
  */
 let _gRunningTest = null;
 let _gTestIndex = 0; // The index of the currently running test.
+let _gTaskRunning = false;
 function run_next_test()
 {
+  if (_gTaskRunning) {
+    throw new Error("run_next_test() called from an add_task() test function. " +
+                    "run_next_test() should not be called from inside add_task() " +
+                    "under any circumstances!");
+  }
+
   function _run_next_test()
   {
     if (_gTestIndex < _gTests.length) {
       let _isTask;
       [_isTask, _gRunningTest] = _gTests[_gTestIndex++];
       print("TEST-INFO | " + _TEST_FILE + " | Starting " + _gRunningTest.name);
       do_test_pending(_gRunningTest.name);
 
       if (_isTask) {
-        _Task.spawn(_gRunningTest)
-             .then(run_next_test, do_report_unexpected_exception);
+        _gTaskRunning = true;
+        _Task.spawn(_gRunningTest).then(
+          () => { _gTaskRunning = false; run_next_test(); },
+          (ex) => { _gTaskRunning = false; do_report_unexpected_exception(ex); }
+        );
       } else {
         // Exceptions do not kill asynchronous tests, so they'll time out.
         try {
           _gRunningTest();
         } catch (e) {
           do_throw(e);
         }
       }
--- a/testing/xpcshell/selftest.py
+++ b/testing/xpcshell/selftest.py
@@ -113,16 +113,26 @@ function run_test() { run_next_test(); }
 
 add_task(function test() {
   let result = yield Promise.resolve(false);
 
   do_check_true(result);
 });
 '''
 
+ADD_TASK_RUN_NEXT_TEST = '''
+function run_test() { run_next_test(); }
+
+add_task(function () {
+  Assert.ok(true);
+
+  run_next_test();
+});
+'''
+
 ADD_TEST_THROW_STRING = '''
 function run_test() {do_throw("Passing a string to do_throw")};
 '''
 
 ADD_TEST_THROW_OBJECT = '''
 let error = {
   message: "Error object",
   fileName: "failure.js",
@@ -511,16 +521,29 @@ tail =
             ADD_TASK_FAILURE_INSIDE)
         self.writeManifest(["test_add_task_failure_inside.js"])
 
         self.assertTestResult(False)
         self.assertEquals(1, self.x.testCount)
         self.assertEquals(0, self.x.passCount)
         self.assertEquals(1, self.x.failCount)
 
+    def testAddTaskRunNextTest(self):
+        """
+        Calling run_next_test() from inside add_task() results in failure.
+        """
+        self.writeFile("test_add_task_run_next_test.js",
+            ADD_TASK_RUN_NEXT_TEST)
+        self.writeManifest(["test_add_task_run_next_test.js"])
+
+        self.assertTestResult(False)
+        self.assertEquals(1, self.x.testCount)
+        self.assertEquals(0, self.x.passCount)
+        self.assertEquals(1, self.x.failCount)
+
     def testMissingHeadFile(self):
         """
         Ensure that missing head file results in fatal error.
         """
         self.writeFile("test_basic.js", SIMPLE_PASSING_TEST)
         self.writeManifest([("test_basic.js", "head = missing.js")])
 
         raised = False
--- a/toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
+++ b/toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
@@ -36,18 +36,16 @@ add_task(function* test_constructor_inva
 });
 
 add_task(function* test_get_manager() {
   let m = yield getManager();
   Assert.ok(m, "CrashManager obtained.");
 
   yield m.createDummyDump(true);
   yield m.createDummyDump(false);
-
-  run_next_test();
 });
 
 // Unsubmitted dump files on disk are detected properly.
 add_task(function* test_pending_dumps() {
   let m = yield getManager();
   let now = Date.now();
   let ids = [];
   const COUNT = 5;