Skip to content

Commit 3d7f036

Browse files
committed
Prevent log corruption when using mini reporter. Fixes #392.
This prevents interference between the mini logger and child processes that use `console.log`. The mini reporter previously used logUpdate which deletes lines previously written with the logUpdate API before writing a new one. This caused problems if lines from console.log were written inbetween logUpdate calls. It would delete the users log messages instead of the test status line. To fix this, we store the last written last log line, clear it, write the users log output, then restore the last log line. This keeps the miniReporter output always at the bottom of the log output. It also fixes an incorrect use of the `child_process` API. We were using the `stdio` option for `child_process.fork`, but that option is ignored (it is honored for just about every other method in that API). See: https://github.com/nodejs/node/blob/7b355c5bb30d7f9b38654fdb50e58dbd1dcd990a/lib/child_process.js#L49-L50 It also adds a set of visual tests which can be run via `npm run visual`. They must be run manually, and should be run as part of our pre-release process.
1 parent 7b764ea commit 3d7f036

17 files changed

+418
-49
lines changed

api.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,13 @@ Api.prototype._runFile = function (file) {
5555
.on('stats', this._handleStats)
5656
.on('test', this._handleTest)
5757
.on('unhandledRejections', this._handleRejections)
58-
.on('uncaughtException', this._handleExceptions);
58+
.on('uncaughtException', this._handleExceptions)
59+
.on('stdout', this._handleOutput.bind(this, 'stdout'))
60+
.on('stderr', this._handleOutput.bind(this, 'stderr'));
61+
};
62+
63+
Api.prototype._handleOutput = function (channel, data) {
64+
this.emit(channel, data);
5965
};
6066

