Remove broken mock-heavy tests

tj · Nov 10, 2024 · 7310ffb · 7310ffb
1 parent fc133f4
commit 7310ffb
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 58 deletions.
diff --git a/lib/command.js b/lib/command.js
@@ -2566,9 +2566,9 @@ function useColor() {
   // References:
   // https://no-color.org
   // https://bixense.com/clicolors/
-  // https://force-color.org (not canonical, recent web page from 2023)
   // https://github.com/nodejs/node/blob/0a00217a5f67ef4a22384cfc80eb6dd9a917fdc1/lib/internal/tty.js#L109
   // https://github.com/chalk/supports-color/blob/c214314a14bcb174b12b3014b2b0a8de375029ae/index.js#L33
+  // (https://force-color.org recent web page from 2023, does not match major javascript implementations)
 
   if (
     process.env.NO_COLOR ||

diff --git a/tests/command.configureOutput.test.js b/tests/command.configureOutput.test.js
@@ -1,4 +1,5 @@
 const commander = require('../');
+const process = require('node:process');
 
 test('when default writeErr() then error on stderr', () => {
   const writeSpy = jest
@@ -328,57 +329,26 @@ test('when custom outputErr and writeErr and error then outputErr passed writeEr
   );
 });
 
-describe.each([
-  ['getOutHasColors', process.stdout],
-  ['getErrHasColors', process.stderr],
-])('%s', (configProperty, stream) => {
-  // Just varying isTTY and not testing stream.hasColors() as such.
-  const holdIsTTY = stream.isTTY;
-  let hasColorsSpy;
-  const getHasColors = new commander.Command().configureOutput()[
-    configProperty
-  ];
-
-  beforeAll(() => {
-    // When running tests, hasColors is usually undefined.
-    if (stream.hasColors) {
-      hasColorsSpy = jest.spyOn(stream, 'hasColors').mockImplementation(() => {
-        return true;
-      });
-    } else {
-      stream.hasColors = () => true;
-    }
-  });
-
-  beforeEach(() => {
-    stream.isTTY = true;
-  });
-
-  afterAll(() => {
-    if (hasColorsSpy) hasColorsSpy.mockRestore();
-    stream.isTTY = holdIsTTY;
-  });
-
-  test('when isTTY=true then returns true', () => {
-    stream.isTTY = true;
-    expect(getHasColors()).toBe(true);
-  });
-
-  test('when isTTY=false then returns false', () => {
-    stream.isTTY = false;
-    expect(getHasColors()).toBe(true);
-  });
-
-  test.each([
-    [true, 'NO_COLOR', false],
-    [false, 'FORCE_COLOR', true],
-    [false, 'CLICOLOR_FORCE', true],
-  ])('when isTTY=%o but %s then returns %o', (isTTY, envvar, result) => {
-    const holdEnv = process.env[envvar];
-    process.env[envvar] = '1';
-    stream.isTTY = isTTY;
-    expect(getHasColors()).toBe(result);
-    if (holdEnv === undefined) delete process.env[envvar];
-    else process.env[envvar] = holdEnv;
-  });
-});
+describe.each([['getOutHasColors'], ['getErrHasColors']])(
+  '%s',
+  (configProperty) => {
+    // Tried and failed to mock/modify process.stdout.isTTY to test that part of implementation.
+    // Just test overrides work as expected!
+
+    const getHasColors = new commander.Command().configureOutput()[
+      configProperty
+    ];
+
+    test.each([
+      [true, 'NO_COLOR', false],
+      [false, 'FORCE_COLOR', true],
+      [false, 'CLICOLOR_FORCE', true],
+    ])('when isTTY=%o but %s then returns %o', (isTTY, envvar, result) => {
+      const holdEnv = process.env[envvar];
+      process.env[envvar] = '1';
+      expect(getHasColors()).toBe(result);
+      if (holdEnv === undefined) delete process.env[envvar];
+      else process.env[envvar] = holdEnv;
+    });
+  },
+);
diff --git a/tests/useColor.test.js b/tests/useColor.test.js
@@ -61,14 +61,13 @@ describe('internal useColor environment variable support', () => {
     expect(useColor()).toBe(false);
   });
 
-  // https://force-color.org (not canonical, recent web page from 2023)
   // chalk: https://github.com/chalk/supports-color/blob/c214314a14bcb174b12b3014b2b0a8de375029ae/index.js#L33
   // node: https://github.com/nodejs/node/blob/0a00217a5f67ef4a22384cfc80eb6dd9a917fdc1/lib/internal/tty.js#L109
+  // (https://force-color.org recent web page from 2023, does not match major javascript implementations)
   //
-  // Implementations vary in treatment of value.
   // Chalk ignores anything except for 0,1,2,3,4,true,false values.
   // Node somewhat follows Chalk with 0,1,2,3,true, but treats empty as true and unexpected values as false.
-  // Test for the expected Chalk values (which do produce same result in node).
+  // Test the expected Chalk values (which do produce same result in node).
 
   test.each([
     ['true', true],