Skip to content

Commit

Permalink
fix(testing): perform string -> boolean type coercion for Jest config
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alicewriteswrongs committed Apr 15, 2024
1 parent 2082368 commit 89b6b45
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 9 deletions.
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
6 changes: 5 additions & 1 deletion scripts/esbuild/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')],
Expand All @@ -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'),
Expand Down
1 change: 1 addition & 0 deletions scripts/esbuild/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/cli/index.ts
Original file line number Diff line number Diff line change
@@ -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';
7 changes: 5 additions & 2 deletions src/testing/jest/jest-27-and-under/jest-config.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/testing/jest/jest-27-and-under/test/jest-config.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions src/testing/jest/jest-28/jest-config.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/testing/jest/jest-28/test/jest-config.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions src/testing/jest/jest-29/jest-config.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/testing/jest/jest-29/test/jest-config.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down

0 comments on commit 89b6b45

Please sign in to comment.