From 0a0024e804375eca5ac7fc968df74818c284f277 Mon Sep 17 00:00:00 2001 From: Ben Schmidt Date: Tue, 19 Jul 2016 13:50:27 +1000 Subject: [PATCH 1/4] tty: add ref() so process.stdin.ref() etc. work --- src/tty_wrap.cc | 1 + test/README.md | 5 ++++ test/pseudo-tty/ref_keeps_node_running.js | 27 ++++++++++++++++++++++ test/pseudo-tty/ref_keeps_node_running.out | 0 4 files changed, 33 insertions(+) create mode 100644 test/pseudo-tty/ref_keeps_node_running.js create mode 100644 test/pseudo-tty/ref_keeps_node_running.out diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 26f061b99b34e8..5b7518324773ae 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -34,6 +34,7 @@ void TTYWrap::Initialize(Local target, env->SetProtoMethod(t, "close", HandleWrap::Close); env->SetProtoMethod(t, "unref", HandleWrap::Unref); + env->SetProtoMethod(t, "ref", HandleWrap::Ref); env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef); StreamWrap::AddMethods(env, t, StreamBase::kFlagNoShutdown); diff --git a/test/README.md b/test/README.md index b79a8efb795f44..6ab0a21976de5c 100644 --- a/test/README.md +++ b/test/README.md @@ -105,6 +105,11 @@ On how to run tests in this direcotry, see Yes Various tests that are able to be run in parallel. + + pseudo-tty + Yes + Tests that require stdin/stdout/stderr to be a TTY. + pummel No diff --git a/test/pseudo-tty/ref_keeps_node_running.js b/test/pseudo-tty/ref_keeps_node_running.js new file mode 100644 index 00000000000000..de3c6531ef6924 --- /dev/null +++ b/test/pseudo-tty/ref_keeps_node_running.js @@ -0,0 +1,27 @@ +'use strict'; +require('../common'); + +const { TTY, isTTY } = process.binding('tty_wrap'); +const strictEqual = require('assert').strictEqual; + +strictEqual(isTTY(0), true, 'fd 0 is not a TTY'); + +const handle = new TTY(0); +handle.readStart(); +handle.onread = () => {}; + +function isHandleActive(handle) { + return process._getActiveHandles().some((active) => active === handle); +} + +strictEqual(isHandleActive(handle), true, 'TTY handle not initially active'); + +handle.unref(); + +strictEqual(isHandleActive(handle), false, 'TTY handle active after unref()'); + +handle.ref(); + +strictEqual(isHandleActive(handle), true, 'TTY handle inactive after ref()'); + +handle.unref(); diff --git a/test/pseudo-tty/ref_keeps_node_running.out b/test/pseudo-tty/ref_keeps_node_running.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 From 5dcc6a985c562862868b3030c39dd40d2bb085ae Mon Sep 17 00:00:00 2001 From: Ben Schmidt Date: Tue, 19 Jul 2016 15:40:24 +1000 Subject: [PATCH 2/4] test: move tty-wrap isrefed test to pseudo-tty/ --- test/{parallel => pseudo-tty}/test-handle-wrap-isrefed-tty.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{parallel => pseudo-tty}/test-handle-wrap-isrefed-tty.js (100%) diff --git a/test/parallel/test-handle-wrap-isrefed-tty.js b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js similarity index 100% rename from test/parallel/test-handle-wrap-isrefed-tty.js rename to test/pseudo-tty/test-handle-wrap-isrefed-tty.js From 18d9be21826ad098f30c8e2643e5c8194aae7c78 Mon Sep 17 00:00:00 2001 From: Ben Schmidt Date: Tue, 19 Jul 2016 15:40:58 +1000 Subject: [PATCH 3/4] test: test tty-wrap handle isrefed properly --- test/parallel/test-handle-wrap-isrefed.js | 3 ++ .../test-handle-wrap-isrefed-tty.js | 36 ++++++++----------- .../test-handle-wrap-isrefed-tty.out | 0 3 files changed, 18 insertions(+), 21 deletions(-) create mode 100644 test/pseudo-tty/test-handle-wrap-isrefed-tty.out diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js index b5dbeb23bfd63a..007e27c6cd0e11 100644 --- a/test/parallel/test-handle-wrap-isrefed.js +++ b/test/parallel/test-handle-wrap-isrefed.js @@ -99,3 +99,6 @@ function makeAssert(message) { timer._handle.close( common.mustCall(() => assert(timer._handle.hasRef(), false))); } + + +// see also test/pseudo-tty/test-handle-wrap-isrefed-tty.js diff --git a/test/pseudo-tty/test-handle-wrap-isrefed-tty.js b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js index ad312be1f77862..815e71cc7443a7 100644 --- a/test/pseudo-tty/test-handle-wrap-isrefed-tty.js +++ b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js @@ -1,33 +1,27 @@ 'use strict'; +// see also test/parallel/test-handle-wrap-isrefed.js + const common = require('../common'); const strictEqual = require('assert').strictEqual; -const spawn = require('child_process').spawn; function makeAssert(message) { return function(actual, expected) { strictEqual(actual, expected, message); }; } -const assert = makeAssert('hasRef() not working on tty_wrap'); -if (process.argv[2] === 'child') { - // Test tty_wrap in piped child to guarentee stdin being a TTY. - const ReadStream = require('tty').ReadStream; - const tty = new ReadStream(0); - assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'), true); - assert(tty._handle.hasRef(), true); - tty.unref(); - assert(tty._handle.hasRef(), false); - tty._handle.close( - common.mustCall(() => assert(tty._handle.hasRef(), false))); - return; -} +const assert = makeAssert('hasRef() not working on tty_wrap'); -// Use spawn so that we can be sure that stdin has a _handle property. -// Refs: https://github.com/nodejs/node/pull/5916 -const proc = spawn(process.execPath, [__filename, 'child'], { stdio: 'pipe' }); -proc.stderr.pipe(process.stderr); -proc.on('exit', common.mustCall(function(exitCode) { - process.exitCode = exitCode; -})); +const ReadStream = require('tty').ReadStream; +const tty = new ReadStream(0); +const isTTY = process.binding('tty_wrap').isTTY; +assert(isTTY(0), true); +assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'), true); +assert(tty._handle.hasRef(), true); +tty.unref(); +assert(tty._handle.hasRef(), false); +tty.ref(); +assert(tty._handle.hasRef(), true); +tty._handle.close( + common.mustCall(() => assert(tty._handle.hasRef(), false))); diff --git a/test/pseudo-tty/test-handle-wrap-isrefed-tty.out b/test/pseudo-tty/test-handle-wrap-isrefed-tty.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 From c81f71069c811fe202e8da0c3df7544d6337e039 Mon Sep 17 00:00:00 2001 From: Ben Schmidt Date: Thu, 23 Feb 2017 08:28:45 +1100 Subject: [PATCH 4/4] test: improve failure messages in isrefed tests --- test/parallel/test-handle-wrap-isrefed.js | 126 +++++++++++------- .../test-handle-wrap-isrefed-tty.js | 28 ++-- 2 files changed, 88 insertions(+), 66 deletions(-) diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js index 007e27c6cd0e11..66353fcc0362b3 100644 --- a/test/parallel/test-handle-wrap-isrefed.js +++ b/test/parallel/test-handle-wrap-isrefed.js @@ -3,101 +3,127 @@ const common = require('../common'); const strictEqual = require('assert').strictEqual; -function makeAssert(message) { - return function(actual, expected) { - strictEqual(actual, expected, message); - }; -} - - // child_process { - const assert = makeAssert('hasRef() not working on process_wrap'); const spawn = require('child_process').spawn; const cmd = common.isWindows ? 'rundll32' : 'ls'; const cp = spawn(cmd); - assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('hasRef'), true); - assert(cp._handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(cp._handle).hasOwnProperty('hasRef'), + true, 'process_wrap: hasRef() missing'); + strictEqual(cp._handle.hasRef(), + true, 'process_wrap: not initially refed'); cp.unref(); - assert(cp._handle.hasRef(), false); + strictEqual(cp._handle.hasRef(), + false, 'process_wrap: unref() ineffective'); cp.ref(); - assert(cp._handle.hasRef(), true); - cp._handle.close(common.mustCall(() => assert(cp._handle.hasRef(), false))); + strictEqual(cp._handle.hasRef(), + true, 'process_wrap: ref() ineffective'); + cp._handle.close(common.mustCall(() => + strictEqual(cp._handle.hasRef(), + false, 'process_wrap: not unrefed on close'))); } -// dgram +// dgram ipv4 { - const assert = makeAssert('hasRef() not working on udp_wrap'); const dgram = require('dgram'); - const sock4 = dgram.createSocket('udp4'); - assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('hasRef'), true); - assert(sock4._handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(sock4._handle).hasOwnProperty('hasRef'), + true, 'udp_wrap: ipv4: hasRef() missing'); + strictEqual(sock4._handle.hasRef(), + true, 'udp_wrap: ipv4: not initially refed'); sock4.unref(); - assert(sock4._handle.hasRef(), false); + strictEqual(sock4._handle.hasRef(), + false, 'udp_wrap: ipv4: unref() ineffective'); sock4.ref(); - assert(sock4._handle.hasRef(), true); - sock4._handle.close( - common.mustCall(() => assert(sock4._handle.hasRef(), false))); + strictEqual(sock4._handle.hasRef(), + true, 'udp_wrap: ipv4: ref() ineffective'); + sock4._handle.close(common.mustCall(() => + strictEqual(sock4._handle.hasRef(), + false, 'udp_wrap: ipv4: not unrefed on close'))); +} + +// dgram ipv6 +{ + const dgram = require('dgram'); const sock6 = dgram.createSocket('udp6'); - assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('hasRef'), true); - assert(sock6._handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(sock6._handle).hasOwnProperty('hasRef'), + true, 'udp_wrap: ipv6: hasRef() missing'); + strictEqual(sock6._handle.hasRef(), + true, 'udp_wrap: ipv6: not initially refed'); sock6.unref(); - assert(sock6._handle.hasRef(), false); + strictEqual(sock6._handle.hasRef(), + false, 'udp_wrap: ipv6: unref() ineffective'); sock6.ref(); - assert(sock6._handle.hasRef(), true); - sock6._handle.close( - common.mustCall(() => assert(sock6._handle.hasRef(), false))); + strictEqual(sock6._handle.hasRef(), + true, 'udp_wrap: ipv6: ref() ineffective'); + sock6._handle.close(common.mustCall(() => + strictEqual(sock6._handle.hasRef(), + false, 'udp_wrap: ipv6: not unrefed on close'))); } // pipe { - const assert = makeAssert('hasRef() not working on pipe_wrap'); const Pipe = process.binding('pipe_wrap').Pipe; const handle = new Pipe(); - assert(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), true); - assert(handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), + true, 'pipe_wrap: hasRef() missing'); + strictEqual(handle.hasRef(), + true, 'pipe_wrap: not initially refed'); handle.unref(); - assert(handle.hasRef(), false); + strictEqual(handle.hasRef(), + false, 'pipe_wrap: unref() ineffective'); handle.ref(); - assert(handle.hasRef(), true); - handle.close(common.mustCall(() => assert(handle.hasRef(), false))); + strictEqual(handle.hasRef(), + true, 'pipe_wrap: ref() ineffective'); + handle.close(common.mustCall(() => + strictEqual(handle.hasRef(), + false, 'pipe_wrap: not unrefed on close'))); } // tcp { - const assert = makeAssert('hasRef() not working on tcp_wrap'); const net = require('net'); const server = net.createServer(() => {}).listen(0); - assert(Object.getPrototypeOf(server._handle).hasOwnProperty('hasRef'), true); - assert(server._handle.hasRef(), true); - assert(server._unref, false); + strictEqual(Object.getPrototypeOf(server._handle).hasOwnProperty('hasRef'), + true, 'tcp_wrap: hasRef() missing'); + strictEqual(server._handle.hasRef(), + true, 'tcp_wrap: not initially refed'); + strictEqual(server._unref, + false, 'tcp_wrap: _unref initially incorrect'); server.unref(); - assert(server._handle.hasRef(), false); - assert(server._unref, true); + strictEqual(server._handle.hasRef(), + false, 'tcp_wrap: unref() ineffective'); + strictEqual(server._unref, + true, 'tcp_wrap: _unref not updated on unref()'); server.ref(); - assert(server._handle.hasRef(), true); - assert(server._unref, false); - server._handle.close( - common.mustCall(() => assert(server._handle.hasRef(), false))); + strictEqual(server._handle.hasRef(), + true, 'tcp_wrap: ref() ineffective'); + strictEqual(server._unref, + false, 'tcp_wrap: _unref not updated on ref()'); + server._handle.close(common.mustCall(() => + strictEqual(server._handle.hasRef(), + false, 'tcp_wrap: not unrefed on close'))); } // timers { - const assert = makeAssert('hasRef() not working on timer_wrap'); const timer = setTimeout(() => {}, 500); timer.unref(); - assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), true); - assert(timer._handle.hasRef(), false); + strictEqual(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), + true, 'timer_wrap: hasRef() missing'); + strictEqual(timer._handle.hasRef(), + false, 'timer_wrap: unref() ineffective'); timer.ref(); - assert(timer._handle.hasRef(), true); - timer._handle.close( - common.mustCall(() => assert(timer._handle.hasRef(), false))); + strictEqual(timer._handle.hasRef(), + true, 'timer_wrap: ref() ineffective'); + timer._handle.close(common.mustCall(() => + strictEqual(timer._handle.hasRef(), + false, 'timer_wrap: not unrefed on close'))); } diff --git a/test/pseudo-tty/test-handle-wrap-isrefed-tty.js b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js index 815e71cc7443a7..4baf6200962cc3 100644 --- a/test/pseudo-tty/test-handle-wrap-isrefed-tty.js +++ b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js @@ -4,24 +4,20 @@ const common = require('../common'); const strictEqual = require('assert').strictEqual; - -function makeAssert(message) { - return function(actual, expected) { - strictEqual(actual, expected, message); - }; -} - -const assert = makeAssert('hasRef() not working on tty_wrap'); - const ReadStream = require('tty').ReadStream; const tty = new ReadStream(0); const isTTY = process.binding('tty_wrap').isTTY; -assert(isTTY(0), true); -assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'), true); -assert(tty._handle.hasRef(), true); +strictEqual(isTTY(0), true, 'tty_wrap: stdin is not a TTY'); +strictEqual(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'), + true, 'tty_wrap: hasRef() missing'); +strictEqual(tty._handle.hasRef(), + true, 'tty_wrap: not initially refed'); tty.unref(); -assert(tty._handle.hasRef(), false); +strictEqual(tty._handle.hasRef(), + false, 'tty_wrap: unref() ineffective'); tty.ref(); -assert(tty._handle.hasRef(), true); -tty._handle.close( - common.mustCall(() => assert(tty._handle.hasRef(), false))); +strictEqual(tty._handle.hasRef(), + true, 'tty_wrap: ref() ineffective'); +tty._handle.close(common.mustCall(() => + strictEqual(tty._handle.hasRef(), + false, 'tty_wrap: not unrefed on close')));