From 7ec72e30c852ee1ee87ac956e1ac2721fa0a3ccd Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 7 Sep 2020 23:22:23 +0100 Subject: [PATCH] Reporter: Support multi-line strings in TAP failures data This is a different approach to fixing https://github.com/js-reporters/js-reporters/issues/109. The issue was previously fixed in js-reporters 1.2.2 by https://github.com/js-reporters/js-reporters/pull/110 but that made multi-line strings difficult to read and left numerous escape hatches in place. That fix has since been reverted to keep 1.x behaving the same has before. The new approach will be part of 2.0. Fixes https://github.com/js-reporters/js-reporters/issues/109. --- lib/reporters/TapReporter.js | 134 ++++++++++++++++++++++++++++++++--- test/unit/data.js | 33 ++++++++- test/unit/tap-reporter.js | 21 ++---- 3 files changed, 163 insertions(+), 25 deletions(-) diff --git a/lib/reporters/TapReporter.js b/lib/reporters/TapReporter.js index 6a844e6..3c76c50 100644 --- a/lib/reporters/TapReporter.js +++ b/lib/reporters/TapReporter.js @@ -2,6 +2,121 @@ const chalk = require('chalk'); const hasOwn = Object.hasOwnProperty; +/** + * Format a given value into YAML. + * + * YAML is a superset of JSON that supports all the same data + * types and syntax, and more. As such, it is always possible + * to fallback to JSON.stringfify, but we generally avoid + * that to make output easier to read for humans. + * + * Supported data types: + * + * - null + * - boolean + * - number + * - string + * - array + * - object + * + * Anything else (including NaN, Infinity, and undefined) + * must be described in strings, for display purposes. + * + * Note that quotes are optional in YAML strings if the + * strings are "simple", and as such we generally prefer + * that for improved readability. We output strings in + * one of three ways: + * + * - bare unquoted text, for simple one-line strings. + * - JSON (quoted text), for complex one-line strings. + * - YAML Block, for complex multi-line strings. + */ +function prettyYamlValue (value, indent = 4) { + if (value === undefined) { + // Not supported in JSON/YAML, turn into string + // and let the below output it as bare string. + value = String(value); + } + + if (typeof value === 'number' && !Number.isFinite(value)) { + // Turn NaN and Infinity into simple strings. + // Paranoia: Don't return directly just in case there's + // a way to add special characters here. + value = String(value); + } + + if (typeof value === 'number') { + // Simple numbers + return JSON.stringify(value); + } + + if (typeof value === 'string') { + // If any of these match, then we can't output it + // as bare unquoted text, because that would either + // cause data loss or invalid YAML syntax. + // + // - Quotes, escapes, line breaks, or JSON-like stuff. + const rSpecialJson = /['"\\/[{}\]\r\n]/; + // - Characters that are special at the start of a YAML value + const rSpecialYaml = /[-?:,[\]{}#&*!|=>'"%@`]/; + // - Leading or trailing whitespace. + const rUntrimmed = /(^\s|\s$)/; + // - Ambiguous as YAML number, e.g. '2', '-1.2', '.2', or '2_000' + const rNumerical = /^[\d._-]+$/; + // - Ambiguous as YAML bool. + // Use case-insensitive match, although technically only + // fully-lower, fully-upper, or uppercase-first would be ambiguous. + // e.g. true/True/TRUE, but not tRUe. + const rBool = /^(true|false|y|n|yes|no|on|off)$/i; + + // Is this a complex string? + if ( + value === '' || + rSpecialJson.test(value) || + rSpecialYaml.test(value[0]) || + rUntrimmed.test(value) || + rNumerical.test(value) || + rBool.test(value) + ) { + if (!/\n/.test(value)) { + // Complex one-line string, use JSON (quoted string) + return JSON.stringify(value); + } + + // See also + const prefix = ' '.repeat(indent); + + const trailingLinebreakMatch = value.match(/\n+$/); + const trailingLinebreaks = trailingLinebreakMatch ? trailingLinebreakMatch[0].length : 0; + + if (trailingLinebreaks === 1) { + // Use the most straight-forward "Block" string in YAML + // without any "Chomping" indicators. + const lines = value + // Ignore the last new line, since we'll get that one for free + // with the straight-forward Block syntax. + .replace(/\n$/, '') + .split('\n') + .map(line => prefix + line); + return '|\n' + lines.join('\n'); + } else { + // This has either no trailing new lines, or more than 1. + // Use |+ so that YAML parsers will preserve it exactly. + const lines = value + .split('\n') + .map(line => prefix + line); + return '|+\n' + lines.join('\n'); + } + } else { + // Simple string, use bare unquoted text + return value; + } + } + + // Handle null, boolean, array, and object + return JSON.stringify(value, null, 2); +} + module.exports = class TapReporter { constructor (runner) { this.testCount = 0; @@ -44,24 +159,25 @@ module.exports = class TapReporter { } logError (error, severity) { - console.log(' ---'); - console.log(` message: "${(error.message || 'failed').replace(/"/g, '\\"')}"`); - console.log(` severity: ${severity || 'failed'}`); + let out = ' ---'; + out += `\n message: ${prettyYamlValue(error.message || 'failed')}`; + out += `\n severity: ${prettyYamlValue(severity || 'failed')}`; if (hasOwn.call(error, 'actual')) { - const actualStr = error.actual !== undefined ? ('"' + JSON.stringify(error.actual, null, 2).replace(/"/g, '\\"').replace(/\n/g, '\\n') + '"') : 'undefined'; - console.log(` actual : ${actualStr}`); + out += `\n actual : ${prettyYamlValue(error.actual)}`; } if (hasOwn.call(error, 'expected')) { - const expectedStr = error.expected !== undefined ? ('"' + JSON.stringify(error.expected, null, 2).replace(/"/g, '\\"').replace(/\n/g, '\\n') + '"') : 'undefined'; - console.log(` expected: ${expectedStr}`); + out += `\n expected: ${prettyYamlValue(error.expected)}`; } if (error.stack) { - console.log(` stack: "${error.stack.replace(/"/g, '\\"').replace(/\n/g, '\\n')}"`); + // Since stacks aren't user generated, take a bit of liberty by + // adding a trailing new line to allow a straight-forward YAML Blocks. + out += `\n stack: ${prettyYamlValue(error.stack + '\n')}`; } - console.log(' ...'); + out += '\n ...'; + console.log(out); } }; diff --git a/test/unit/data.js b/test/unit/data.js index b11ab42..2df1caf 100644 --- a/test/unit/data.js +++ b/test/unit/data.js @@ -1,10 +1,41 @@ const { TestEnd, TestStart, SuiteStart, SuiteEnd } = require('../../'); +function mockStack (error) { + error.stack = ` at Object. (/dev/null/test/unit/data.js:6:5) + at require (internal/modules/cjs/helpers.js:22:18) + at /dev/null/node_modules/mocha/lib/mocha.js:220:27 + at startup (internal/bootstrap/node.js:283:19) + at bootstrapNodeJSCore (internal/bootstrap/node.js:743:3)`; + return error; +} + module.exports = { passingTest: new TestEnd('pass', undefined, [], 'passed', 0, []), failingTest: new TestEnd('fail', undefined, [], 'failed', 0, [ - new Error('first error'), new Error('second error') + mockStack(new Error('first error')), mockStack(new Error('second error')) ]), + failingTapData: [ + ` --- + message: first error + severity: failed + stack: | + at Object. (/dev/null/test/unit/data.js:6:5) + at require (internal/modules/cjs/helpers.js:22:18) + at /dev/null/node_modules/mocha/lib/mocha.js:220:27 + at startup (internal/bootstrap/node.js:283:19) + at bootstrapNodeJSCore (internal/bootstrap/node.js:743:3) + ...`, + ` --- + message: second error + severity: failed + stack: | + at Object. (/dev/null/test/unit/data.js:6:5) + at require (internal/modules/cjs/helpers.js:22:18) + at /dev/null/node_modules/mocha/lib/mocha.js:220:27 + at startup (internal/bootstrap/node.js:283:19) + at bootstrapNodeJSCore (internal/bootstrap/node.js:743:3) + ...` + ], actualUndefinedTest: new TestEnd('fail', undefined, [], 'failed', 0, [{ passed: false, actual: undefined, diff --git a/test/unit/tap-reporter.js b/test/unit/tap-reporter.js index 33ed390..f3564ff 100644 --- a/test/unit/tap-reporter.js +++ b/test/unit/tap-reporter.js @@ -65,19 +65,10 @@ describe('Tap reporter', function () { it('should output all errors for a failing test', sinon.test(function () { const spy = this.stub(console, 'log'); - const expected = []; - - data.failingTest.errors.forEach(function (error) { - expected.push(' ---'); - expected.push(' message: "' + error.message.replace(/"/g, '\\"') + '"'); - expected.push(' severity: failed'); - expected.push(' stack: "' + error.stack.replace(/"/g, '\\"').replace(/\n/g, '\\n') + '"'); - expected.push(' ...'); - }); emitter.emit('testEnd', data.failingTest); - for (let i = 0; i < expected.length; i++) { - expect(spy).to.have.been.calledWith(expected[i]); + for (let i = 0; i < data.failingTapData.length; i++) { + expect(spy).to.have.been.calledWith(data.failingTapData[i]); } })); @@ -86,7 +77,7 @@ describe('Tap reporter', function () { emitter.emit('testEnd', data.actualUndefinedTest); - expect(spy).to.have.been.calledWith(' actual : undefined'); + expect(spy).to.have.been.calledWithMatch(/^ {2}actual {2}: undefined$/m); })); it('should output actual value for failed assertions even it was falsy', sinon.test(function () { @@ -94,7 +85,7 @@ describe('Tap reporter', function () { emitter.emit('testEnd', data.actualFalsyTest); - expect(spy).to.have.been.calledWith(' actual : "0"'); + expect(spy).to.have.been.calledWithMatch(/^ {2}actual {2}: 0$/m); })); it('should output expected value for failed assertions even it was undefined', sinon.test(function () { @@ -102,7 +93,7 @@ describe('Tap reporter', function () { emitter.emit('testEnd', data.expectedUndefinedTest); - expect(spy).to.have.been.calledWith(' expected: undefined'); + expect(spy).to.have.been.calledWithMatch(/^ {2}expected: undefined$/m); })); it('should output expected value for failed assertions even it was falsy', sinon.test(function () { @@ -110,7 +101,7 @@ describe('Tap reporter', function () { emitter.emit('testEnd', data.expectedFalsyTest); - expect(spy).to.have.been.calledWith(' expected: "0"'); + expect(spy).to.have.been.calledWithMatch(/^ {2}expected: 0$/m); })); it('should output the total number of tests', sinon.test(function () {