Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add hijackStdout and hijackStderr #13439

Closed
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,14 @@ Checks whether `IPv6` is supported on this platform.

Checks if there are multiple localhosts available.

### hijackStderr/Stdout(listener)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be listed as separate functions (even if it means duplicating the text)

Copy link
Contributor

@refack refack Jun 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell I suggested the the merge (since it's an internal guide) #13439 (comment)
Please 👍 / 👎 again, just so we don't ask @XadillaX to do unnecessary work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep the style consistent throughout, so would really prefer them separate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.
Sorry @XadillaX

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

* `listener` [<Function>][MDN-Function]: a listener with a single parameter called `data`.

Eavesdrop to `process.stderr.write` or `process.stdout.write` calls. Once
`process.std*.write` is called, `listener` will also be called and the `data` of
`write` function will be passed to `listener`. What's more,
`process.std*.writeTimes` is a count of the number of calls.

### inFreeBSDJail
* return [<Boolean>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type)

Expand Down Expand Up @@ -250,6 +258,14 @@ Port tests are running on.

Deletes the 'tmp' dir and recreates it

### restoreStderr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are functions, then please include the ()


Restore the original `process.stderr.write`.

### restoreStdout

Restore the original `process.stdout.write`.

### rootDir
* return [<String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)

Expand Down Expand Up @@ -290,3 +306,5 @@ Node.js
[WHATWG URL API](https://nodejs.org/api/url.html#url_the_whatwg_url_api)
implementation with tests from
[W3C Web Platform Tests](https://github.com/w3c/web-platform-tests).

[MDN-Function]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Normal_objects_and_functions
24 changes: 24 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -759,3 +759,27 @@ exports.getTTYfd = function getTTYfd() {
}
return tty_fd;
};

// Hijack stdout and stderr
const stdWrite = {};
function hijackStdWritable(name, listener) {
const stream = process[name];
const _write = stdWrite[name] = stream.write;

stream.writeTimes = 0;
stream.write = function(data, callback) {
listener(data);
_write.call(stream, data, callback);
stream.writeTimes++;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic seem not go well with the writable.write specification:

callback (Function) :Callback for when this chunk of data is flushed

Your logic will cause:

  • Issue the completion handler before the actual write (_write)
  • Listener will be called in the same loop tick, making it a direct call, not a callback.

I propose:
_wite(data, listener)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, listener is something like .emit('write') but not .emit('written'), so it's sync.

And I've added the callback in _write now.

};
}

function restoreWritable(name) {
process[name].write = stdWrite[name];
delete process[name].writeTimes;
}

exports.hijackStdout = hijackStdWritable.bind(null, 'stdout');
exports.hijackStderr = hijackStdWritable.bind(null, 'stderr');
exports.restoreStdout = restoreWritable.bind(null, 'stdout');
exports.restoreStderr = restoreWritable.bind(null, 'stderr');
2 changes: 1 addition & 1 deletion test/fixtures/echo-close-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const fs = require('fs');

process.stdout.write('hello world\r\n');

var stdin = process.openStdin();
const stdin = process.openStdin();

stdin.on('data', function(data) {
process.stdout.write(data.toString());
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,23 @@ for (const p of failFixtures) {
assert.strictEqual(firstLine, expected);
}));
}

// hijackStderr and hijackStdout
const HIJACK_TEST_ARRAY = [ 'foo\n', 'bar\n', 'baz\n' ];
[ 'err', 'out' ].forEach((txt) => {
const stream = process[`std${txt}`];
const originalWrite = stream.write;

common[`hijackStd${txt}`](common.mustCall(function(data) {
assert.strictEqual(data, HIJACK_TEST_ARRAY[stream.writeTimes]);
}, HIJACK_TEST_ARRAY.length));
assert.notStrictEqual(originalWrite, stream.write);

HIJACK_TEST_ARRAY.forEach((val) => {
stream.write(val, common.mustCall(common.noop));
});

assert.strictEqual(HIJACK_TEST_ARRAY.length, stream.writeTimes);
common[`restoreStd${txt}`]();
assert.strictEqual(originalWrite, stream.write);
});
34 changes: 21 additions & 13 deletions test/parallel/test-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,14 @@ assert.doesNotThrow(function() {
// an Object with a custom .inspect() function
const custom_inspect = { foo: 'bar', inspect: () => 'inspect' };

const stdout_write = global.process.stdout.write;
const stderr_write = global.process.stderr.write;
const strings = [];
const errStrings = [];
global.process.stdout.write = function(string) {
strings.push(string);
};
global.process.stderr.write = function(string) {
errStrings.push(string);
};
common.hijackStdout(function(data) {
strings.push(data);
});
common.hijackStderr(function(data) {
errStrings.push(data);
});

// test console.log() goes to stdout
console.log('foo');
Expand Down Expand Up @@ -105,8 +103,10 @@ console.timeEnd('constructor');
console.time('hasOwnProperty');
console.timeEnd('hasOwnProperty');

global.process.stdout.write = stdout_write;
global.process.stderr.write = stderr_write;
assert.strictEqual(strings.length, process.stdout.writeTimes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this fail after you implemented @gireeshpunathil's async comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, listener is sync which means a write action is emitted but not finished.

I just added the callback to the original _write function to make it may receivecallback.

assert.strictEqual(errStrings.length, process.stderr.writeTimes);
common.restoreStdout();
common.restoreStderr();

// verify that console.timeEnd() doesn't leave dead links
const timesMapSize = console._times.size;
Expand Down Expand Up @@ -146,9 +146,6 @@ assert.ok(/^hasOwnProperty: \d+\.\d{3}ms$/.test(strings.shift().trim()));
assert.strictEqual('Trace: This is a {"formatted":"trace"} 10 foo',
errStrings.shift().split('\n').shift());

assert.strictEqual(strings.length, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this.
But I assume you'll need to solve async issues with this whole block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same which mentioned before.

And assert.strictEqual(strings.length, 0) equals above assert.strictEqual(strings.length, process.stdout.writeTimes);.

assert.strictEqual(errStrings.length, 0);

assert.throws(() => {
console.assert(false, 'should throw');
}, common.expectsError({
Expand All @@ -159,3 +156,14 @@ assert.throws(() => {
assert.doesNotThrow(() => {
console.assert(true, 'this should not throw');
});

// hijack stderr to catch `process.emitWarning` which is using
// `process.nextTick`
common.hijackStderr(common.mustCall(function(data) {
common.restoreStderr();

// stderr.write will catch sync error, so use `process.nextTick` here
process.nextTick(function() {
assert.strictEqual(data.includes('no such label'), true);
});
}));
31 changes: 12 additions & 19 deletions test/parallel/test-global-console-exists.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,30 @@
const common = require('../common');
const assert = require('assert');
const EventEmitter = require('events');
const leak_warning = /EventEmitter memory leak detected\. 2 hello listeners/;
const leakWarning = /EventEmitter memory leak detected\. 2 hello listeners/;

let write_calls = 0;
common.hijackStderr(common.mustCall(function(data) {
if (process.stderr.writeTimes === 0) {
assert.ok(data.match(leakWarning));
} else {
assert.fail('stderr.write should be called only once');
}
}));

process.on('warning', (warning) => {
process.on('warning', function(warning) {
// This will be called after the default internal
// process warning handler is called. The default
// process warning writes to the console, which will
// invoke the monkeypatched process.stderr.write
// below.
assert.strictEqual(write_calls, 1);
EventEmitter.defaultMaxListeners = old_default;
assert.strictEqual(process.stderr.writeTimes, 1);
EventEmitter.defaultMaxListeners = oldDefault;
// when we get here, we should be done
});

process.stderr.write = (data) => {
if (write_calls === 0)
assert.ok(data.match(leak_warning));
else
assert.fail('stderr.write should be called only once');

write_calls++;
};

const old_default = EventEmitter.defaultMaxListeners;
const oldDefault = EventEmitter.defaultMaxListeners;
EventEmitter.defaultMaxListeners = 1;

const e = new EventEmitter();
e.on('hello', common.noop);
e.on('hello', common.noop);

// TODO: Figure out how to validate console. Currently,
// there is no obvious way of validating that console
// exists here exactly when it should.
7 changes: 2 additions & 5 deletions test/parallel/test-process-raw-debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const os = require('os');

Expand Down Expand Up @@ -63,10 +63,7 @@ function child() {
throw new Error('No ticking!');
};

const stderr = process.stderr;
stderr.write = function() {
throw new Error('No writing to stderr!');
};
common.hijackStderr(common.mustNotCall('stderr.write must not be called.'));

process._rawDebug('I can still %s!', 'debug');
}
15 changes: 8 additions & 7 deletions test/parallel/test-util-log.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,18 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const util = require('util');

assert.ok(process.stdout.writable);
assert.ok(process.stderr.writable);

const stdout_write = global.process.stdout.write;
const strings = [];
global.process.stdout.write = function(string) {
strings.push(string);
};
console._stderr = process.stdout;
common.hijackStdout(function(data) {
strings.push(data);
});
common.hijackStderr(common.mustNotCall('stderr.write must not be called'));

const tests = [
{input: 'foo', output: 'foo'},
Expand All @@ -56,4 +55,6 @@ tests.forEach(function(test) {
assert.strictEqual(match[1], test.output);
});

global.process.stdout.write = stdout_write;
assert.strictEqual(process.stdout.writeTimes, tests.length);

common.restoreStdout();