test: fix e2e web settings issues locator W-21754661 (#7069)

* test: fix e2e web settings issues locator

* chore: comment cleanup

* test: e2e web settings for newer vscode versions

* docs: e2e test running/iteration

* test: e2e windows settings overlay close

* test: settings close after upsert

Author: mshanemc

.claude/skills/playwright-e2e/references/iterating-playwright-tests.md

@@ -11,7 +11,17 @@ Scratch org setup: see `references/local-setup.md`.
 1. run `web` locally (use `--retries 0`, follow debugging tips)
 2. run `desktop` locally (use `--retries 0`)
 
-**Passing args:** Use `--` to forward params to the underlying command, e.g. `npm run test:web -w <package> -- --retries 0` or `npm run test:desktop -w <package> -- --retries 0` 3. edit github workflows if needed 4. CI (windows, gha) - see `analyze-e2e.md` for monitoring and analyzing results
+**Passing args:** Use `--` to forward params to the underlying playwright command. Prefer file path over `--grep` (exact match, immune to title changes):
+```bash
+WIREIT_CACHE=none npm run test:web -w <package> -- --retries 0 test/playwright/specs/myTest.headless.spec.ts
+WIREIT_CACHE=none npm run test:desktop -w <package> -- --retries 0 test/playwright/specs/myTest.headless.spec.ts
+```
+`WIREIT_CACHE=none` is always required when running a subset — wireit's cache key is based on input file fingerprints, not CLI args, so it serves the cached full-suite result otherwise.
+
+**Port conflict (web):** If a previous web server is still on port 3001, playwright fails immediately with `http://localhost:3001 is already used`. Fix: `lsof -ti :3001 | xargs kill -9`
+
+3. edit github workflows if needed
+4. CI (windows, gha) - see `analyze-e2e.md` for monitoring and analyzing results
 
 After passing, clean up while keeping tests passing:
 

packages/playwright-vscode-ext/src/pages/settings.ts

@@ -11,6 +11,7 @@ import { saveScreenshot } from '../shared/screenshotUtils';
 import {
   waitForVSCodeWorkbench,
   closeWelcomeTabs,
+  closeSettingsTab,
   waitForWorkspaceReady,
   isMacDesktop,
   isDesktop
@@ -120,15 +121,23 @@ export const upsertSettings = async (page: Page, settings: Record<string, string
     // Wait for search results to appear - wait for any search result element to indicate search completed
     await page.locator('[data-id^="searchResultModel_"]').first().waitFor({ state: 'attached', timeout: 15_000 });
 
-    // Deterministic locator: target the element that actually contains the `data-id` attribute
-    // VS Code only replaces the FIRST dot with underscore in data-id
-    // e.g., "salesforcedx-vscode-metadata.deployOnSave.enabled" -> "searchResultModel_salesforcedx-vscode-metadata_deployOnSave.enabled"
+    // VS Code sanitizeId(key) builds the data-id, but the behavior changed in 1.113:
+    // < 1.113: replace(/[\.\/]/, '_') — only the FIRST dot replaced
+    //          e.g. "a.b.c" → "searchResultModel_a_b.c"
+    // ≥ 1.113: replace(/[\.\/]/g, '_') — ALL dots replaced
+    //          e.g. "a.b.c" → "searchResultModel_a_b_c"
+    // Use an OR selector so this works across VS Code versions.
     // Use .last() when duplicates exist (User + Workspace): we're on Workspace tab, so Workspace row is last
-    const searchResultId = `searchResultModel_${id.replace(/\./, '_')}`;
-    const row = page.locator(`[data-id="${searchResultId}"]`).last();
+    const allDotsId = `searchResultModel_${id.replaceAll('.', '_')}`;
+    const firstDotId = `searchResultModel_${id.replace('.', '_')}`;
+    const dataIdSelector =
+      allDotsId === firstDotId
+        ? `[data-id="${allDotsId}"]`
+        : `[data-id="${allDotsId}"], [data-id="${firstDotId}"]`;
+    const row = page.locator(dataIdSelector).last();
 
     if (debugAria) {
-      console.log(`[upsertSettings] using deterministic locator for ${id}: data-id="${searchResultId}"`);
+      console.log(`[upsertSettings] using deterministic locator for ${id}: selector="${dataIdSelector}"`);
       // Capture HTML to debug selector issues - look for settings results
       try {
         const settingsBody = page.locator('.settings-body, .settings-tree-container, [class*="settings"]').first();
@@ -220,4 +229,7 @@ export const upsertSettings = async (page: Page, settings: Record<string, string
       } catch {}
     }
   }
+
+  // Close the settings overlay/tab so callers can open command palette etc.
+  await closeSettingsTab(page);
 };

packages/playwright-vscode-ext/src/utils/helpers.ts

@@ -200,8 +200,13 @@ export const closeWelcomeTabs = async (page: Page): Promise<void> => {
   }).toPass({ timeout: 30_000 });
 };
 
-/** Closes any visible Settings tabs */
+/** Closes any visible Settings tabs or the floating Settings overlay on Windows desktop */
 export const closeSettingsTab = async (page: Page): Promise<void> => {
+  if (isWindowsDesktop()) {
+    // On Windows desktop, Settings opens as a floating overlay dialog, not a tab
+    await page.keyboard.press('Escape');
+    return;
+  }
   const settingsTab = page
     .locator(TAB)
     .filter({ hasText: /Settings/i })

packages/playwright-vscode-ext/test/playwright/specs/settings.headless.spec.ts

@@ -98,8 +98,13 @@ test.describe.serial('Settings', () => {
       await page.keyboard.press('Backspace');
       await page.keyboard.type(settingKey);
 
-      const searchResultId = `searchResultModel_${settingKey.replace(/\./, '_')}`;
-      const row = page.locator(`[data-id="${searchResultId}"]`).last();
+      const allDotsId = `searchResultModel_${settingKey.replaceAll('.', '_')}`;
+      const firstDotId = `searchResultModel_${settingKey.replace('.', '_')}`;
+      const dataIdSelector =
+        allDotsId === firstDotId
+          ? `[data-id="${allDotsId}"]`
+          : `[data-id="${allDotsId}"], [data-id="${firstDotId}"]`;
+      const row = page.locator(dataIdSelector).last();
       await row.waitFor({ state: 'visible', timeout: 15_000 });
       const minimapCheckbox = row.getByRole('checkbox').first();
       await expect(minimapCheckbox).not.toBeChecked();