Skip to content

Commit 57890d8

Browse files
committed
Cucumber testrunner command capture fix
1 parent 11f56c2 commit 57890d8

6 files changed

Lines changed: 250 additions & 46 deletions

File tree

packages/app/src/controller/DataManager.ts

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,20 @@ export const isTestRunningContext = createContext<boolean>(
4949
)
5050

5151
interface SocketMessage<
52-
T extends keyof TraceLog | 'testStopped' | 'clearExecutionData' =
52+
T extends keyof TraceLog | 'testStopped' | 'clearExecutionData' | 'replaceCommand' =
5353
| keyof TraceLog
5454
| 'testStopped'
5555
| 'clearExecutionData'
56+
| 'replaceCommand'
5657
> {
5758
scope: T
5859
data: T extends keyof TraceLog
5960
? TraceLog[T]
6061
: T extends 'clearExecutionData'
6162
? { uid?: string }
62-
: unknown
63+
: T extends 'replaceCommand'
64+
? { oldTimestamp: number; command: CommandLog }
65+
: unknown
6366
}
6467

6568
export class DataManagerController implements ReactiveController {
@@ -285,6 +288,14 @@ export class DataManagerController implements ReactiveController {
285288
return
286289
}
287290

291+
// Handle in-place command replacement (retry deduplication)
292+
if (scope === 'replaceCommand') {
293+
const { oldTimestamp, command } = data as { oldTimestamp: number; command: CommandLog }
294+
this.#handleReplaceCommand(oldTimestamp, command)
295+
this.#host.requestUpdate()
296+
return
297+
}
298+
288299
// Check for new run BEFORE processing suites data
289300
if (scope === 'suites') {
290301
const shouldReset = this.#shouldResetForNewRun(data)
@@ -338,12 +349,41 @@ export class DataManagerController implements ReactiveController {
338349
? suite.start.getTime()
339350
: typeof suite.start === 'number'
340351
? suite.start
341-
: 0
352+
: typeof suite.start === 'string'
353+
? new Date(suite.start as string).getTime() || 0
354+
: 0
355+
356+
if (suiteStartTime <= 0) {
357+
continue
358+
}
342359

343-
// New run detected if we see a newer start timestamp
360+
// New run detected if we see a newer start timestamp.
361+
// Exception: if the existing suite for this uid has no end time, it is
362+
// still an ongoing run (e.g. a Cucumber feature spanning multiple
363+
// scenarios) — treat it as a continuation, not a new run.
344364
if (suiteStartTime > this.#lastSeenRunTimestamp) {
365+
const existingChunks = this.suitesContextProvider.value || []
366+
let existingEnd: unknown = undefined
367+
outer: for (const ec of existingChunks) {
368+
for (const [uid, existing] of Object.entries(
369+
ec as Record<string, SuiteStatsFragment>
370+
)) {
371+
if (uid === Object.keys(chunk)[0]) {
372+
existingEnd = (existing as any)?.end
373+
break outer
374+
}
375+
}
376+
}
377+
// Only reset if the previous run was already finished (had an end time).
378+
// An ongoing run (end == null / undefined) is just a continuation.
379+
const previousRunFinished =
380+
existingEnd !== null && existingEnd !== undefined
381+
if (previousRunFinished) {
382+
this.#lastSeenRunTimestamp = suiteStartTime
383+
return true
384+
}
385+
// Continuation — update tracking timestamp but do NOT reset
345386
this.#lastSeenRunTimestamp = suiteStartTime
346-
return true
347387
}
348388
}
349389
}
@@ -428,6 +468,20 @@ export class DataManagerController implements ReactiveController {
428468
])
429469
}
430470