6167
Api.prototype._handleRejections = function (data) {

cli.js

+3
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ logger.start();
104104
api.on('test', logger.test);
105105
api.on('error', logger.unhandledError);
106106

107+
api.on('stdout', logger.stdout);
108+
api.on('stderr', logger.stderr);
109+
107110
api.run()
108111
.then(function () {
109112
logger.finish();

lib/fork.js

+20-3
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,17 @@ var AvaError = require('./ava-error');
88
var send = require('./send');
99

1010
module.exports = function (file, opts) {
11-
opts = objectAssign({file: file}, opts);
11+
opts = objectAssign({
12+
file: file,
13+
tty: process.stdout.isTTY ? {
14+
columns: process.stdout.columns,
15+
rows: process.stdout.rows
16+
} : false
17+
}, opts);
1218

1319
var ps = childProcess.fork(path.join(__dirname, 'test-worker.js'), [JSON.stringify(opts)], {
1420
cwd: path.dirname(file),
15-
stdio: ['ignore', process.stderr, process.stderr],
16-
silent: opts.silent
21+
silent: true
1722
});
1823

1924
var relFile = path.relative('.', file);
@@ -77,6 +82,18 @@ module.exports = function (file, opts) {
7782
send(ps, 'teardown');
7883
});
7984

85+
ps.stdout.on('data', function (data) {
86+
if (!opts.silent) {
87+
ps.emit('stdout', data);
88+
}
89+
});
90+
91+
ps.stderr.on('data', function (data) {
92+
if (!opts.silent) {
93+
ps.emit('stderr', data);
94+
}
95+
});
96+
8097
promise.on = function () {
8198
ps.on.apply(ps, arguments);
8299

lib/logger.js

+16
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,22 @@ Logger.prototype.write = function (str) {
5353
this.reporter.write(str);
5454
};
5555

56+
Logger.prototype.stdout = function (data) {
57+
if (!this.reporter.stdout) {
58+
return;
59+
}
60+
61+
this.reporter.stdout(data);
62+
};
63+
64+
Logger.prototype.stderr = function (data) {
65+
if (!this.reporter.stderr) {
66+
return;
67+
}
68+
69+
this.reporter.stderr(data);
70+
};
71+
5672
Logger.prototype.exit = function (code) {
5773
// TODO: figure out why this needs to be here to
5874
// correctly flush the output when multiple test files

lib/reporters/mini.js

+81-10
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
'use strict';
2-
var logUpdate = require('log-update');
3-
var colors = require('../colors');
2+
var cliCursor = require('cli-cursor');
3+
var lastLineTracker = require('last-line-stream/tracker');
4+
var StringDecoder = require('string_decoder').StringDecoder;
45
var plur = require('plur');
6+
var colors = require('../colors');
57
var beautifyStack = require('../beautify-stack');
68

79
function MiniReporter() {
@@ -14,7 +16,11 @@ function MiniReporter() {
1416
this.skipCount = 0;
1517
this.rejectionCount = 0;
1618
this.exceptionCount = 0;
17-
this.finished = false;
19+
this.currentStatus = '';
20+
this.statusLineCount = 0;
21+
this.lastLineTracker = lastLineTracker();
22+
this.stream = process.stderr;
23+
this.stringDecoder = new StringDecoder();
1824
}
1925

2026
module.exports = MiniReporter;
@@ -24,7 +30,7 @@ MiniReporter.prototype.start = function () {
2430
};
2531

2632
MiniReporter.prototype.test = function (test) {
27-
var status = '\n';
33+
var status = '';
2834
var title;
2935

3036
if (test.skip) {
@@ -61,9 +67,7 @@ MiniReporter.prototype.unhandledError = function (err) {
6167
};
6268

6369
MiniReporter.prototype.finish = function () {
64-
this.finished = true;
65-
66-
var status = '\n';
70+
var status = '';
6771

6872
if (this.passCount > 0) {
6973
status += ' ' + colors.pass(this.passCount, 'passed');
@@ -134,9 +138,76 @@ MiniReporter.prototype.finish = function () {
134138
};
135139

136140
MiniReporter.prototype.write = function (str) {
137-
logUpdate.stderr(str);
141+
cliCursor.hide();
142+
this.currentStatus = str + '\n';
143+
this._update();
144+
this.statusLineCount = this.currentStatus.split('\n').length;
145+
};
146+
147+
MiniReporter.prototype.stdout = MiniReporter.prototype.stderr = function (data) {
148+
this._update(data);
149+
};
150+
151+
MiniReporter.prototype._update = function (data) {
152+
var str = '';
153+
var ct = this.statusLineCount;
154+
var columns = process.stdout.columns;
155+
var lastLine = this.lastLineTracker.lastLine();
156+
157+
// Terminals automatically wrap text. We only need the last log line as seen on the screen.
158+
lastLine = lastLine.substring(lastLine.length - (lastLine.length % columns));
159+
160+
// Don't delete the last log line if it's completely empty.
161+
if (lastLine.length) {
162+
ct++;
163+
}
164+
165+
// Erase the existing status message, plus the last log line.
166+
str += eraseLines(ct);
167+
168+
// Rewrite the last log line.
169+
str += lastLine;
138170

139-
if (this.finished) {
140-
logUpdate.stderr.done();
171+
if (str.length) {
172+
this.stream.write(str);
173+
}
174+
175+
if (data) {
176+
// send new log data to the terminal, and update the last line status.
177+
this.lastLineTracker.update(this.stringDecoder.write(data));
178+
this.stream.write(data);
179+
}
180+
181+
var currentStatus = this.currentStatus;
182+
if (currentStatus.length) {
183+
lastLine = this.lastLineTracker.lastLine();
184+
// We need a newline at the end of the last log line, before the status message.
185+
// However, if the last log line is the exact width of the terminal a newline is implied,
186+
// and adding a second will cause problems.
187+
if (lastLine.length % columns) {
188+
currentStatus = '\n' + currentStatus;
189+
}
190+
// rewrite the status message.
191+
this.stream.write(currentStatus);
141192
}
142193
};
194+
195+
// TODO(@jamestalamge): This should be fixed in log-update and ansi-escapes once we are confident it's a good solution.
196+
var CSI = '\u001b[';
197+
var ERASE_LINE = CSI + '2K';
198+
var CURSOR_TO_COLUMN_0 = CSI + '0G';
199+
var CURSOR_UP = CSI + '1A';
200+
201+
// Returns a string that will erase `count` lines from the end of the terminal.
202+
function eraseLines(count) {
203+
var clear = '';
204+
205+
for (var i = 0; i < count; i++) {
206+
clear += ERASE_LINE + (i < count - 1 ? CURSOR_UP : '');
207+
}
208+
if (count) {
209+
clear += CURSOR_TO_COLUMN_0;
210+
}
211+
212+
return clear;
213+
}

lib/reporters/tap.js

+4
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,7 @@ TapReporter.prototype.finish = function () {
7878
TapReporter.prototype.write = function (str) {
7979
console.log(str);
8080
};
81+
82+
TapReporter.prototype.stdout = TapReporter.prototype.stderr = function (data) {
83+
process.stderr.write(data);
84+
};

lib/reporters/verbose.js

+4
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,7 @@ VerboseReporter.prototype.finish = function () {
100100
VerboseReporter.prototype.write = function (str) {
101101
console.error(str);
102102
};
103+
104+
VerboseReporter.prototype.stdout = VerboseReporter.prototype.stderr = function (data) {
105+
process.stderr.write(data);
106+
};

lib/test-worker.js

+18-3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,27 @@
11
'use strict';
2+
var opts = JSON.parse(process.argv[2]);
3+
var testPath = opts.file;
4+
5+
// Fake TTY support
6+
if (opts.tty) {
7+
process.stdout.isTTY = true;
8+
process.stdout.columns = opts.tty.columns || 80;
9+
process.stdout.rows = opts.tty.rows;
10+
var tty = require('tty');
11+
var isatty = tty.isatty;
12+
tty.isatty = function (fd) {
13+
if (fd === 1 || fd === process.stdout) {
14+
return true;
15+
}
16+
return isatty(fd);
17+
};
18+
}
19+
220
var path = require('path');
321
var fs = require('fs');
422
var debug = require('debug')('ava');
523
var sourceMapSupport = require('source-map-support');
624

7-
var opts = JSON.parse(process.argv[2]);
8-
var testPath = opts.file;
9-
1025
if (debug.enabled) {
1126
// Forward the `time-require` `--sorted` flag.
1227
// Intended for internal optimization tests only.

package.json

+7-3
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
},
3939
"scripts": {
4040
"test": "xo && nyc --cache --reporter=lcov --reporter=text tap --no-cov --timeout=150 test/*.js test/reporters/*.js",
41-
"test-win": "tap --no-cov --reporter=classic --timeout=150 test/*.js test/reporters/*.js"
41+
"test-win": "tap --no-cov --reporter=classic --timeout=150 test/*.js test/reporters/*.js",
42+
"visual": "node test/visual/run-visual-tests.js"
4243
},
4344
"files": [
4445
"lib",
@@ -87,6 +88,7 @@
8788
"bluebird": "^3.0.0",
8889
"caching-transform": "^1.0.0",
8990
"chalk": "^1.0.0",
91+
"cli-cursor": "^1.0.2",
9092
"co-with-promise": "^4.6.0",
9193
"commondir": "^1.0.1",
9294
"convert-source-map": "^1.1.2",
@@ -101,7 +103,7 @@
101103
"is-generator-fn": "^1.0.0",
102104
"is-observable": "^0.1.0",
103105
"is-promise": "^2.1.0",
104-
"log-update": "^1.0.2",
106+
"last-line-stream": "^1.0.0",
105107
"loud-rejection": "^1.2.0",
106108
"max-timeout": "^1.0.0",
107109
"md5-hex": "^1.2.0",
@@ -127,8 +129,10 @@
127129
"coveralls": "^2.11.4",
128130
"delay": "^1.3.0",
129131
"get-stream": "^1.1.0",
130-
"rimraf": "^2.5.0",
132+
"inquirer": "^0.11.1",
131133
"nyc": "^5.1.0",
134+
"pify": "^2.3.0",
135+
"rimraf": "^2.5.0",
132136
"signal-exit": "^2.1.2",
133137
"sinon": "^1.17.2",
134138
"source-map-fixtures": "^0.4.0",

0 commit comments

Comments
 (0)