From a0060647d8a7e2df478cf301cdb5db5b1b0d6779 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinkle@fastmail.com>
Date: Wed, 9 Oct 2024 20:09:33 -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

Closes https://github.com/qunitjs/qunit/pull/1801.
---
 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..e4f3aa183 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'
     );
   });
 });