Skip to content

Commit 969c2b3

Browse files
sorccusorccu
authored
fix: default playwrightConfig was not being set for Browser/MultiStep checks (#1104)
The issue was caused by the introduction of typed defaults in version 6.1.0. However, it turns out that the type had never included `playwrightConfig`, but it had still been working because all properties were overwritten blindly regardless of whether they were present in the type or not. This change makes BrowserChecks and MultiStepChecks use the appropriate default playwrightConfig again. Tests were added to make sure it won't break again.
1 parent 37acb02 commit 969c2b3

File tree

6 files changed

+208
-10
lines changed

6 files changed

+208
-10
lines changed

‎packages/cli/src/constructs/__tests__/browser-check.spec.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,4 +201,91 @@ describe('BrowserCheck', () => {
201201
const bundle = await check.bundle()
202202
expect(bundle.synthesize()).toMatchObject({ groupId: { ref: 'main-group' } })
203203
})
204+
205+
describe('playwrightConfig', () => {
206+
it('should set from check defaults', () => {
207+
Session.project = new Project('project-id', {
208+
name: 'Test Project',
209+
repoUrl: 'https://github.com/checkly/checkly-cli',
210+
})
211+
const defaults = {
212+
playwrightConfig: {
213+
use: {
214+
baseURL: 'https://example.org',
215+
},
216+
},
217+
}
218+
Session.checkDefaults = {
219+
...defaults,
220+
}
221+
Session.browserCheckDefaults = {}
222+
const browserCheck = new BrowserCheck('test-check', {
223+
name: 'Test Check',
224+
code: { content: 'console.log("test check")' },
225+
})
226+
Session.checkDefaults = undefined
227+
Session.browserCheckDefaults = undefined
228+
expect(browserCheck).toMatchObject(defaults)
229+
})
230+
231+
it('should set from browser check defaults', () => {
232+
Session.project = new Project('project-id', {
233+
name: 'Test Project',
234+
repoUrl: 'https://github.com/checkly/checkly-cli',
235+
})
236+
const defaults = {
237+
playwrightConfig: {
238+
use: {
239+
baseURL: 'https://example.org',
240+
},
241+
},
242+
}
243+
Session.checkDefaults = {}
244+
Session.browserCheckDefaults = {
245+
...defaults,
246+
}
247+
const browserCheck = new BrowserCheck('test-check', {
248+
name: 'Test Check',
249+
code: { content: 'console.log("test check")' },
250+
})
251+
Session.checkDefaults = undefined
252+
Session.browserCheckDefaults = undefined
253+
expect(browserCheck).toMatchObject(defaults)
254+
})
255+
256+
it('should not override with default if set', () => {
257+
Session.project = new Project('project-id', {
258+
name: 'Test Project',
259+
repoUrl: 'https://github.com/checkly/checkly-cli',
260+
})
261+
const defaults = {
262+
playwrightConfig: {
263+
use: {
264+
baseURL: 'https://example.org/foo',
265+
},
266+
},
267+
}
268+
Session.checkDefaults = {
269+
...defaults,
270+
}
271+
Session.browserCheckDefaults = {
272+
...defaults,
273+
}
274+
const custom = {
275+
playwrightConfig: {
276+
use: {
277+
baseURL: 'https://example.org/bar',
278+
}
279+
}
280+
}
281+
const browserCheck = new BrowserCheck('test-check', {
282+
name: 'Test Check',
283+
code: { content: 'console.log("test check")' },
284+
...custom,
285+
})
286+
Session.checkDefaults = undefined
287+
Session.browserCheckDefaults = undefined
288+
expect(browserCheck).toMatchObject(custom)
289+
})
290+
})
204291
})

‎packages/cli/src/constructs/__tests__/multi-step-check.spec.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,4 +140,91 @@ describe('MultistepCheck', () => {
140140
Session.defaultRuntimeId = undefined
141141
}
142142
})
143+
144+
describe('playwrightConfig', () => {
145+
it('should set from check defaults', () => {
146+
Session.project = new Project('project-id', {
147+
name: 'Test Project',
148+
repoUrl: 'https://github.com/checkly/checkly-cli',
149+
})
150+
const defaults = {
151+
playwrightConfig: {
152+
use: {
153+
baseURL: 'https://example.org',
154+
},
155+
},
156+
}
157+
Session.checkDefaults = {
158+
...defaults,
159+
}
160+
Session.browserCheckDefaults = {}
161+
const browserCheck = new MultiStepCheck('test-check', {
162+
name: 'Test Check',
163+
code: { content: 'console.log("test check")' },
164+
})
165+
Session.checkDefaults = undefined
166+
Session.browserCheckDefaults = undefined
167+
expect(browserCheck).toMatchObject(defaults)
168+
})
169+
170+
it('should set from multi step check defaults', () => {
171+
Session.project = new Project('project-id', {
172+
name: 'Test Project',
173+
repoUrl: 'https://github.com/checkly/checkly-cli',
174+
})
175+
const defaults = {
176+
playwrightConfig: {
177+
use: {
178+
baseURL: 'https://example.org',
179+
},
180+
},
181+
}
182+
Session.checkDefaults = {}
183+
Session.multiStepCheckDefaults = {
184+
...defaults,
185+
}
186+
const browserCheck = new MultiStepCheck('test-check', {
187+
name: 'Test Check',
188+
code: { content: 'console.log("test check")' },
189+
})
190+
Session.checkDefaults = undefined
191+
Session.multiStepCheckDefaults = undefined
192+
expect(browserCheck).toMatchObject(defaults)
193+
})
194+
195+
it('should not override with default if set', () => {
196+
Session.project = new Project('project-id', {
197+
name: 'Test Project',
198+
repoUrl: 'https://github.com/checkly/checkly-cli',
199+
})
200+
const defaults = {
201+
playwrightConfig: {
202+
use: {
203+
baseURL: 'https://example.org/foo',
204+
},
205+
},
206+
}
207+
Session.checkDefaults = {
208+
...defaults,
209+
}
210+
Session.browserCheckDefaults = {
211+
...defaults,
212+
}
213+
const custom = {
214+
playwrightConfig: {
215+
use: {
216+
baseURL: 'https://example.org/bar',
217+
}
218+
}
219+
}
220+
const browserCheck = new MultiStepCheck('test-check', {
221+
name: 'Test Check',
222+
code: { content: 'console.log("test check")' },
223+
...custom,
224+
})
225+
Session.checkDefaults = undefined
226+
Session.browserCheckDefaults = undefined
227+
expect(browserCheck).toMatchObject(custom)
228+
})
229+
})
143230
})

