|
| 1 | +# W-23073729 — E2E flake: LWC/Aura 2nd rename (editor context-menu) race |
| 2 | + |
| 3 | +## Context |
| 4 | + |
| 5 | +2nd rename via editor context-menu intermittently no-ops; renamed treeitem never appears. |
| 6 | + |
| 7 | +- LWC `lwcRename.headless.spec.ts` retryRate 86% (flakes thru `toPass(20s)`). |
| 8 | +- Aura `auraRename.desktop.spec.ts` HARD-FAILS — LWC mitigations never back-ported. |
| 9 | + |
| 10 | +Already-fixed LWC (lwcRename:92-108): |
| 11 | + |
| 12 | +- atomic `activeQuickInputTextField(page).fill(finalName, { force: true })` (line 95) |
| 13 | +- `toPass({ timeout: 20_000 })` poll on renamed treeitem (line 106) |
| 14 | + |
| 15 | +Aura old racy (auraRename:82-96): |
| 16 | + |
| 17 | +- `ControlOrMeta+a` + `keyboard.type` (lines 85-86) — select-all/type focus race |
| 18 | +- `toBeVisible({ timeout: 10_000 })`, no poll (line 96) |
| 19 | + |
| 20 | +Shared root: `executeEditorContextMenuCommand` (contextMenu.ts:128) opens editor menu by `data-uri` substring (openEditorContextMenu:13-41). The fileName match is a **synchronous for-loop snapshot** (`count()` then `getAttribute` per editor, :22-35): it reads the editor list ONCE with no retry. For static-path callers (deploy/retrieve/manifest pass `manifest/package.xml`, `${className}.cls` — editor already open) editor exists instantly, snapshot finds it. For the 2nd-rename callers, the renamed `data-uri` is still settling when the snapshot runs → no match (throws) or matches a stale tab. Plus `selectContextMenuItem` keyboard-Enter then a best-effort menu-hidden wait whose catch (:122) hides whether the command actually fired. |
| 21 | + |
| 22 | +### Caller inventory (must not regress) |
| 23 | + |
| 24 | +`executeEditorContextMenuCommand` callers — only the fileName-match path changes; the no-fileName `.first()` path (apex-oas :53) is untouched: |
| 25 | + |
| 26 | +- static-path (editor pre-open, expects instant match): `deploySourcePath`, `deployManifest`, `generateManifest`, `retrieveInManifest` (x2), `nonTrackingOrgDeployRetrieveManifest` (x2) — all in `salesforcedx-vscode-metadata` |
| 27 | +- no-fileName (`.first()`): `apex-oas/contextMenuEditor` (:53), `playwright-vscode-ext/contextMenu.headless` (:34) |
| 28 | +- settling-URI (the flake): `lwcRename` (:93), `auraRename` (:83) |
| 29 | + |
| 30 | +`selectContextMenuItem` is also reached by `executeExplorerContextMenuCommand` (all explorer-menu callers) — any change here is global. |
| 31 | + |
| 32 | +Files: |
| 33 | + |
| 34 | +- `packages/salesforcedx-vscode-lightning/test/playwright/specs/auraRename.desktop.spec.ts` |
| 35 | +- `packages/playwright-vscode-ext/src/pages/contextMenu.ts` |
| 36 | +- (ref only) `packages/salesforcedx-vscode-lwc/test/playwright/specs/lwcRename.headless.spec.ts` |
| 37 | + |
| 38 | +Helpers: `activeQuickInputTextField` (quickInput.ts:21), `EDITOR_WITH_URI`/`CONTEXT_MENU` (locators.ts). |
| 39 | + |
| 40 | +## Phases |
| 41 | + |
| 42 | +### Phase 1 — harden openEditorContextMenu fileName match (settling URI only) |
| 43 | + |
| 44 | +`contextMenu.ts:13-50`. Goal: settling renamed URI → bounded wait; static-path callers → instant match; `selectContextMenuItem` behavior unchanged. |
| 45 | + |
| 46 | +**Match: auto-retrying locator (not expect.poll).** CSS attribute-substring locators auto-retry: instant when open, bounded wait when settling — the exact behavioral split the findings require, without changing static-path callers. |
| 47 | + |
| 48 | +- delete the for-loop (:22-41); build the target as one locator: |
| 49 | + `const editor = fileName ? page.locator(`${EDITOR_WITH_URI}${cssAttrSubstring('data-uri', fileName)}`).first() : page.locator(EDITOR_WITH_URI).first();` |
| 50 | + where `data-uri*="<escaped>"` mirrors the old `dataUri.includes(fileName)` substring semantics. fileName values are paths (`manifest/package.xml`, `${newName}.js`, `${newName}.cmp`) — escape `"`/`\` for the CSS string, or use Playwright `getByXxx`-style escaping; no fileName contains quotes today but escape anyway. |
| 51 | +- keep the existing `await editor.waitFor({ state: 'visible', timeout: 10_000 })` (:45) — this is now the single auto-retry point: instant for static-path callers (editor already visible), waits up-to-10s for the renamed URI to attach+settle. |
| 52 | +- timeout semantics to document in the plan/PR: static-path callers resolve on first poll (no added latency); settling-URI callers retry up to 10s then throw a clear "editor not visible" error. The OLD error string ("No editor found … Available data-uris") is lost — preserve diagnostics by catching the `waitFor` timeout and rethrowing with `${fileName}` + a list of currently-present `data-uri`s (one extra `count()`/`getAttribute` pass, only on failure). |
| 53 | +- the `EDITOR_WITH_URI.first().waitFor` pre-wait (:16-17) becomes redundant (the combined locator's waitFor covers it) — remove it to avoid a stale first-editor wait masking the targeted wait. |
| 54 | + |
| 55 | +**Menu-fired signal: do NOT throw inside the shared helper.** Findings are correct that dropping the catch at :122 throws for any instant-close caller (explorer + editor, 10+ specs) and there is no in-helper way to distinguish "closed instantly = OK" from "never opened = fail". The helper has no caller-specific success signal to assert on. So: |
| 56 | + |
| 57 | +- leave `selectContextMenuItem` menu-hidden wait + catch (:122-124) unchanged (still best-effort, non-throwing) — global safety, no regression risk. |
| 58 | +- move the real "command fired" assertion to the rename callers, which DO have a deterministic side-effect: the rename quick-input widget. Both rename specs already call `await activeQuickInputWidget(page).waitFor({ state: 'attached', timeout: 10_000 })` immediately after (lwcRename:94, auraRename:84) — that IS the assertion that the editor menu fired. Phase 2 ensures aura has it (it does) and that it fails loud (it already throws on timeout). No new assertion needed; document that the quick-input wait is the menu-fired proof. |
| 59 | + |
| 60 | +commit: `test(e2e): editor context-menu match waits for settling URI - W-23073729` |
| 61 | + |
| 62 | +### Phase 2 — back-port LWC mitigations to Aura |
| 63 | + |
| 64 | +`auraRename.desktop.spec.ts:82-96` |
| 65 | + |
| 66 | +- line 85-86 → atomic fill: replace `ControlOrMeta+a` + `keyboard.type(finalName)` with `activeQuickInputTextField(page).fill(finalName, { force: true })`; import `activeQuickInputTextField` |
| 67 | +- line 96 → `toPass({ timeout: 20_000 })` poll on `finalFolder` visible (mirror lwcRename:106-108) |
| 68 | +- optionally harden 1st-rename block (63-64) same way for consistency — only if low-risk |
| 69 | + |
| 70 | +commit: `test(lightning): back-port lwc rename mitigations to aura 2nd rename - W-23073729` |
| 71 | + |
| 72 | +## Skills to apply |
| 73 | + |
| 74 | +- playwright-e2e (running/iterating/analyze CI) |
| 75 | +- analyze-e2e (CI run inspection) |
| 76 | +- concise (any md) |
| 77 | +- verification |
| 78 | +- typescript |
| 79 | + |
| 80 | +## Verification |
| 81 | + |
| 82 | +- Run lwc + aura rename specs locally, repeat N to confirm flake gone: `npm run test:desktop` in salesforcedx-vscode-lightning (Aura) + salesforcedx-vscode-lwc (LWC). |
| 83 | +- Run every fileName-path caller spec (REQUIRED — helper change is global): prove locator swap keeps instant-match for pre-open editors. Rename specs alone insufficient. |
| 84 | + - `salesforcedx-vscode-metadata`: `deploySourcePath`, `deployManifest`, `generateManifest`, `retrieveInManifest`, `nonTrackingOrgDeployRetrieveManifest`. |
| 85 | + - No-fileName path sanity: `apex-oas/contextMenuEditor`, `playwright-vscode-ext/contextMenu.headless`. |
| 86 | + - Any added latency or "not visible" throw → substring-escape or `.first()` selection wrong; fix before merge. |
| 87 | +- Typecheck/lint `playwright-vscode-ext`: `npm run compile` + eslint. |
| 88 | +- Confirm `selectContextMenuItem` byte-for-byte unchanged (explorer callers depend on instant-close tolerance). |
0 commit comments