Reporter: Support multi-line strings in TAP failures data

This is a different approach to fixing #109.

The issue was previously fixed in js-reporters 1.2.2 by
#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 #109.

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 <https://yaml-multiline.info/>
+      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);
   }
 };

test/unit/data.js

@@ -1,10 +1,41 @@
 const { TestEnd, TestStart, SuiteStart, SuiteEnd } = require('../../');
 
+function mockStack (error) {
+  error.stack = `    at Object.<anonymous> (/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.<anonymous> (/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.<anonymous> (/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,

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,31 +77,31 @@ 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 () {
     const spy = this.stub(console, 'log');
 
     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 () {
     const spy = this.stub(console, 'log');
 
     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 () {
     const spy = this.stub(console, 'log');
 
     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 () {