Skip to content

Commit

Permalink
feat(testing): support puppeteer's 'headless': 'new' (#4356)
Browse files Browse the repository at this point in the history
this commit creates a new CLI flag type to represent flags that can be
either a boolean or a string. e.g.:

- `--headless` <- creates a truthy flag, 'headless'
- `--headless=new` <- creates a flag, 'headless', with the value "new"
- `--headless new` <- creates a flag, 'headless', with the value "new"

the parsing for this type of flag is distinct from boolean arguments in
that if a value is provided, we should _not_ coerce it to a boolean:
--headless new  <- 'new' should be the accepted value
--headless=new  <- 'new' should be the accepted value

the parsing for this type of flag is distinct from string arguments in
that a non-string value is acceptable:
--headless <- `true` should be the accepted value

this allows the `browserHeadless` field to be 'new' both internally and in
a user's `stencil.config.ts`

this commit updates the testing validation code to correctly set
`browserHeadless` to 'new' in all circumstances where config.headless is
set to 'new'
  • Loading branch information
rwaskiewicz committed May 11, 2023
1 parent fa17303 commit 79dc015
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 32 deletions.
25 changes: 24 additions & 1 deletion src/cli/config-flags.ts
Expand Up @@ -16,7 +16,6 @@ export const BOOLEAN_CLI_FLAGS = [
'e2e',
'es5',
'esm',
'headless',
'help',
'log',
'open',
Expand Down Expand Up @@ -172,6 +171,21 @@ export const STRING_ARRAY_CLI_FLAGS = [
*/
export const STRING_NUMBER_CLI_FLAGS = ['maxWorkers'] as const;

/**
* All the CLI arguments which may have boolean or string values.
*/
export const BOOLEAN_STRING_CLI_FLAGS = [
/**
* `headless` is an argument passed through to Puppeteer (which is passed to Chrome) for end-to-end testing.
* Prior to Chrome v112, `headless` was treated like a boolean flag. Starting with Chrome v112, 'new' is an accepted
* option to support Chrome's new headless mode. In order to support this option in Stencil, both the boolean and
* string versions of the flag must be accepted.
*
* {@see https://developer.chrome.com/articles/new-headless/}
*/
'headless',
] as const;

/**
* All the LogLevel-type options supported by the Stencil CLI
*
Expand All @@ -193,6 +207,7 @@ export type StringCLIFlag = ArrayValuesAsUnion<typeof STRING_CLI_FLAGS>;
export type StringArrayCLIFlag = ArrayValuesAsUnion<typeof STRING_ARRAY_CLI_FLAGS>;
export type NumberCLIFlag = ArrayValuesAsUnion<typeof NUMBER_CLI_FLAGS>;
export type StringNumberCLIFlag = ArrayValuesAsUnion<typeof STRING_NUMBER_CLI_FLAGS>;
export type BooleanStringCLIFlag = ArrayValuesAsUnion<typeof BOOLEAN_STRING_CLI_FLAGS>;
export type LogCLIFlag = ArrayValuesAsUnion<typeof LOG_LEVEL_CLI_FLAGS>;

export type KnownCLIFlag =
Expand All @@ -201,6 +216,7 @@ export type KnownCLIFlag =
| StringArrayCLIFlag
| NumberCLIFlag
| StringNumberCLIFlag
| BooleanStringCLIFlag
| LogCLIFlag;

type AliasMap = Partial<Record<string, KnownCLIFlag>>;
Expand Down Expand Up @@ -275,6 +291,12 @@ type NumberConfigFlags = ObjectFromKeys<typeof NUMBER_CLI_FLAGS, number>;
*/
type StringNumberConfigFlags = ObjectFromKeys<typeof STRING_NUMBER_CLI_FLAGS, string | number>;

/**
* Type containing the configuration flags which may be set to either string
* or boolean values.
*/
type BooleanStringConfigFlags = ObjectFromKeys<typeof BOOLEAN_STRING_CLI_FLAGS, boolean | string>;

/**
* Type containing the possible LogLevel configuration flags, to be included
* in ConfigFlags, below
Expand All @@ -301,6 +323,7 @@ export interface ConfigFlags
StringArrayConfigFlags,
NumberConfigFlags,
StringNumberConfigFlags,
BooleanStringConfigFlags,
LogLevelFlags {
task: TaskCommand | null;
args: string[];
Expand Down
30 changes: 25 additions & 5 deletions src/cli/parse-flags.ts
Expand Up @@ -3,6 +3,7 @@ import { readOnlyArrayHasStringMember, toCamelCase } from '@utils';
import { LOG_LEVELS, LogLevel, TaskCommand } from '../declarations';
import {
BOOLEAN_CLI_FLAGS,
BOOLEAN_STRING_CLI_FLAGS,
CLI_FLAG_ALIASES,
CLI_FLAG_REGEX,
ConfigFlags,
Expand Down Expand Up @@ -112,6 +113,14 @@ const parseCLITerm = (flags: ConfigFlags, args: string[]) => {
// array is empty, we're done!
if (arg === undefined) return;

// capture whether this is a special case of a negated boolean or boolean-string before we start to test each case
const isNegatedBoolean =
!readOnlyArrayHasStringMember(BOOLEAN_CLI_FLAGS, normalizeFlagName(arg)) &&
readOnlyArrayHasStringMember(BOOLEAN_CLI_FLAGS, normalizeNegativeFlagName(arg));
const isNegatedBooleanOrString =
!readOnlyArrayHasStringMember(BOOLEAN_STRING_CLI_FLAGS, normalizeFlagName(arg)) &&
readOnlyArrayHasStringMember(BOOLEAN_STRING_CLI_FLAGS, normalizeNegativeFlagName(arg));

// EqualsArg → "--" ArgName "=" CLIValue ;
if (arg.startsWith('--') && arg.includes('=')) {
// we're dealing with an EqualsArg, we have a special helper for that
Expand Down Expand Up @@ -141,11 +150,7 @@ const parseCLITerm = (flags: ConfigFlags, args: string[]) => {
}

// NegativeArg → "--no" ArgName ;
else if (
arg.startsWith('--no') &&
!readOnlyArrayHasStringMember(BOOLEAN_CLI_FLAGS, normalizeFlagName(arg)) &&
readOnlyArrayHasStringMember(BOOLEAN_CLI_FLAGS, normalizeNegativeFlagName(arg))
) {
else if (arg.startsWith('--no') && (isNegatedBoolean || isNegatedBooleanOrString)) {
// possibly dealing with a `NegativeArg` here. There is a little ambiguity
// here because we have arguments that already begin with `no` like
// `notify`, so we need to test if a normalized form of the raw argument is
Expand Down Expand Up @@ -300,6 +305,21 @@ const setCLIArg = (flags: ConfigFlags, rawArg: string, normalizedArg: string, va
}
}

// We're setting a value which could be either a boolean _or_ a string
else if (readOnlyArrayHasStringMember(BOOLEAN_STRING_CLI_FLAGS, normalizedArg)) {
const derivedValue =
typeof value === 'string'
? value
? value // use the supplied value if it's a non-empty string
: false // otherwise, default to false for the empty string
: true; // no value was supplied, default to true
flags[normalizedArg] = derivedValue;
flags.knownArgs.push(rawArg);
if (typeof derivedValue === 'string' && derivedValue) {
flags.knownArgs.push(derivedValue);
}
}

// We're setting the log level, which can only be a set of specific string values
else if (readOnlyArrayHasStringMember(LOG_LEVEL_CLI_FLAGS, normalizedArg)) {
if (typeof value === 'string') {
Expand Down
41 changes: 41 additions & 0 deletions src/cli/test/parse-flags.spec.ts
Expand Up @@ -3,6 +3,8 @@ import { toDashCase } from '@utils';
import { LogLevel } from '../../declarations';
import {
BOOLEAN_CLI_FLAGS,
BOOLEAN_STRING_CLI_FLAGS,
BooleanStringCLIFlag,
ConfigFlags,
NUMBER_CLI_FLAGS,
STRING_ARRAY_CLI_FLAGS,
Expand Down Expand Up @@ -132,6 +134,45 @@ describe('parseFlags', () => {
expect(flags.config).toBe('/config-2.js');
});

describe.each(BOOLEAN_STRING_CLI_FLAGS)('boolean-string flag - %s', (cliArg: BooleanStringCLIFlag) => {
it('parses a boolean-string flag as a boolean with no arg', () => {
const args = [`--${cliArg}`];
const flags = parseFlags(args);
expect(flags.headless).toBe(true);
expect(flags.knownArgs).toEqual([`--${cliArg}`]);
});

it(`parses a boolean-string flag as a falsy boolean with "no" arg - --no-${cliArg}`, () => {
const args = [`--no-${cliArg}`];
const flags = parseFlags(args);
expect(flags.headless).toBe(false);
expect(flags.knownArgs).toEqual([`--no-${cliArg}`]);
});

it(`parses a boolean-string flag as a falsy boolean with "no" arg - --no${
cliArg.charAt(0).toUpperCase() + cliArg.slice(1)
}`, () => {
const negativeFlag = '--no' + cliArg.charAt(0).toUpperCase() + cliArg.slice(1);
const flags = parseFlags([negativeFlag]);
expect(flags.headless).toBe(false);
expect(flags.knownArgs).toEqual([negativeFlag]);
});

it('parses a boolean-string flag as a string with a string arg', () => {
const args = [`--${cliArg}`, 'new'];
const flags = parseFlags(args);
expect(flags.headless).toBe('new');
expect(flags.knownArgs).toEqual(['--headless', 'new']);
});

it('parses a boolean-string flag as a string with a string arg using equality', () => {
const args = [`--${cliArg}=new`];
const flags = parseFlags(args);
expect(flags.headless).toBe('new');
expect(flags.knownArgs).toEqual([`--${cliArg}`, 'new']);
});
});

describe.each<LogLevel>(['info', 'warn', 'error', 'debug'])('logLevel %s', (level) => {
it("should parse '--logLevel %s'", () => {
const args = ['--logLevel', level];
Expand Down
65 changes: 46 additions & 19 deletions src/compiler/config/test/validate-testing.spec.ts
Expand Up @@ -31,28 +31,55 @@ describe('validateTesting', () => {
];
});

it('set headless false w/ flag', () => {
userConfig.flags = { ...flags, e2e: true, headless: false };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(false);
});
describe('browserHeadless', () => {
describe("using 'headless' value from cli", () => {
it.each([false, true, 'new'])('sets browserHeadless to %s', (headless) => {
userConfig.flags = { ...flags, e2e: true, headless };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(headless);
});

it('set headless true w/ flag', () => {
userConfig.flags = { ...flags, e2e: true, headless: true };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(true);
});
it('defaults to true outside of CI', () => {
userConfig.flags = { ...flags, e2e: true };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(true);
});
});

it('default headless true', () => {
userConfig.flags = { ...flags, e2e: true };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(true);
});
describe('with ci enabled', () => {
it("forces using the old headless mode when 'headless: false'", () => {
userConfig.flags = { ...flags, ci: true, e2e: true, headless: false };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(true);
});

it('force headless with ci flag', () => {
userConfig.flags = { ...flags, ci: true, e2e: true, headless: false };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(true);
it('allows the new headless mode to be used', () => {
userConfig.flags = { ...flags, ci: true, e2e: true, headless: 'new' };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe('new');
});
});

describe('`testing` configuration', () => {
beforeEach(() => {
userConfig.flags = { ...flags, e2e: true, headless: undefined };
});

it.each<boolean | 'new'>([false, true, 'new'])(
'uses %s browserHeadless mode from testing config',
(browserHeadlessValue) => {
userConfig.testing = { browserHeadless: browserHeadlessValue };
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(browserHeadlessValue);
}
);

it('defaults the headless mode to true when browserHeadless is not provided', () => {
userConfig.testing = {};
const { config } = validateConfig(userConfig, mockLoadConfigInit());
expect(config.testing.browserHeadless).toBe(true);
});
});
});

it('default to no-sandbox browser args with ci flag', () => {
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/config/validate-testing.ts
Expand Up @@ -20,9 +20,9 @@ export const validateTesting = (config: d.ValidatedConfig, diagnostics: d.Diagno
configPathDir = config.rootDir!;
}

if (typeof config.flags.headless === 'boolean') {
if (typeof config.flags.headless === 'boolean' || config.flags.headless === 'new') {
testing.browserHeadless = config.flags.headless;
} else if (typeof testing.browserHeadless !== 'boolean') {
} else if (typeof testing.browserHeadless !== 'boolean' && testing.browserHeadless !== 'new') {
testing.browserHeadless = true;
}

Expand All @@ -38,7 +38,7 @@ export const validateTesting = (config: d.ValidatedConfig, diagnostics: d.Diagno
addTestingConfigOption(testing.browserArgs, '--no-sandbox');
addTestingConfigOption(testing.browserArgs, '--disable-setuid-sandbox');
addTestingConfigOption(testing.browserArgs, '--disable-dev-shm-usage');
testing.browserHeadless = true;
testing.browserHeadless = testing.browserHeadless === 'new' ? 'new' : true;
}

if (typeof testing.rootDir === 'string') {
Expand Down
13 changes: 11 additions & 2 deletions src/declarations/stencil-public-compiler.ts
Expand Up @@ -1775,9 +1775,18 @@ export interface TestingConfig extends JestConfig {
browserWSEndpoint?: string;

/**
* Whether to run browser e2e tests in headless mode. Defaults to true.
* Whether to run browser e2e tests in headless mode.
*
* Starting with Chrome v112, a new headless mode was introduced.
* The new headless mode unifies the "headful" and "headless" code paths in the Chrome distributable.
*
* To enable the "new" headless mode, a string value of "new" must be provided.
* To use the "old" headless mode, a boolean value of `true` must be provided.
* To use "headful" mode, a boolean value of `false` must be provided.
*
* Defaults to true.
*/
browserHeadless?: boolean;
browserHeadless?: boolean | 'new';

/**
* Slows down e2e browser operations by the specified amount of milliseconds.
Expand Down
2 changes: 1 addition & 1 deletion test/bundler/karma.config.ts
Expand Up @@ -11,7 +11,7 @@ const localLaunchers = {
base: CHROME_HEADLESS,
flags: [
// run in headless mode (https://chromium.googlesource.com/chromium/src/+/lkgr/headless/README.md)
'--headless',
'--headless=new',
// use --disable-gpu to avoid an error from a missing Mesa library (https://chromium.googlesource.com/chromium/src/+/lkgr/headless/README.md)
'--disable-gpu',
// without a remote debugging port, Chrome exits immediately.
Expand Down
2 changes: 1 addition & 1 deletion test/karma/karma.config.js
Expand Up @@ -48,7 +48,7 @@ const localLaunchers = {
flags: [
'--no-sandbox',
// See https://chromium.googlesource.com/chromium/src/+/lkgr/headless/README.md
'--headless',
'--headless=new',
'--disable-gpu',
// Without a remote debugging port, Google Chrome exits immediately.
'--remote-debugging-port=9333',
Expand Down

0 comments on commit 79dc015

Please sign in to comment.