test: fix test-domain-exit-dispose-again

Julien Gilli · rvagg · commit d1ba82af1c25 · 2015-12-04T14:28:27.000+11:00
test-domain-exit-dispose-again had been written for node v0.10.x, and was using the fact that callbacks scheduled with `process.nextTick` wouldn't run if the domain attached to it was disposed. This is not longer the case, and as a result the test would not catch any regression: it would always pass. This change rewrites that test to check that the current domain is cleared properly when processing the rest of the timers list if a timer's callback throws an error. This makes the test fail without the original fix, and pass with the original fix, as expected. PR: #3991 PR-URL: #3991 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
diff --git a/test/simple/test-domain-exit-dispose-again.js b/test/simple/test-domain-exit-dispose-again.js
@@ -19,58 +19,41 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-var common = require('../common.js');
+var common = require('../common');
 var assert = require('assert');
 var domain = require('domain');
-var disposalFailed = false;
 
-// no matter what happens, we should increment a 10 times.
-var a = 0;
-log();
-function log(){
-  console.log(a++, process.domain);
-  if (a < 10) setTimeout(log, 20);
-}
-
-var secondTimerRan = false;
-
-// in 50ms we'll throw an error.
+// Use the same timeout value so that both timers' callbacks are called during
+// the same invocation of the underlying native timer's callback (listOnTimeout
+// in lib/timers.js).
 setTimeout(err, 50);
-setTimeout(secondTimer, 50);
-function err(){
+setTimeout(common.mustCall(secondTimer), 50);
+
+function err() {
   var d = domain.create();
-  d.on('error', handle);
+  d.on('error', handleDomainError);
   d.run(err2);
 
   function err2() {
-    // this timeout should never be called, since the domain gets
-    // disposed when the error happens.
-    setTimeout(function() {
-      console.error('This should not happen.');
-      disposalFailed = true;
-      process.exit(1);
-    });
-
     // this function doesn't exist, and throws an error as a result.
     err3();
   }
 
-  function handle(e) {
-    // this should clean up everything properly.
-    d.dispose();
-    console.error(e);
-    console.error('in handler', process.domain, process.domain === d);
+  function handleDomainError(e) {
+    // In the domain's error handler, the current active domain should be the
+    // domain within which the error was thrown.
+    assert.equal(process.domain, d);
   }
 }
 
 function secondTimer() {
-  console.log('In second timer');
-  secondTimerRan = true;
+  // secondTimer was scheduled before any domain had been created, so its
+  // callback should not have any active domain set when it runs.
+  // Do not use assert here, as it throws errors and if a domain with an error
+  // handler is active, then asserting wouldn't make the test fail.
+  if (process.domain !== null) {
+    console.log('process.domain should be null, but instead is:',
+      process.domain);
+    process.exit(1);
+  }
 }
-
-process.on('exit', function() {
-  assert.equal(a, 10);
-  assert.equal(disposalFailed, false);
-  assert(secondTimerRan);
-  console.log('ok');
-});
