Skip to content

Commit

Permalink
Ensure that process.on('exit') handler is run
Browse files Browse the repository at this point in the history
This MAY fix #345, but without knowing the root cause of that problem,
it's hard to say for sure.

What IS definitely fixed by this is the behavior when a test program
does this:

```js
process.on('exit', function () {
  process.exit()
})
```

prior to TAP loading and adding its exit event handler.

This also prevents TAP from hooking up to the process.on('exit') event
if it's not piping anywhere, so just doing this won't change your exit
code:

```js
var Test = require('tap').Test
```
  • Loading branch information
isaacs committed Feb 14, 2017
1 parent dcce742 commit a5a1889
Show file tree
Hide file tree
Showing 6 changed files with 297 additions and 14 deletions.
58 changes: 44 additions & 14 deletions lib/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,54 @@ TAP.prototype.pipe = function () {
this.setTimeout(this.options.timeout)
this.pipe = Test.prototype.pipe
this.push = Test.prototype.push
return this.pipe.apply(this, arguments)
var ret = this.pipe.apply(this, arguments)
this.process()
return ret
}

function monkeypatchEpipe () {
process.stdout.emit = function (emit) {
return function (ev, er) {
if (ev === 'error' && er.code === 'EPIPE')
return this.emit = emit
return emit.apply(this, arguments)
}
}(process.stdout.emit)
}

function monkeypatchExit () {
// ensure that we always get run, even if a user does
// process.on('exit', process.exit)
process.reallyExit = function (original) {
return function reallyExit (code) {
code = onExitEvent(code)
return original.call(this, code)
}
}(process.reallyExit)
}

var didOnExitEvent = false
function onExitEvent (code) {
if (didOnExitEvent)
return process.exitCode || code

didOnExitEvent = true

if (!tap.results)
tap.endAll()

if (tap.results && !tap.results.ok && code === 0)
process.exitCode = 1

return process.exitCode || code || 0
}

TAP.prototype.push = function push () {
this.pipe(process.stdout)
process.stdout.emit = function (emit) { return function (ev, er) {
if (ev === 'error' && er.code === 'EPIPE')
return this.emit = emit
return emit.apply(this, arguments)
}}(process.stdout.emit)

monkeypatchEpipe()
monkeypatchExit()
process.on('exit', onExitEvent)
process.on('uncaughtException', tap.threw)
return this.push.apply(tap, arguments)
}
Expand Down Expand Up @@ -58,14 +96,6 @@ module.exports = tap
tap.mocha = require('./mocha.js')
tap.mochaGlobals = tap.mocha.global

process.on('exit', function (code) {
if (!tap.results && didPipe)
tap.endAll()

if (tap.results && !tap.results.ok && code === 0)
process.exit(1)
})

tap.Test = Test
tap.Spawn = Spawn
tap.Stdin = Stdin
Expand Down
48 changes: 48 additions & 0 deletions test/test/exit-on-exit--bail--buffer.tap
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
TAP version 13
ok 1 - ___/.*(node|iojs)(.exe)?.*/~~~exit-on-exit.js subtest ___/# time=[0-9.]+(ms)?/~~~ {
ok 1 - memoizes identical registry requests ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - got a manifest
ok 2 - got a manifest
}

ok 2 - tag requests memoize versions ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - (unnamed test)
ok 2 - (unnamed test)
}

ok 3 - tag requests memoize tags ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - got a manifest
ok 2 - got a manifest
}

ok 4 - memoization is scoped to a given cache # TODO
ok 5 - inflights concurrent requests ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - got a manifest
ok 2 - got a manifest
}

ok 6 - supports fetching from an optional cache ___/# time=[0-9.]+(ms)?/~~~ {
ok 1 - should be equivalent
1..1
}

ok 7 - expires stale request data # TODO
ok 8 - falls back to registry if cache entry is invalid JSON # TODO
ok 9 - does not insert plain manifests into the cache # TODO
ok 10 - falls back to registry if cache entry missing ___/# time=[0-9.]+(ms)?/~~~ {
1..0
}

ok 11 - allows forcing use of cache when data stale # TODO
1..11
# todo: 5
___/# time=[0-9.]+(ms)?/~~~
}

1..1
___/# time=[0-9.]+(ms)?/~~~

48 changes: 48 additions & 0 deletions test/test/exit-on-exit--bail.tap
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
TAP version 13
ok 1 - ___/.*(node|iojs)(.exe)?.*/~~~exit-on-exit.js subtest ___/# time=[0-9.]+(ms)?/~~~ {
# Subtest: memoizes identical registry requests
1..2
ok 1 - got a manifest
ok 2 - got a manifest
ok 1 - memoizes identical registry requests ___/# time=[0-9.]+(ms)?/~~~

# Subtest: tag requests memoize versions
1..2
ok 1 - (unnamed test)
ok 2 - (unnamed test)
ok 2 - tag requests memoize versions ___/# time=[0-9.]+(ms)?/~~~

# Subtest: tag requests memoize tags
1..2
ok 1 - got a manifest
ok 2 - got a manifest
ok 3 - tag requests memoize tags ___/# time=[0-9.]+(ms)?/~~~

ok 4 - memoization is scoped to a given cache # TODO
# Subtest: inflights concurrent requests
1..2
ok 1 - got a manifest
ok 2 - got a manifest
ok 5 - inflights concurrent requests ___/# time=[0-9.]+(ms)?/~~~

# Subtest: supports fetching from an optional cache
ok 1 - should be equivalent
1..1
ok 6 - supports fetching from an optional cache ___/# time=[0-9.]+(ms)?/~~~

