From 6aa9dd74edea2db823f67434aed93375fb1f91d8 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Mon, 23 Nov 2015 13:22:13 -0800 Subject: [PATCH] test: fix test-domain-exit-dispose-again 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. --- test/simple/test-domain-exit-dispose-again.js | 59 +++++++------------ 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/test/simple/test-domain-exit-dispose-again.js b/test/simple/test-domain-exit-dispose-again.js index 22928f2604511d..fb58a673726bc0 100644 --- 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'); -});