Skip to content

Commit

Permalink
Remove domains
Browse files Browse the repository at this point in the history
Fixes a problem where expected EventEmitter-related throws are
impossible to test, because the domain hijacks the 'error' event.

Of course, this means that asynchronous unexpected throws will always be
assigned to the global root TAP object, but... what can you do?  If
people complain, we can try to figure out something clever using
continuation-local-storage or something.

Ugh.  Domains.  The not-quite-worst-possible solution to any problem,
which always ends up regrettably.

I am, as always, truly and deeply sorry for inflicting this feature on
Node.js
  • Loading branch information
isaacs committed Sep 29, 2015
1 parent e734348 commit c781bd9
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 46 deletions.
8 changes: 8 additions & 0 deletions lib/root.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ tap.mochaGlobals = tap.mocha.global
tap.Test = Test
tap.synonyms = require('./synonyms.js')

process.on('uncaughtException', function onUncaught (er) {
var child = tap
while (child._currentChild && child._currentChild instanceof Test) {
child = child._currentChild
}
child.threw(er)
})

// SIGTERM means being forcibly killed, almost always by timeout
var onExit = require('signal-exit')
onExit(function (code, signal) {
Expand Down
14 changes: 1 addition & 13 deletions lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ var util = require('util')
util.inherits(Test, Readable)

var yaml = require('js-yaml')
var domain = require('domain')
var stack = require('./stack.js')
var tapAsserts = require('./assert.js')
var assert = require('assert')
Expand Down Expand Up @@ -62,11 +61,6 @@ function Test (options) {
this._parent = null
this._printedVersion = false

this._domain = domain.create()
this._domain.add(this)
this._domain.owner = this
this._domain.on('error', domainError)

this._startTime = process.hrtime()
this._calledAt = options.at || stack.at(this.test)
if (!this._calledAt || !this._calledAt.file)
Expand Down Expand Up @@ -147,12 +141,6 @@ Test.prototype._onTimeout = function () {
this.endAll()
}

function domainError (er) {
delete er.domain
delete er.domainThrown
this.owner.threw(er)
}

// Called when endAll() is fired and there's stuff in the queue
Test.prototype._queueFail = function () {
var queue = this._queue
Expand Down Expand Up @@ -408,7 +396,7 @@ Test.prototype.test = function test (name, extra, cb) {
self._level = child
child.comment('Subtest: ' + name)
try {
child._domain.bind(cb)(child)
cb(child)
} catch (er) {
child.threw(er)
}
Expand Down
22 changes: 22 additions & 0 deletions test/expect-error-event.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// make sure domains don't obscure things.
var tap = require('..')
var EE = require('events').EventEmitter

function testFunction (t) {
var ee = new EE

t.throws(function () {
ee.emit('error', new Error('one'))
}, new Error('one'))

try {
ee.emit('error', new Error('two'))
t.fail('should throw')
} catch (er) {
t.match(er, { message: 'two' }, 'threw expected error')
}
t.end()
}

tap.test('child', testFunction)
testFunction(tap)
6 changes: 1 addition & 5 deletions test/test/throw.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var t = require('../../lib/test.js')()
var t = require('../..')

t.test('nesting', function (t) {
t.plan(3)
Expand Down Expand Up @@ -35,7 +35,3 @@ t.test('async kid', function (t) {
})

t.pass('pass after async kid')

t.end()

t.pipe(process.stdout)
2 changes: 1 addition & 1 deletion test/test/throws-and-plans-bail.tap
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ok 1 - expect truthy value

not ok 3 - Error: pwnSync
---
{"at":{"column":11,"file":"test/test/throws-and-plans.js","line":23},"message":"Error: pwnSync","source":"throw new Error('pwnSync')\n","test":"sync thrower"}
{"at":{"column":11,"file":"test/test/throws-and-plans.js","line":10},"message":"Error: pwnSync","source":"throw new Error('pwnSync')\n","test":"sync thrower"}
...
Bail out! # Error: pwnSync
Bail out! # Error: pwnSync
Expand Down
15 changes: 1 addition & 14 deletions test/test/throws-and-plans.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,4 @@
//var t = require('./lib/root.js')

//var t = require('./lib/test.js')()
//t.pipe(process.stdout)

var t = require('../../lib/test.js')()
t.pipe(process.stdout)

process.on('exit', function () {
t.end()
})

t._name = 'ROOT'

var t = require('../..')
t.ok('true')

t.test('plans of 1', function (t) {
Expand Down
26 changes: 13 additions & 13 deletions test/test/throws-and-plans.tap
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ok 1 - expect truthy value

not ok 3 - Error: pwnSync
---
{"at":{"column":11,"file":"test/test/throws-and-plans.js","line":23},"message":"Error: pwnSync","source":"throw new Error('pwnSync')\n","test":"sync thrower"}
{"at":{"column":11,"file":"test/test/throws-and-plans.js","line":10},"message":"Error: pwnSync","source":"throw new Error('pwnSync')\n","test":"sync thrower"}
...
# Subtest: async thrower
1..3
Expand All @@ -23,19 +23,19 @@ ok 1 - expect truthy value
# failed 1 of 5 tests
not ok 2 - plans of 1 ___/# time=[0-9.]+(ms)?/~~~
---
{"at":{"column":3,"file":"test/test/throws-and-plans.js","line":17},"results":{"count":5,"fail":1,"ok":false,"pass":4,"plan":{"end":5,"start":1}},"source":"t.test('plans of 1', function (t) {\n"}
{"at":{"column":3,"file":"test/test/throws-and-plans.js","line":4},"results":{"count":5,"fail":1,"ok":false,"pass":4,"plan":{"end":5,"start":1}},"source":"t.test('plans of 1', function (t) {\n"}
...

# Subtest: no assert only throw
not ok 1 - AssertionError: false is truthy right?
---
{"actual":false,"at":{"column":3,"file":"test/test/throws-and-plans.js","line":42},"expected":true,"generatedMessage":false,"message":"AssertionError: false is truthy right?","name":"AssertionError","operator":"==","source":"assert(false, 'false is truthy right?')\n","test":"no assert only throw"}
{"actual":false,"at":{"column":3,"file":"test/test/throws-and-plans.js","line":29},"expected":true,"generatedMessage":false,"message":"AssertionError: false is truthy right?","name":"AssertionError","operator":"==","source":"assert(false, 'false is truthy right?')\n","test":"no assert only throw"}
...
1..1
# failed 1 of 1 tests
not ok 3 - no assert only throw ___/# time=[0-9.]+(ms)?/~~~
---
{"at":{"column":3,"file":"test/test/throws-and-plans.js","line":39},"results":{"count":1,"fail":1,"ok":false,"pass":0,"plan":{"end":1,"start":1}},"source":"t.test('no assert only throw', function (t) {\n"}
{"at":{"column":3,"file":"test/test/throws-and-plans.js","line":26},"results":{"count":1,"fail":1,"ok":false,"pass":0,"plan":{"end":1,"start":1}},"source":"t.test('no assert only throw', function (t) {\n"}
...

# Subtest: plans of 8
Expand All @@ -45,7 +45,7 @@ not ok 3 - no assert only throw ___/# time=[0-9.]+(ms)?/~~~
ok 1 - before the bomb
not ok 2 - Error: pwnSync
---
{"at":{"column":11,"file":"test/test/throws-and-plans.js","line":52},"message":"Error: pwnSync","source":"throw new Error('pwnSync')\n","test":"sync thrower"}
{"at":{"column":11,"file":"test/test/throws-and-plans.js","line":39},"message":"Error: pwnSync","source":"throw new Error('pwnSync')\n","test":"sync thrower"}
...
not ok 3 - missing test
not ok 4 - missing test
Expand All @@ -56,39 +56,39 @@ not ok 3 - no assert only throw ___/# time=[0-9.]+(ms)?/~~~
# failed 7 of 8 tests
not ok 2 - sync thrower ___/# time=[0-9.]+(ms)?/~~~
---
{"at":{"column":5,"file":"test/test/throws-and-plans.js","line":48},"results":{"count":8,"fail":7,"ok":false,"pass":1,"plan":{"end":8,"start":1}},"source":"t.test('sync thrower', function (tt) {\n"}
{"at":{"column":5,"file":"test/test/throws-and-plans.js","line":35},"results":{"count":8,"fail":7,"ok":false,"pass":1,"plan":{"end":8,"start":1}},"source":"t.test('sync thrower', function (tt) {\n"}
...

# Subtest: async thrower
1..8
ok 1 - before set the bomb
ok 2 - after set the bomb
ok 3 - before the bomb
not ok 4 - Error: pwn
not ok 3 - Error: pwn
---
{"at":{"column":13,"file":"test/test/throws-and-plans.js","function":"null._onTimeout","line":64},"message":"Error: pwn","source":"throw new Error('pwn')\n","test":"async thrower"}
{"at":{"column":13,"file":"test/test/throws-and-plans.js","function":"null._onTimeout","line":18},"message":"Error: pwn","source":"throw new Error('pwn')\n","test":"async thrower"}
...
not ok 4 - missing test
not ok 5 - missing test
not ok 6 - missing test
not ok 7 - missing test
not ok 8 - missing test
# failed 5 of 8 tests
# failed 6 of 8 tests
not ok 3 - async thrower ___/# time=[0-9.]+(ms)?/~~~
---
{"at":{"column":5,"file":"test/test/throws-and-plans.js","line":59},"results":{"count":8,"fail":5,"ok":false,"pass":3,"plan":{"end":8,"start":1}},"source":"t.test('async thrower', function (tt) {\n"}
{"at":{"column":5,"file":"test/test/throws-and-plans.js","line":46},"results":{"count":8,"fail":6,"ok":false,"pass":2,"plan":{"end":8,"start":1}},"source":"t.test('async thrower', function (tt) {\n"}
...

ok 4 - after child
1..4
# failed 2 of 4 tests
not ok 4 - plans of 8 ___/# time=[0-9.]+(ms)?/~~~
---
{"at":{"column":3,"file":"test/test/throws-and-plans.js","line":45},"results":{"count":4,"fail":2,"ok":false,"pass":2,"plan":{"end":4,"start":1}},"source":"t.test('plans of 8', function (t) {\n"}
{"at":{"column":3,"file":"test/test/throws-and-plans.js","line":32},"results":{"count":4,"fail":2,"ok":false,"pass":2,"plan":{"end":4,"start":1}},"source":"t.test('plans of 8', function (t) {\n"}
...

not ok 5 - Error: pwn
---
{"at":{"column":13,"file":"test/test/throws-and-plans.js","function":"null._onTimeout","line":31},"message":"Error: pwn","source":"throw new Error('pwn')\n","test":"async thrower"}
{"at":{"column":13,"file":"test/test/throws-and-plans.js","function":"null._onTimeout","line":51},"message":"Error: pwn","source":"throw new Error('pwn')\n","test":"TAP"}
...
1..5
# failed 4 of 5 tests
Expand Down

0 comments on commit c781bd9

Please sign in to comment.