471+
#handleReplaceCommand(oldTimestamp: number, newCommand: CommandLog) {
472+
const current = this.commandsContextProvider.value || []
473+
// Find the last entry with the matching timestamp (most recent retry)
474+
const idx = current.map((c) => c.timestamp).lastIndexOf(oldTimestamp)
475+
if (idx !== -1) {
476+
const updated = [...current]
477+
updated[idx] = newCommand
478+
this.commandsContextProvider.setValue(updated)
479+
} else {
480+
// No matching entry found — just append
481+
this.commandsContextProvider.setValue([...current, newCommand])
482+
}
483+
}
484+
431485
#handleConsoleLogsUpdate(data: string[]) {
432486
this.consoleLogsContextProvider.setValue([
433487
...(this.consoleLogsContextProvider.value || []),

packages/nightwatch-devtools/src/helpers/browserProxy.ts

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,41 @@ import type { NightwatchBrowser, CommandStackFrame } from '../types.js'
1313
const log = logger('@wdio/nightwatch-devtools:browserProxy')
1414

1515
export class BrowserProxy {
16-
private browserProxied = false
16+
/** Tracks which browser *instances* have already been proxied to avoid double-wrapping. */
17+
private proxiedBrowsers = new WeakSet<object>()
1718
private commandStack: CommandStackFrame[] = []
1819
private lastCommandSig: string | null = null
1920
private currentTestFullPath: string | null = null
21+
/**
22+
* Tracks the last captured command so that consecutive retries of the same
23+
* command (e.g. getText inside a waitFor loop) overwrite the previous entry
24+
* rather than appending, showing only the final execution result.
25+
*/
26+
private lastCapturedSig: string | null = null
27+
private lastCapturedId: number | null = null
2028

2129
constructor(
2230
private sessionCapturer: SessionCapturer,
2331
private testManager: TestManager,
2432
private getCurrentTest: () => any
2533
) {}
2634

35+
/**
36+
* Update the session capturer reference after a WebDriver session change.
37+
* Does NOT re-wrap browser methods — wrapping is permanent per browser object.
38+
*/
39+
updateSessionCapturer(capturer: SessionCapturer): void {
40+
this.sessionCapturer = capturer
41+
}
42+
2743
/**
2844
* Reset command tracking for new test
2945
*/
3046
resetCommandTracking(): void {
3147
this.commandStack = []
3248
this.lastCommandSig = null
49+
this.lastCapturedSig = null
50+
this.lastCapturedId = null
3351
}
3452

3553
/**
@@ -82,7 +100,7 @@ export class BrowserProxy {
82100
* Wrap all browser commands to capture them
83101
*/
84102
wrapBrowserCommands(browser: NightwatchBrowser): void {
85-
if (this.browserProxied) {
103+
if (this.proxiedBrowsers.has(browser as object)) {
86104
return
87105
}
88106

@@ -123,7 +141,7 @@ export class BrowserProxy {
123141
wrappedMethods.push(methodName)
124142
})
125143

126-
this.browserProxied = true
144+
this.proxiedBrowsers.add(browser as object)
127145
log.info(`✓ Wrapped ${wrappedMethods.length} browser methods`)
128146
}
129147

@@ -243,8 +261,14 @@ export class BrowserProxy {
243261
const effectiveUid = currentTest?.uid ?? testUid
244262

245263
if (effectiveUid) {
246-
this.sessionCapturer
247-
.captureCommand(
264+
const isRetry =
265+
cmdSig === this.lastCapturedSig && this.lastCapturedId !== null
266+
267+
if (isRetry) {
268+
// Same command fired again (internal retry) — replace the previous
269+
// entry so only the final result appears in the UI.
270+
const { entry, oldTimestamp } = this.sessionCapturer.replaceCommand(
271+
this.lastCapturedId!,
248272
methodName,
249273
logArgs,
250274
serializedResult,
@@ -253,18 +277,40 @@ export class BrowserProxy {
253277
callSource,
254278
commandTimestamp
255279
)
256-
.then(() => {
257-
const lastCommand =
258-
this.sessionCapturer.commandsLog[
259-
this.sessionCapturer.commandsLog.length - 1
260-
]
261-
if (lastCommand) {
262-
this.sessionCapturer.sendCommand(lastCommand)
263-
}
264-
})
265-
.catch((err: any) =>
266-
log.error(`Failed to capture ${methodName}: ${err.message}`)
267-
)
280+
this.lastCapturedId = entry._id ?? null
281+
this.sessionCapturer.sendReplaceCommand(oldTimestamp, entry)
282+
} else {
283+
// New command — capture and track.
284+
// captureCommand() pushes the entry to commandsLog synchronously
285+
// before any async work (navigation perf capture), so we can grab
286+
// the ID immediately after the call — before any microtask fires.
287+
// This avoids the race where a Nightwatch retry callback executes
288+
// before .then() sets lastCapturedId, causing missed dedup.
289+
this.lastCapturedSig = cmdSig
290+
this.lastCapturedId = null
291+
this.sessionCapturer
292+
.captureCommand(
293+
methodName,
294+
logArgs,
295+
serializedResult,
296+
undefined,
297+
effectiveUid,
298+
callSource,
299+
commandTimestamp
300+
)
301+
.catch((err: any) =>
302+
log.error(`Failed to capture ${methodName}: ${err.message}`)
303+
)
304+
// Read the entry synchronously — it was already pushed above.
305+
const lastCommand =
306+
this.sessionCapturer.commandsLog[
307+
this.sessionCapturer.commandsLog.length - 1
308+
]
309+
if (lastCommand) {
310+
this.lastCapturedId = (lastCommand as any)._id ?? null
311+
this.sessionCapturer.sendCommand(lastCommand)
312+
}
313+
}
268314
}
269315

270316
// Forward to the user's original callback (if any)
@@ -330,9 +376,9 @@ export class BrowserProxy {
330376
}
331377

332378
/**
333-
* Check if browser is already proxied
379+
* Check if a specific browser instance is already proxied
334380
*/
335-
isProxied(): boolean {
336-
return this.browserProxied
381+
isProxied(browser: NightwatchBrowser): boolean {
382+
return this.proxiedBrowsers.has(browser as object)
337383
}
338384
}

packages/nightwatch-devtools/src/helpers/utils.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,21 @@ export function resetSignatureCounters() {
4949
signatureCounters.clear()
5050
}
5151

52+
/**
53+
* Compute a purely deterministic UID from arbitrary string parts.
54+
* Unlike generateStableUid this NEVER uses the signature counter, so calling
55+
* it multiple times with the same inputs always returns the same value.
56+
* Use this wherever the same entity (e.g. a Cucumber scenario) must map to
57+
* the same UID across retries.
58+
*/
59+
export function deterministicUid(...parts: string[]): string {
60+
const hash = parts
61+
.join('::')
62+
.split('')
63+
.reduce((acc, char) => ((acc << 5) - acc + char.charCodeAt(0)) | 0, 0)
64+
return `stable-${Math.abs(hash).toString(36)}`
65+
}
66+
5267
// File patterns for test file identification
5368
const SPEC_FILE_PATTERN = /\/(test|spec|tests)\//i
5469
const SPEC_FILE_RE = /\.(?:test|spec)\.[cm]?[jt]sx?$/i

0 commit comments

Comments
 (0)