fix(testing): perform string -> boolean type coercion for Jest config

This adds some manual string -> boolean type coercion to the code that we use to convert CLI flags to a Jest config (after parsing them with yargs). This fixes an issue documented in #5640 where boolean CLI flags like `--watchAll` were being passed to Jest as strings, instead of as booleans, so that it was not possible to _disable_ a flag by doing `--flagName=false` (the value of the flag would be set to the string `"false"` which is a truthy value). There was also a somewhat related issue with not properly parsing boolean flags in our CLI parser which was recently fixed in #5646. fixes #5640 STENCIL-1259
ionic-team · Apr 15, 2024 · 9fc215a · 9fc215a
1 parent 2082368
commit 9fc215a
Show file tree

Hide file tree

Showing 11 changed files with 69 additions and 9 deletions.
diff --git a/jest.config.js b/jest.config.js
@@ -4,7 +4,7 @@ module.exports = {
     '@app-globals': '<rootDir>/internal/app-globals/index.cjs',
     '@platform': '<rootDir>/internal/testing/index.js',
     '@runtime': '<rootDir>/internal/testing/index.js',
-    '@stencil/core/cli': '<rootDir>/cli/index.js',
+    '@stencil/core/cli': '<rootDir>/cli/index.cjs',
     '@stencil/core/compiler': '<rootDir>/compiler/stencil.js',
     '@stencil/core/mock-doc': '<rootDir>/mock-doc/index.cjs',
     '@stencil/core/testing': '<rootDir>/testing/index.js',

diff --git a/scripts/esbuild/testing.ts b/scripts/esbuild/testing.ts
@@ -49,6 +49,10 @@ export async function buildTesting(opts: BuildOptions) {
     '../compiler/stencil.js',
   ];
 
+  const aliases = getEsbuildAliases();
+  // we want to point at the cjs module here because we're building cjs
+  aliases['@stencil/core/cli'] = './cli/index.cjs';
+
   const testingEsbuildOptions: ESBuildOptions = {
     ...getBaseEsbuildOptions(),
     entryPoints: [join(sourceDir, 'index.ts')],
@@ -63,7 +67,7 @@ export async function buildTesting(opts: BuildOptions) {
      * in `lazyRequirePlugin` and modify the imports
      */
     write: false,
-    alias: getEsbuildAliases(),
+    alias: aliases,
     banner: { js: getBanner(opts, `Stencil Testing`, true) },
     plugins: [
       externalAlias('@app-data', '@stencil/core/internal/app-data'),

diff --git a/scripts/esbuild/util.ts b/scripts/esbuild/util.ts
@@ -21,6 +21,7 @@ export function getEsbuildAliases(): Record<string, string> {
     '@stencil/core/dev-server': './dev-server/index.js',
     '@stencil/core/mock-doc': './mock-doc/index.cjs',
     '@stencil/core/internal/testing': './internal/testing/index.js',
+    '@stencil/core/cli': './cli/index.js',
     '@sys-api-node': './sys/node/index.js',
 
     // mapping node.js module names to `sys/node` "cache"

diff --git a/src/cli/index.ts b/src/cli/index.ts
@@ -1,3 +1,3 @@
-export { ConfigFlags } from './config-flags';
+export { BOOLEAN_CLI_FLAGS, ConfigFlags } from './config-flags';
 export { parseFlags } from './parse-flags';
 export { run, runTask } from './run';
diff --git a/src/testing/jest/jest-27-and-under/jest-config.ts b/src/testing/jest/jest-27-and-under/jest-config.ts
@@ -1,4 +1,5 @@
 import type { Config } from '@jest/types';
+import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
 import type * as d from '@stencil/core/internal';
 import { isString } from '@utils';
 
@@ -81,8 +82,10 @@ export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
     } catch (e) {}
   }
 
-  if (typeof jestArgv.ci === 'string') {
-    jestArgv.ci = jestArgv.ci === 'true' || jestArgv.ci === '';
+  for (const flag of BOOLEAN_CLI_FLAGS) {
+    if (typeof jestArgv[flag] === 'string') {
+      jestArgv[flag] = jestArgv[flag] === 'true';
+    }
   }
 
   return jestArgv;

diff --git a/src/testing/jest/jest-27-and-under/test/jest-config.spec.ts b/src/testing/jest/jest-27-and-under/test/jest-config.spec.ts
@@ -1,4 +1,5 @@
 import type { Config } from '@jest/types';
+import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
 import type * as d from '@stencil/core/declarations';
 import { mockValidatedConfig } from '@stencil/core/testing';
 import path from 'path';
@@ -20,6 +21,20 @@ describe('jest-config', () => {
     expect(jestArgv.maxWorkers).toBe(2);
   });
 
+  // the 'help' and 'version' flags are problematic with yargs (they cause a
+  // hang) and we intercept those flags and do our own thing before turning
+  // over to jest anyway
+  const cliFlagsToTest = BOOLEAN_CLI_FLAGS.filter((flag) => flag !== 'help' && flag !== 'version');
+  describe.each(cliFlagsToTest)('should deal with flag %s correctly', (cliArg) => {
+    it('converts boolean flag to boolean', () => {
+      const args = ['test', `--${cliArg}`];
+      const config = mockValidatedConfig();
+      config.flags = parseFlags(args);
+      const jestArgv = buildJestArgv(config);
+      expect(jestArgv[cliArg]).toBe(true);
+    });
+  });
+
   it('marks outputFile as a Jest argument', () => {
     const args = ['test', '--ci', '--outputFile=path/to/my-file'];
     const config = mockValidatedConfig();

diff --git a/src/testing/jest/jest-28/jest-config.ts b/src/testing/jest/jest-28/jest-config.ts
@@ -1,4 +1,5 @@
 import type { Config } from '@jest/types';
+import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
 import type * as d from '@stencil/core/internal';
 import { isString } from '@utils';
 
@@ -45,8 +46,10 @@ export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
     } catch (e) {}
   }
 
-  if (typeof jestArgv.ci === 'string') {
-    jestArgv.ci = jestArgv.ci === 'true' || jestArgv.ci === '';
+  for (const flag of BOOLEAN_CLI_FLAGS) {
+    if (typeof jestArgv[flag] === 'string') {
+      jestArgv[flag] = jestArgv[flag] === 'true';
+    }
   }
 
   return jestArgv;

diff --git a/src/testing/jest/jest-28/test/jest-config.spec.ts b/src/testing/jest/jest-28/test/jest-config.spec.ts
@@ -1,4 +1,5 @@
 import type { Config } from '@jest/types';
+import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
 import type * as d from '@stencil/core/declarations';
 import { mockValidatedConfig } from '@stencil/core/testing';
 import path from 'path';
@@ -20,6 +21,20 @@ describe('jest-config', () => {
     expect(jestArgv.maxWorkers).toBe(2);
   });
 
+  // the 'help' and 'version' flags are problematic with yargs (they cause a
+  // hang) and we intercept those flags and do our own thing before turning
+  // over to jest anyway
+  const cliFlagsToTest = BOOLEAN_CLI_FLAGS.filter((flag) => flag !== 'help' && flag !== 'version');
+  describe.each(cliFlagsToTest)('should deal with flag %s correctly', (cliArg) => {
+    it('converts boolean flag to boolean', () => {
+      const args = ['test', `--${cliArg}`];
+      const config = mockValidatedConfig();
+      config.flags = parseFlags(args);
+      const jestArgv = buildJestArgv(config);
+      expect(jestArgv[cliArg]).toBe(true);
+    });
+  });
+
   it('marks outputFile as a Jest argument', () => {
     const args = ['test', '--ci', '--outputFile=path/to/my-file'];
     const config = mockValidatedConfig();

diff --git a/src/testing/jest/jest-29/jest-config.ts b/src/testing/jest/jest-29/jest-config.ts
@@ -1,4 +1,5 @@
 import type { Config } from '@jest/types';
+import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
 import type * as d from '@stencil/core/internal';
 import { isString } from '@utils';
 
@@ -45,8 +46,10 @@ export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
     } catch (e) {}
   }
 
-  if (typeof jestArgv.ci === 'string') {
-    jestArgv.ci = jestArgv.ci === 'true' || jestArgv.ci === '';
+  for (const flag of BOOLEAN_CLI_FLAGS) {
+    if (typeof jestArgv[flag] === 'string') {
+      jestArgv[flag] = jestArgv[flag] === 'true';
+    }
   }
 
   return jestArgv;

diff --git a/src/testing/jest/jest-29/test/jest-config.spec.ts b/src/testing/jest/jest-29/test/jest-config.spec.ts
@@ -1,4 +1,5 @@
 import type { Config } from '@jest/types';
+import { BOOLEAN_CLI_FLAGS } from '@stencil/core/cli';
 import type * as d from '@stencil/core/declarations';
 import { mockValidatedConfig } from '@stencil/core/testing';
 import path from 'path';
@@ -20,6 +21,20 @@ describe('jest-config', () => {
     expect(jestArgv.maxWorkers).toBe(2);
   });
 
+  // the 'help' and 'version' flags are problematic with yargs (they cause a
+  // hang) and we intercept those flags and do our own thing before turning
+  // over to jest anyway
+  const cliFlagsToTest = BOOLEAN_CLI_FLAGS.filter((flag) => flag !== 'help' && flag !== 'version');
+  describe.each(cliFlagsToTest)('should deal with flag %s correctly', (cliArg) => {
+    it('converts boolean flag to boolean', () => {
+      const args = ['test', `--${cliArg}`];
+      const config = mockValidatedConfig();
+      config.flags = parseFlags(args);
+      const jestArgv = buildJestArgv(config);
+      expect(jestArgv[cliArg]).toBe(true);
+    });
+  });
+
   it('marks outputFile as a Jest argument', () => {
     const args = ['test', '--ci', '--outputFile=path/to/my-file'];
     const config = mockValidatedConfig();

diff --git a/tsconfig.json b/tsconfig.json
@@ -37,6 +37,7 @@
       "@stencil/core/internal/testing": ["src/testing/platform/index.ts"],
       "@stencil/core/mock-doc": ["src/mock-doc/index.ts"],
       "@stencil/core/testing": ["src/testing/index.ts"],
+      "@stencil/core/cli": ["src/cli/index.ts"],
       "@sys-api-node": ["src/sys/node/index.ts"],
       "@utils": ["src/utils/index.ts"],
       "@utils/shadow-css": ["src/utils/shadow-css"]