ok 7 - expires stale request data # TODO
ok 8 - falls back to registry if cache entry is invalid JSON # TODO
ok 9 - does not insert plain manifests into the cache # TODO
# Subtest: falls back to registry if cache entry missing
1..0
ok 10 - falls back to registry if cache entry missing ___/# time=[0-9.]+(ms)?/~~~

ok 11 - allows forcing use of cache when data stale # TODO
1..11
# todo: 5
___/# time=[0-9.]+(ms)?/~~~
}

1..1
___/# time=[0-9.]+(ms)?/~~~

48 changes: 48 additions & 0 deletions test/test/exit-on-exit--buffer.tap
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
TAP version 13
ok 1 - ___/.*(node|iojs)(.exe)?.*/~~~exit-on-exit.js subtest ___/# time=[0-9.]+(ms)?/~~~ {
ok 1 - memoizes identical registry requests ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - got a manifest
ok 2 - got a manifest
}

ok 2 - tag requests memoize versions ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - (unnamed test)
ok 2 - (unnamed test)
}

ok 3 - tag requests memoize tags ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - got a manifest
ok 2 - got a manifest
}

ok 4 - memoization is scoped to a given cache # TODO
ok 5 - inflights concurrent requests ___/# time=[0-9.]+(ms)?/~~~ {
1..2
ok 1 - got a manifest
ok 2 - got a manifest
}

ok 6 - supports fetching from an optional cache ___/# time=[0-9.]+(ms)?/~~~ {
ok 1 - should be equivalent
1..1
}

ok 7 - expires stale request data # TODO
ok 8 - falls back to registry if cache entry is invalid JSON # TODO
ok 9 - does not insert plain manifests into the cache # TODO
ok 10 - falls back to registry if cache entry missing ___/# time=[0-9.]+(ms)?/~~~ {
1..0
}

ok 11 - allows forcing use of cache when data stale # TODO
1..11
# todo: 5
___/# time=[0-9.]+(ms)?/~~~
}

1..1
___/# time=[0-9.]+(ms)?/~~~

61 changes: 61 additions & 0 deletions test/test/exit-on-exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
process.on('exit', function (code) {
process.exit()
})

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

if (process.argv[2] !== 'subtest')
t.spawn(process.execPath, [__filename, 'subtest'], { buffered: true })
else {
var BIG = 10
t.test('memoizes identical registry requests', function (t) {
t.tearDown(function () {})
t.plan(2)
setTimeout(function () {
t.pass('got a manifest')
t.pass('got a manifest')
})
})

t.test('tag requests memoize versions', function (t) {
t.plan(2)
setTimeout(t.pass.bind('got a manifest'))
setTimeout(t.pass.bind('got a manifest'))
})

t.test('tag requests memoize tags', function (t) {
t.plan(2)
t.tearDown(function () {})
setTimeout(function () {
t.pass('got a manifest')
t.pass('got a manifest')
})
})

t.test('memoization is scoped to a given cache')
t.test('inflights concurrent requests', function (t) {
t.plan(2)
t.tearDown(function () {})
setTimeout(function () {
t.pass('got a manifest')
t.pass('got a manifest')
})
})

t.test('supports fetching from an optional cache', function (t) {
t.same(1, '1')
t.tearDown(function () {})
setTimeout(t.end)
})

t.test('expires stale request data')
t.test('falls back to registry if cache entry is invalid JSON')
t.test('does not insert plain manifests into the cache')
t.test('falls back to registry if cache entry missing', function (t) {
t.tearDown(function () {})
setTimeout(function () {
t.end()
})
})
t.test('allows forcing use of cache when data stale')
}
48 changes: 48 additions & 0 deletions test/test/exit-on-exit.tap
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
TAP version 13
ok 1 - ___/.*(node|iojs)(.exe)?.*/~~~exit-on-exit.js subtest ___/# time=[0-9.]+(ms)?/~~~ {
# Subtest: memoizes identical registry requests
1..2
ok 1 - got a manifest
ok 2 - got a manifest
ok 1 - memoizes identical registry requests ___/# time=[0-9.]+(ms)?/~~~

# Subtest: tag requests memoize versions
1..2
ok 1 - (unnamed test)
ok 2 - (unnamed test)
ok 2 - tag requests memoize versions ___/# time=[0-9.]+(ms)?/~~~

# Subtest: tag requests memoize tags
1..2
ok 1 - got a manifest
ok 2 - got a manifest
ok 3 - tag requests memoize tags ___/# time=[0-9.]+(ms)?/~~~

ok 4 - memoization is scoped to a given cache # TODO
# Subtest: inflights concurrent requests
1..2
ok 1 - got a manifest
ok 2 - got a manifest
ok 5 - inflights concurrent requests ___/# time=[0-9.]+(ms)?/~~~

# Subtest: supports fetching from an optional cache
ok 1 - should be equivalent
1..1
ok 6 - supports fetching from an optional cache ___/# time=[0-9.]+(ms)?/~~~

ok 7 - expires stale request data # TODO
ok 8 - falls back to registry if cache entry is invalid JSON # TODO
ok 9 - does not insert plain manifests into the cache # TODO
# Subtest: falls back to registry if cache entry missing
1..0
ok 10 - falls back to registry if cache entry missing ___/# time=[0-9.]+(ms)?/~~~

ok 11 - allows forcing use of cache when data stale # TODO
1..11
# todo: 5
___/# time=[0-9.]+(ms)?/~~~
}

1..1
___/# time=[0-9.]+(ms)?/~~~

0 comments on commit a5a1889

Please sign in to comment.