From 823f7b1ab03d5ed31b859738753324fc62f5d5c7 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinkle@fastmail.com>
Date: Sun, 29 Sep 2024 13:14:38 -0400
Subject: [PATCH] CLI: Limit colors in TAP reporter to test names

Don't apply colors to the "ok 1" and "not ok 1" prefix. This has
the benefit of making the test names easier to visually extract
for developers.

It also increases compatibility with the popular `tap-parser`
package which doesn't parse TAP lines correctly otherwise, since
blocks like "not ok" must start on a new line, and with the color
code in front, they are technically either a continuation of the
previous block, or an ignored "Anything" line between two blocks.

https://github.com/tapjs/tapjs/tree/ccfea67fec/src/parser

https://testanything.org/tap-version-13-specification.html
---
 src/core/reporters/TapReporter.js | 14 +++++++-------
 test/main/TapReporter.js          | 14 +++++++-------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/core/reporters/TapReporter.js b/src/core/reporters/TapReporter.js
index 8ae7caf18..9447810ad 100644
--- a/src/core/reporters/TapReporter.js
+++ b/src/core/reporters/TapReporter.js
@@ -210,7 +210,7 @@ export default class TapReporter {
     if (!this.ended) {
       this.onRunStart();
       this.testCount = this.testCount + 1;
-      this.log(kleur.red(`not ok ${this.testCount} global failure`));
+      this.log(`not ok ${this.testCount} ${kleur.red('global failure')}`);
       this.logError(error);
     }
 
@@ -227,16 +227,16 @@ export default class TapReporter {
       this.log(`ok ${this.testCount} ${test.fullName.join(' > ')}`);
     } else if (test.status === 'skipped') {
       this.log(
-        kleur.yellow(`ok ${this.testCount} # SKIP ${test.fullName.join(' > ')}`)
+        `ok ${this.testCount} ${kleur.yellow(`# SKIP ${test.fullName.join(' > ')}`)}`
       );
     } else if (test.status === 'todo') {
       this.log(
-        kleur.cyan(`not ok ${this.testCount} # TODO ${test.fullName.join(' > ')}`)
+        `not ok ${this.testCount} ${kleur.cyan(`# TODO ${test.fullName.join(' > ')}`)}`
       );
       test.errors.forEach((error) => this.logAssertion(error, 'todo'));
     } else {
       this.log(
-        kleur.red(`not ok ${this.testCount} ${test.fullName.join(' > ')}`)
+        `not ok ${this.testCount} ${kleur.red(test.fullName.join(' > '))}`
       );
       test.errors.forEach((error) => this.logAssertion(error));
     }
@@ -247,9 +247,9 @@ export default class TapReporter {
 
     this.log(`1..${runEnd.testCounts.total}`);
     this.log(`# pass ${runEnd.testCounts.passed}`);
-    this.log(kleur.yellow(`# skip ${runEnd.testCounts.skipped}`));
-    this.log(kleur.cyan(`# todo ${runEnd.testCounts.todo}`));
-    this.log(kleur.red(`# fail ${runEnd.testCounts.failed}`));
+    this.log(`# ${kleur.yellow('skip ' + runEnd.testCounts.skipped)}`);
+    this.log(`# ${kleur.cyan('todo ' + runEnd.testCounts.todo)}`);
+    this.log(`# ${kleur.red('fail ' + runEnd.testCounts.failed)}`);
   }
 
   logAssertion (error, severity) {
diff --git a/test/main/TapReporter.js b/test/main/TapReporter.js
index 44ba65462..a598a7f4e 100644
--- a/test/main/TapReporter.js
+++ b/test/main/TapReporter.js
@@ -85,7 +85,7 @@ QUnit.module('TapReporter', function (hooks) {
   });
 
   QUnit.test('output ok for a skipped test', function (assert) {
-    var expected = kleur.yellow('ok 1 # SKIP name');
+    var expected = 'ok 1 ' + kleur.yellow('# SKIP name');
 
     emitter.emit('testEnd', {
       name: 'name',
@@ -100,7 +100,7 @@ QUnit.module('TapReporter', function (hooks) {
   });
 
   QUnit.test('output not ok for a todo test', function (assert) {
-    var expected = kleur.cyan('not ok 1 # TODO name');
+    var expected = 'not ok 1 ' + kleur.cyan('# TODO name');
 
     emitter.emit('testEnd', {
       name: 'name',
@@ -115,7 +115,7 @@ QUnit.module('TapReporter', function (hooks) {
   });
 
   QUnit.test('output not ok for a failing test', function (assert) {
-    var expected = kleur.red('not ok 1 name');
+    var expected = 'not ok 1 ' + kleur.red('name');
 
     emitter.emit('testEnd', {
       name: 'name',
@@ -143,7 +143,7 @@ QUnit.module('TapReporter', function (hooks) {
       assertions: []
     });
 
-    assert.strictEqual(buffer, kleur.red('not ok 1 name') + '\n'
+    assert.strictEqual(buffer, 'not ok 1 ' + kleur.red('name') + '\n'
 + '  ---\n'
 + '  message: first error\n'
 + '  severity: failed\n'
@@ -168,7 +168,7 @@ QUnit.module('TapReporter', function (hooks) {
     emitter.clear();
     emitter.emit('error', 'Boo');
 
-    assert.strictEqual(buffer, kleur.red('not ok 1 global failure') + '\n'
+    assert.strictEqual(buffer, 'not ok 1 ' + kleur.red('global failure') + '\n'
 + '  ---\n'
 + '  message: Boo\n'
 + '  severity: failed\n'
@@ -184,7 +184,7 @@ QUnit.module('TapReporter', function (hooks) {
     emitter.clear();
     emitter.emit('error', err);
 
-    assert.strictEqual(buffer, kleur.red('not ok 1 global failure') + '\n'
+    assert.strictEqual(buffer, 'not ok 1 ' + kleur.red('global failure') + '\n'
 + '  ---\n'
 + '  message: ReferenceError: Boo is not defined\n'
 + '  severity: failed\n'
@@ -437,7 +437,7 @@ QUnit.module('TapReporter', function (hooks) {
     });
 
     assert.strictEqual(buffer, '1..6\n'
-+ '# pass 3\n' + kleur.yellow('# skip 1') + '\n' + kleur.cyan('# todo 0') + '\n' + kleur.red('# fail 2') + '\n'
++ '# pass 3\n# ' + kleur.yellow('skip 1') + '\n# ' + kleur.cyan('todo 0') + '\n# ' + kleur.red('fail 2') + '\n'
     );
   });
 });