Skip to content

Commit d32f2e1

Browse files
authored
feat: add webserver guardrail for playwright checks [tra-1789] (#1196)
* feat: add webserver guardrail [tra-1789] * fix: add warning * fix: catch [tra-1789] * fix: merge [tra-1789] * fix: test [tra-1789]
1 parent 609c4cd commit d32f2e1

File tree

7 files changed

+144
-3
lines changed

7 files changed

+144
-3
lines changed

packages/cli/src/commands/pw-test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { Flags } from '@oclif/core'
1818
import { createReporters, ReporterType } from '../reporters/reporter'
1919
import TestRunner from '../services/test-runner'
2020
import {
21-
DEFAULT_CHECK_RUN_TIMEOUT_SECONDS,
2221
DEFAULT_PLAYWRIGHT_CHECK_RUN_TIMEOUT_SECONDS,
2322
Events,
2423
SequenceId,
@@ -95,6 +94,12 @@ export default class PwTestCommand extends AuthCommand {
9594
default: true,
9695
allowNo: true,
9796
}),
97+
'include': Flags.string({
98+
char: 'i',
99+
description: 'File patterns to include when bundling the test project (e.g., "utils/**/*").',
100+
multiple: true,
101+
default: [],
102+
}),
98103
}
99104

100105
async run (): Promise<void> {
@@ -116,6 +121,7 @@ export default class PwTestCommand extends AuthCommand {
116121
'test-session-name': testSessionName,
117122
'create-check': createCheck,
118123
'stream-logs': streamLogs,
124+
'include': includeFlag,
119125
} = flags
120126
const { configDirectory, configFilenames } = splitConfigFilePath(configFilename)
121127
const pwPathFlag = this.getConfigPath(playwrightFlags)
@@ -174,8 +180,10 @@ export default class PwTestCommand extends AuthCommand {
174180
verifyRuntimeDependencies: false,
175181
checklyConfigConstructs,
176182
playwrightConfigPath,
177-
include: checklyConfig.checks?.include,
183+
include: includeFlag.length ? includeFlag : checklyConfig.checks?.include,
184+
includeFlagProvided: includeFlag.length > 0,
178185
playwrightChecks: [playwrightCheck],
186+
currentCommand: 'pw-test',
179187
checkFilter: check => {
180188
// Skip non Playwright checks
181189
if (!(check instanceof PlaywrightCheck)) {
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "playwright-check-webserver-fixture",
3+
"devDependencies": {
4+
"@playwright/test": "^1.55.1"
5+
}
6+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { defineConfig } from '@playwright/test'
2+
3+
export default defineConfig({
4+
testDir: './tests',
5+
webServer: {
6+
command: 'npm run start',
7+
url: 'http://localhost:3000',
8+
},
9+
})

packages/cli/src/constructs/__tests__/playwright-check.spec.ts

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import path from 'node:path'
22

3-
import { describe, it, expect, beforeEach, vi } from 'vitest'
3+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
44
import { AxiosHeaders } from 'axios'
55

66
import { CheckGroupV2, Diagnostics, PlaywrightCheck, RetryStrategyBuilder } from '../index'
@@ -19,6 +19,13 @@ describe('PlaywrightCheck', () => {
1919
headers: new AxiosHeaders(),
2020
},
2121
})
22+
Session.currentCommand = undefined
23+
Session.includeFlagProvided = undefined
24+
})
25+
26+
afterEach(() => {
27+
Session.currentCommand = undefined
28+
Session.includeFlagProvided = undefined
2229
})
2330

2431
it('should synthesize groupName', async () => {
@@ -366,6 +373,78 @@ describe('PlaywrightCheck', () => {
366373
}),
367374
]))
368375
})
376+
377+
it('should warn if webServer is configured in playwright config when running pw-test', async () => {
378+
Session.basePath = path.resolve(__dirname, './fixtures/playwright-check-webserver')
379+
Session.project = new Project('project-id', {
380+
name: 'Test Project',
381+
repoUrl: 'https://github.com/checkly/checkly-cli',
382+
})
383+
Session.currentCommand = 'pw-test'
384+
385+
const check = new PlaywrightCheck('foo', {
386+
name: 'Test Check',
387+
playwrightConfigPath: path.resolve(__dirname, './fixtures/playwright-check-webserver/playwright.config.ts'),
388+
})
389+
390+
const diags = new Diagnostics()
391+
await check.validate(diags)
392+
393+
expect(diags.isFatal()).toEqual(false)
394+
expect(diags.observations).toEqual(expect.arrayContaining([
395+
expect.objectContaining({
396+
title: expect.stringContaining('webServer configuration detected'),
397+
message: expect.stringContaining('webServer configuration requires additional files'),
398+
}),
399+
]))
400+
})
401+
402+
it('should not warn about webServer when not running pw-test command', async () => {
403+
Session.basePath = path.resolve(__dirname, './fixtures/playwright-check-webserver')
404+
Session.project = new Project('project-id', {
405+
name: 'Test Project',
406+
repoUrl: 'https://github.com/checkly/checkly-cli',
407+
})
408+
Session.currentCommand = 'test'
409+
410+
const check = new PlaywrightCheck('foo', {
411+
name: 'Test Check',
412+
playwrightConfigPath: path.resolve(__dirname, './fixtures/playwright-check-webserver/playwright.config.ts'),
413+
})
414+
415+
const diags = new Diagnostics()
416+
await check.validate(diags)
417+
418+
expect(diags.observations).not.toEqual(expect.arrayContaining([
419+
expect.objectContaining({
420+
title: expect.stringContaining('webServer configuration detected'),
421+
}),
422+
]))
423+
})
424+
425+
it('should not warn about webServer when --include flag is provided', async () => {
426+
Session.basePath = path.resolve(__dirname, './fixtures/playwright-check-webserver')
427+
Session.project = new Project('project-id', {
428+
name: 'Test Project',
429+
repoUrl: 'https://github.com/checkly/checkly-cli',
430+
})
431+
Session.currentCommand = 'pw-test'
432+
Session.includeFlagProvided = true
433+
434+
const check = new PlaywrightCheck('foo', {
435+
name: 'Test Check',
436+
playwrightConfigPath: path.resolve(__dirname, './fixtures/playwright-check-webserver/playwright.config.ts'),
437+
})
438+
439+
const diags = new Diagnostics()
440+
await check.validate(diags)
441+
442+
expect(diags.observations).not.toEqual(expect.arrayContaining([
443+
expect.objectContaining({
444+
title: expect.stringContaining('webServer configuration detected'),
445+
}),
446+
]))
447+
})
369448
})
370449

