Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
28 changes: 28 additions & 0 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,26 @@ Checks whether `IPv6` is supported on this platform.

Checks if there are multiple localhosts available.

### hijackStderr(listener)
* `listener`
[<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Normal_objects_and_functions):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(I know everywhere else uses it this way but) if you are using this link more than one you could use [MDN-Function][], and put

[MDN-Function]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Normal_objects_and_functions

the the bottom of the file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👌

a listener with a single parameter called `data`.

Hijack `process.stderr` to listen `write` action. Once `process.stderr.write` is
Copy link
Copy Markdown
Contributor

@refack refack Jun 4, 2017

Choose a reason for hiding this comment

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

Hijack `process.stderr` to listen `write` action => Eavesdrop to `process.stderr.write` calls

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👌

called, `listener` will also be called and the `data` of `write` function will
be passed to `listener`. What's more, `process.stderr.writeTimes` will plus one
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... `process.stderr.writeTimes` is a count of the number of calls

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👌

then.

### hijackStdout(listener)
* `listener`
[<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Normal_objects_and_functions):
a listener with a single parameter called `data`.

Hijack `process.stdout` to listen `write` action. Once `process.stdout.write` is
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above.

IMHO since this is an "internal" guide you could merge then into
### hijackStdout/Stderr(listener)

called, `listener` will also be called and the `data` of `write` function will
be passed to `listener`. What's more, `process.stdout.writeTimes` will plus one
then.

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

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

Deletes the 'tmp' dir and recreates it

### restoreStderr
Copy link
Copy Markdown
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
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.bind(stream);

stream.writeTimes = 0;
stream.write = function(data, callback) {
listener(data);
_write(data, callback);
stream.writeTimes++;
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👌

Copy link
Copy Markdown
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');
8 changes: 7 additions & 1 deletion test/fixtures/echo-close-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ const fs = require('fs');

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

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

let current;
common.hijackStdout(common.mustCall(function(data) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't this this is an interesting use of hijackStdout, it just tests the mechanics of hijackStdout.
BTW: you should do that in explicitly in test/parallel/test-common.js

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👌

assert.strictEqual(data, current);
}));

stdin.on('data', function(data) {
current = data.toString();
process.stdout.write(data.toString());
});

Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/echo.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ process.stdout.write('hello world\r\n');

var stdin = process.openStdin();

let current;
stdin.on('data', function(data) {
current = data;
process.stdout.write(data.toString());
});

common.hijackStdout(function(data) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

assert.equal(data, current);
});
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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(/no such label/.test(data));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like assert.strictEqual(data.includes('no such label'), true) better

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👌

});
}));
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();