‎packages/cli/src/constructs/browser-check.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,11 @@ export class BrowserCheck extends RuntimeCheck {
9191
constructor (logicalId: string, props: BrowserCheckProps) {
9292
super(logicalId, props)
9393

94-
this.code = props.code
95-
this.sslCheckDomain = props.sslCheckDomain
96-
this.playwrightConfig = props.playwrightConfig
94+
const config = this.applyConfigDefaults(props)
95+
96+
this.code = config.code
97+
this.sslCheckDomain = config.sslCheckDomain
98+
this.playwrightConfig = config.playwrightConfig
9799

98100
Session.registerConstruct(this)
99101
this.addSubscriptions()
@@ -109,6 +111,15 @@ export class BrowserCheck extends RuntimeCheck {
109111
)
110112
}
111113

114+
protected applyConfigDefaults<T extends RuntimeCheckProps & Pick<BrowserCheckProps, 'playwrightConfig'>> (props: T): T {
115+
const config = super.applyConfigDefaults(props)
116+
const defaults = this.configDefaultsGetter(props)
117+
118+
config.playwrightConfig ??= defaults("playwrightConfig")
119+
120+
return config
121+
}
122+
112123
async validate (diagnostics: Diagnostics): Promise<void> {
113124
if (!isEntrypoint(this.code) && !isContent(this.code)) {
114125
diagnostics.add(new InvalidPropertyValueDiagnostic(

‎packages/cli/src/constructs/multi-step-check.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ export class MultiStepCheck extends RuntimeCheck {
4040
constructor (logicalId: string, props: MultiStepCheckProps) {
4141
super(logicalId, props)
4242

43-
this.code = props.code
44-
this.playwrightConfig = props.playwrightConfig
43+
const config = this.applyConfigDefaults(props)
44+
45+
this.code = config.code
46+
this.playwrightConfig = config.playwrightConfig
4547

4648
Session.registerConstruct(this)
4749
this.addSubscriptions()
@@ -97,6 +99,15 @@ export class MultiStepCheck extends RuntimeCheck {
9799
)
98100
}
99101

102+
protected applyConfigDefaults<T extends RuntimeCheckProps & Pick<MultiStepCheckProps, 'playwrightConfig'>> (props: T): T {
103+
const config = super.applyConfigDefaults(props)
104+
const defaults = this.configDefaultsGetter(props)
105+
106+
config.playwrightConfig ??= defaults("playwrightConfig")
107+
108+
return config
109+
}
110+
100111
static async bundle (entry: string, runtimeId?: string) {
101112
const runtime = Session.getRuntime(runtimeId)
102113
if (!runtime) {

‎packages/cli/src/services/checkly-config-codegen.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ function buildCheckConfigDefaults (
107107
if (resource.alertEscalationPolicy !== undefined) {
108108
builder.value('alertEscalationPolicy', valueForAlertEscalation(file, resource.alertEscalationPolicy))
109109
}
110+
111+
if (resource.playwrightConfig !== undefined) {
112+
builder.value('playwrightConfig', valueForPlaywrightConfig(resource.playwrightConfig))
113+
}
110114
}
111115

112116
function valueForStringOrStringArray (value: string | string[]) {
@@ -164,10 +168,6 @@ more information.`))
164168
})
165169
}
166170

167-
if (checks.playwrightConfig !== undefined) {
168-
builder.value('playwrightConfig', valueForPlaywrightConfig(checks.playwrightConfig))
169-
}
170-
171171
buildCheckConfigDefaults(program, file, context, builder, checks)
172172

173173
if (checks.browserChecks !== undefined) {

‎packages/cli/src/services/checkly-config-loader.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export type CheckConfigDefaults =
2828
Pick<RuntimeCheckProps,
2929
| 'environmentVariables'
3030
| 'runtimeId'
31-
>
31+
> &
32+
// This is used by BrowserChecks and MultiStepChecks.
33+
{ playwrightConfig?: PlaywrightConfig }
3234

3335
export type PlaywrightSlimmedProp = Pick<PlaywrightCheckProps, 'name' | 'activated'
3436
| 'muted' | 'shouldFail' | 'locations' | 'tags' | 'frequency' | 'environmentVariables'

0 commit comments

Comments
 (0)