371450
describe('defaults', () => {

packages/cli/src/constructs/playwright-check.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
InvalidPropertyValueDiagnostic,
1515
UnsupportedPropertyDiagnostic,
1616
} from './construct-diagnostics'
17+
import { WarningDiagnostic } from './diagnostics'
1718
import { Diagnostics } from './diagnostics'
1819
import { PlaywrightCheckBundle } from './playwright-check-bundle'
1920
import { Session } from './project'
@@ -283,12 +284,42 @@ export class PlaywrightCheck extends RuntimeCheck {
283284
}
284285
}
285286

287+
protected async validateWebServerConfig (diagnostics: Diagnostics): Promise<void> {
288+
// Only show webServer warning for pw-test command when --include is not provided
289+
if (Session.currentCommand !== 'pw-test' || Session.includeFlagProvided) {
290+
return
291+
}
292+
293+
try {
294+
const playwrightConfig = await Session.loadFile(this.playwrightConfigPath)
295+
296+
if (playwrightConfig && typeof playwrightConfig === 'object' && 'webServer' in playwrightConfig) {
297+
diagnostics.add(new WarningDiagnostic({
298+
title: 'webServer configuration detected',
299+
message:
300+
`webServer configuration requires additional files. `
301+
+ `Include all files needed to start the server (e.g., server scripts, config files) `
302+
+ `by passing them via the --include flag.`,
303+
}))
304+
}
305+
} catch (err: any) {
306+
diagnostics.add(new InvalidPropertyValueDiagnostic(
307+
'playwrightConfigPath',
308+
new Error(
309+
`Unable to parse Playwright config "${this.playwrightConfigPath}": ${err.message}`,
310+
{ cause: err },
311+
),
312+
))
313+
}
314+
}
315+
286316
async validate (diagnostics: Diagnostics): Promise<void> {
287317
await super.validate(diagnostics)
288318
await this.validateRetryStrategy(diagnostics)
289319

290320
try {
291321
await fs.access(this.playwrightConfigPath, fs.constants.R_OK)
322+
await this.validateWebServerConfig(diagnostics)
292323
} catch (err: any) {
293324
diagnostics.add(new InvalidPropertyValueDiagnostic(
294325
'playwrightConfigPath',

packages/cli/src/constructs/project.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@ export class Session {
244244
static parsers = new Map<string, Parser>()
245245
static constructExports: ConstructExport[] = []
246246
static ignoreDirectoriesMatch: string[] = []
247+
static currentCommand?: 'pw-test' | 'test' | 'deploy'
248+
static includeFlagProvided?: boolean
247249

248250
static async loadFile<T = unknown> (filePath: string): Promise<T> {
249251
const loader = this.loader

packages/cli/src/services/project-parser.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ type ProjectParseOpts = {
3333
checklyConfigConstructs?: Construct[]
3434
playwrightConfigPath?: string
3535
include?: string | string[]
36+
includeFlagProvided?: boolean
3637
playwrightChecks?: PlaywrightSlimmedProp[]
38+
currentCommand?: 'pw-test' | 'test' | 'deploy'
3739
}
3840

3941
const BASE_CHECK_DEFAULTS = {
@@ -59,7 +61,9 @@ export async function parseProject (opts: ProjectParseOpts): Promise<Project> {
5961
checklyConfigConstructs,
6062
playwrightConfigPath,
6163
include,
64+
includeFlagProvided,
6265
playwrightChecks,
66+
currentCommand,
6367
} = opts
6468
const project = new Project(projectLogicalId, {
6569
name: projectName,
@@ -82,6 +86,8 @@ export async function parseProject (opts: ProjectParseOpts): Promise<Project> {
8286
Session.defaultRuntimeId = defaultRuntimeId
8387
Session.verifyRuntimeDependencies = verifyRuntimeDependencies ?? true
8488
Session.ignoreDirectoriesMatch = ignoreDirectoriesMatch
89+
Session.currentCommand = currentCommand
90+
Session.includeFlagProvided = includeFlagProvided
8591

8692
// TODO: Do we really need all of the ** globs, or could we just put node_modules?
8793
const ignoreDirectories = ['**/node_modules/**', '**/.git/**', ...ignoreDirectoriesMatch]

0 commit comments

Comments
 (0)