Skip to content

Commit 8d4478c

Browse files
DavertMikgololdf1shclaudeDavertMik
authored
Fix/pr 5554 rebased (#5557)
* fix(plugins): resolve async race conditions in aiTrace, analyze, screencast, pageInfo, heal Fix 8 bugs across 6 plugin files where async operations outside the recorder chain, missing force flags, and incorrect filtering caused silent data loss, premature process exit, and broken healing limits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(analyze,screencast): rework for new autoExit + fix unit tests analyze.js: CODECEPT_DISABLE_AUTO_EXIT was removed in #5556 (autoExit refactor). Push printReport onto the recorder so it's awaited inside codecept.run()'s done() before autoExit fires. screencast tests: emit event.test.started in unit tests to match the production event sequence (asyncWrapper.js fires it between event.test.before and the first event.step.started). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: gololdf1sh <oleksandr.kiriukhin@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: DavertMik <davert@testomat.io>
1 parent 4c0689f commit 8d4478c

7 files changed

Lines changed: 46 additions & 31 deletions

File tree

lib/heal.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ class Heal {
5454

5555
async getCodeSuggestions(context) {
5656
const suggestions = []
57+
const stepName = context.step?.name
5758
const recipes = matchRecipes(this.recipes, this.contextName)
59+
.filter(r => !r.steps || !stepName || r.steps.includes(stepName))
5860

5961
debug('Recipes', recipes)
6062

lib/plugin/aiTrace.js

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export default function (config = {}) {
9292
let testStartTime
9393
let currentUrl = null
9494
let testFailed = false
95+
let pendingArtifactCapture = null
9596
let firstFailedStepSaved = false
9697

9798
const reportDir = config.output ? path.resolve(store.codeceptDir, config.output) : defaultConfig.output
@@ -129,6 +130,7 @@ export default function (config = {}) {
129130
currentUrl = null
130131
testFailed = false
131132
firstFailedStepSaved = false
133+
pendingArtifactCapture = null
132134
})
133135

134136
event.dispatcher.on(event.step.after, step => {
@@ -162,13 +164,12 @@ export default function (config = {}) {
162164
return
163165
}
164166

165-
const stepPersistPromise = persistStep(step).catch(err => {
167+
recorder.add(`aiTrace step persistence: ${step.toString()}`, () => persistStep(step).catch(err => {
166168
output.debug(`aiTrace: Error saving step: ${err.message}`)
167-
})
168-
recorder.add(`wait aiTrace step persistence: ${step.toString()}`, () => stepPersistPromise, true)
169+
}), true)
169170
})
170171

171-
event.dispatcher.on(event.step.failed, async step => {
172+
event.dispatcher.on(event.step.failed, step => {
172173
if (!currentTest) return
173174
if (step.status === 'queued' && testFailed) {
174175
output.debug(`aiTrace: Skipping queued failed step "${step.toString()}" - testFailed: ${testFailed}`)
@@ -188,11 +189,9 @@ export default function (config = {}) {
188189
}
189190
existingStep.status = 'failed'
190191

191-
try {
192-
await captureArtifactsForStep(step, existingStep, existingStep.prefix)
193-
} catch (err) {
192+
pendingArtifactCapture = captureArtifactsForStep(step, existingStep, existingStep.prefix).catch(err => {
194193
output.debug(`aiTrace: Error updating failed step: ${err.message}`)
195-
}
194+
})
196195
} else {
197196
if (stepNum === -1) return
198197
if (isStepIgnored(step)) return
@@ -218,11 +217,9 @@ export default function (config = {}) {
218217
steps.push(stepData)
219218
firstFailedStepSaved = true
220219

221-
try {
222-
await captureArtifactsForStep(step, stepData, stepPrefix)
223-
} catch (err) {
220+
pendingArtifactCapture = captureArtifactsForStep(step, stepData, stepPrefix).catch(err => {
224221
output.debug(`aiTrace: Error capturing failed step artifacts: ${err.message}`)
225-
}
222+
})
226223
}
227224
})
228225

@@ -238,7 +235,13 @@ export default function (config = {}) {
238235
if (hookName === 'BeforeSuite' || hookName === 'AfterSuite') {
239236
return
240237
}
241-
persist(test, 'failed')
238+
recorder.add('aiTrace:persist failed', async () => {
239+
if (pendingArtifactCapture) {
240+
await pendingArtifactCapture
241+
pendingArtifactCapture = null
242+
}
243+
persist(test, 'failed')
244+
}, true)
242245
})
243246

244247
async function persistStep(step) {

lib/plugin/analyze.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const ai = aiModule.default || aiModule
1212
import colors from 'chalk'
1313
import ora from 'ora'
1414
import event from '../event.js'
15+
import recorder from '../recorder.js'
1516

1617
import output from '../output.js'
1718

@@ -227,14 +228,14 @@ export default function (config = {}) {
227228
console.log('Enabled AI analysis')
228229
})
229230

230-
event.dispatcher.on(event.all.result, async result => {
231+
event.dispatcher.on(event.all.result, result => {
231232
if (!isMainThread) return // run only on main thread
232233
if (!ai.isEnabled) {
233234
console.log('AI is disabled, no analysis will be performed. Run tests with --ai flag to enable it.')
234235
return
235236
}
236237

237-
printReport(result)
238+
recorder.add('analyze:print-ai-report', () => printReport(result), true)
238239
})
239240

240241
event.dispatcher.on(event.workers.result, async result => {
@@ -248,7 +249,7 @@ export default function (config = {}) {
248249
return
249250
}
250251

251-
printReport(result)
252+
await printReport(result)
252253
})
253254

254255
async function printReport(result) {
@@ -294,7 +295,7 @@ export default function (config = {}) {
294295
console.error('Error analyzing failed tests', err)
295296
}
296297

297-
if (!Object.keys(container.plugins()).includes('pageInfo')) {
298+
if (!Object.keys(Container.plugins()).includes('pageInfo')) {
298299
console.log('To improve analysis, enable pageInfo plugin to get more context for failed tests.')
299300
}
300301
}

lib/plugin/heal.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export default function (config = {}) {
8080
event.dispatcher.on(event.test.before, test => {
8181
currentTest = test
8282
healedSteps = 0
83+
healTries = 0
8384
caughtError = null
8485
})
8586

@@ -94,7 +95,9 @@ export default function (config = {}) {
9495
if (trigger.on === 'file' && !matchStepFile(step, trigger.path, trigger.line)) return
9596

9697
recorder.catchWithoutStop(async err => {
98+
if (healTries >= config.healLimit) throw err
9799
isHealing = true
100+
healTries++
98101
if (caughtError === err) throw err // avoid double handling
99102
caughtError = err
100103

@@ -121,8 +124,6 @@ export default function (config = {}) {
121124

122125
await heal.healStep(step, err, { test })
123126

124-
healTries++
125-
126127
recorder.add('close healing session', () => {
127128
recorder.reset()
128129
recorder.session.restore('heal')

lib/plugin/pageInfo.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ const defaultConfig = {
4040
export default function (config = {}) {
4141
config = Object.assign(defaultConfig, config)
4242

43-
const helper = pickActingHelper(Container.helpers())
44-
if (!helper) return
45-
4643
event.dispatcher.on(event.test.failed, test => {
44+
const helper = pickActingHelper(Container.helpers())
45+
if (!helper) return
46+
4747
const pageState = {}
4848

4949
recorder.add('pageInfo capture', async () => {
@@ -60,8 +60,6 @@ export default function (config = {}) {
6060
if (captured.html) {
6161
const htmlPath = path.join(store.outputDir, captured.html)
6262
pageState.htmlSnapshot = htmlPath
63-
// Scan raw HTML (pre-cleanHtml) so error classes containing digits
64-
// or trash-class prefixes aren't stripped before detection.
6563
const htmlForScan = captured.htmlRaw || (() => {
6664
try { return fs.readFileSync(htmlPath, 'utf8') } catch { return '' }
6765
})()
@@ -90,7 +88,7 @@ export default function (config = {}) {
9088
} catch {}
9189
}
9290
} catch {}
93-
})
91+
}, true)
9492

9593
recorder.add('Save page info', () => {
9694
test.addNote('pageInfo', pageStateToMarkdown(pageState))
@@ -99,7 +97,7 @@ export default function (config = {}) {
9997
fs.writeFileSync(pageStateFileName, pageStateToMarkdown(pageState))
10098
test.artifacts.pageInfo = pageStateFileName
10199
return pageState
102-
})
100+
}, true)
103101
})
104102
}
105103

lib/plugin/screencast.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@ function wireScreencast(mode, options) {
106106
state.startedAt = options.subtitles ? Date.now() : null
107107
})
108108

109+
event.dispatcher.on(event.test.started, test => {
110+
if (!options.video || state.startQueued) return
111+
state.startQueued = true
112+
recorder.add('screencast:start', async () => startScreencast(state.test, options, state), true)
113+
})
114+
109115
event.dispatcher.on(event.step.started, step => {
110116
if (state.steps) {
111117
const at = Date.now()
@@ -116,10 +122,6 @@ function wireScreencast(mode, options) {
116122
title: stepTitle(step),
117123
}
118124
}
119-
if (!options.video || state.startQueued || !state.test) return
120-
state.startQueued = true
121-
const test = state.test
122-
recorder.add('screencast:start', async () => startScreencast(test, options, state), true)
123125
})
124126

125127
if (options.subtitles) {

test/unit/plugin/screencast_test.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ function fakeHelper(screencastApi) {
3434

3535
function detachAll() {
3636
for (const evt of [
37-
event.test.before, event.test.after, event.test.failed,
37+
event.test.before, event.test.started, event.test.after, event.test.failed,
3838
event.step.started, event.step.finished,
3939
]) {
4040
event.dispatcher.removeAllListeners(evt)
@@ -73,6 +73,7 @@ describe('screencast', () => {
7373
const test = createTest('keep-on-pass')
7474

7575
event.dispatcher.emit(event.test.before, test)
76+
event.dispatcher.emit(event.test.started, test)
7677
event.dispatcher.emit(event.step.started, aStep())
7778
await recorder.promise()
7879

@@ -93,6 +94,7 @@ describe('screencast', () => {
9394
const test = createTest('delete-on-pass')
9495

9596
event.dispatcher.emit(event.test.before, test)
97+
event.dispatcher.emit(event.test.started, test)
9698
event.dispatcher.emit(event.step.started, aStep())
9799
await recorder.promise()
98100

@@ -114,6 +116,7 @@ describe('screencast', () => {
114116
const test = createTest('keep-on-fail')
115117

116118
event.dispatcher.emit(event.test.before, test)
119+
event.dispatcher.emit(event.test.started, test)
117120
event.dispatcher.emit(event.step.started, aStep())
118121
await recorder.promise()
119122

@@ -132,6 +135,7 @@ describe('screencast', () => {
132135
screencast({ on: 'test', captions: true })
133136
let test = createTest('with-captions')
134137
event.dispatcher.emit(event.test.before, test)
138+
event.dispatcher.emit(event.test.started, test)
135139
event.dispatcher.emit(event.step.started, aStep())
136140
await recorder.promise()
137141
event.dispatcher.emit(event.test.after, test)
@@ -144,6 +148,7 @@ describe('screencast', () => {
144148
screencast({ on: 'test', captions: false })
145149
test = createTest('no-captions')
146150
event.dispatcher.emit(event.test.before, test)
151+
event.dispatcher.emit(event.test.started, test)
147152
event.dispatcher.emit(event.step.started, aStep())
148153
await recorder.promise()
149154
event.dispatcher.emit(event.test.after, test)
@@ -159,6 +164,7 @@ describe('screencast', () => {
159164
const test = createTest('with-srt')
160165

161166
event.dispatcher.emit(event.test.before, test)
167+
event.dispatcher.emit(event.test.started, test)
162168
await recorder.promise()
163169

164170
const step = { name: 'click', actor: 'I', args: ['Continue'] }
@@ -183,6 +189,7 @@ describe('screencast', () => {
183189
test.artifacts.video = '/tmp/some-video-dir/myrun.webm'
184190

185191
event.dispatcher.emit(event.test.before, test)
192+
event.dispatcher.emit(event.test.started, test)
186193
const step = { name: 'see', actor: 'I', args: ['Github'] }
187194
event.dispatcher.emit(event.step.started, step)
188195
event.dispatcher.emit(event.step.finished, step)
@@ -202,6 +209,7 @@ describe('screencast', () => {
202209
const test = createTest('no-api')
203210

204211
event.dispatcher.emit(event.test.before, test)
212+
event.dispatcher.emit(event.test.started, test)
205213
await recorder.promise()
206214
event.dispatcher.emit(event.test.after, test)
207215
await recorder.promise()

0 commit comments

Comments
 (0)