fix: don't insert stale recorded action after user edits the file

src/reusedBrowser.ts

@@ -32,6 +32,7 @@ export class ReusedBrowser implements vscodeTypes.Disposable {
   private _cancelRecording: (() => void) | undefined;
   private _isRunningTests?: 'run' | 'debug';
   private _insertedEditActionCount = 0;
+  private _lastInsertedActionText: string | undefined;
   private _envProvider: (configFile: string) => NodeJS.ProcessEnv;
   private _disposables: vscodeTypes.Disposable[] = [];
   private _pageCount = 0;
@@ -172,31 +173,51 @@ export class ReusedBrowser implements vscodeTypes.Disposable {
         if (!params.actions || !params.actions.length)
           return;
         const targetIndentation = guessIndentation(editor);
-
-        // Previous action committed, insert new line & collapse selection.
-        if (params.actions.length > 1 && params.actions?.length > this._insertedEditActionCount) {
-          const range = new this._vscode.Range(editor.selection.end, editor.selection.end);
-          await editor.edit(async editBuilder => {
-            editBuilder.replace(range, '\n' + ' '.repeat(targetIndentation));
-          });
-          editor.selection = new this._vscode.Selection(editor.selection.end, editor.selection.end);
-          this._insertedEditActionCount = params.actions.length;
-        }
-
-        // Replace selection with the current action.
-        if (params.actions.length) {
+        const newText = indentBlock(params.actions[params.actions.length - 1], targetIndentation);
+        const isNewAction = params.actions.length > this._insertedEditActionCount;
+
+        if (isNewAction) {
+          // Previous action committed, insert new line & collapse selection.
+          if (params.actions.length > 1) {
+            const range = new this._vscode.Range(editor.selection.end, editor.selection.end);
+            await editor.edit(async editBuilder => {
+              editBuilder.replace(range, '\n' + ' '.repeat(targetIndentation));
+            });
+            editor.selection = new this._vscode.Selection(editor.selection.end, editor.selection.end);
+          }
+
+          // Replace selection with the current action.
           const selectionStart = editor.selection.start;
           await editor.edit(async editBuilder => {
             if (!editor)
               return;
-            const action = params.actions[params.actions.length - 1];
-            const newText = indentBlock(action, targetIndentation);
             if (editor.document.getText(editor.selection) !== newText)
               editBuilder.replace(editor.selection, newText);
           });
           const selectionEnd = editor.selection.end;
           editor.selection = new this._vscode.Selection(selectionStart, selectionEnd);
+          this._insertedEditActionCount = params.actions.length;
+          this._lastInsertedActionText = newText;
+          return;
         }
+
+        // Re-render of the last action, e.g. a dialog/popup/assertion signal
+        // attached to it, or two fills merged into one. Only update the editor
+        // while the selection still spans the text we had inserted; if the user
+        // edited the document meanwhile, respect their edits.
+        if (newText === this._lastInsertedActionText)
+          return;
+        if (editor.document.getText(editor.selection) !== this._lastInsertedActionText)
+          return;
+        const selectionStart = editor.selection.start;
+        await editor.edit(async editBuilder => {
+          if (!editor)
+            return;
+          editBuilder.replace(editor.selection, newText);
+        });
+        const selectionEnd = editor.selection.end;
+        editor.selection = new this._vscode.Selection(selectionStart, selectionEnd);
+        this._lastInsertedActionText = newText;
       });
     });
   }
@@ -323,6 +344,7 @@ export class ReusedBrowser implements vscodeTypes.Disposable {
   private async _doRecord(progress: vscodeTypes.Progress<{ message?: string; increment?: number }>, model: TestModel, testIdAttributeName: string | undefined, token: vscodeTypes.CancellationToken) {
     await this._startBackendIfNeeded(model.config);
     this._insertedEditActionCount = 0;
+    this._lastInsertedActionText = undefined;
 
     progress.report({ message: 'starting\u2026' });
 
@@ -398,6 +420,7 @@ export class ReusedBrowser implements vscodeTypes.Disposable {
 
   private _resetExtensionState() {
     this._insertedEditActionCount = 0;
+    this._lastInsertedActionText = undefined;
     this._cancelRecording?.();
     this._cancelRecording = undefined;
   }

tests/codegen.spec.ts

@@ -17,6 +17,11 @@
 import { connectToSharedBrowser, expect, test, waitForPage } from './utils';
 import fs from 'node:fs';
 
+// The cross-session reset of the recorded actions landed in core:
+// https://github.com/microsoft/playwright/pull/42458, released with 1.63.0.
+const [pwMajor, pwMinor] = require('@playwright/test/package.json').version.split('.').map((p: string) => parseInt(p, 10));
+const hasCrossSessionReset = pwMajor > 1 || (pwMajor === 1 && pwMinor >= 63);
+
 test('should generate code', async ({ activate }) => {
   test.slow();
 
@@ -150,3 +155,156 @@ test('Record at Cursor should respect custom testId', async ({ activate, showBro
 
   vscode.lastWithProgressToken!.cancel();
 });
+
+test('should update recorded action when a signal is attached to it', async ({ activate, showBrowser }) => {
+  test.skip(!showBrowser);
+
+  const { vscode, testController } = await activate({
+    'playwright.config.js': `module.exports = {}`,
+    'tests/test.spec.ts': `
+      import { test } from '@playwright/test';
+      test('should pass', async ({ page }) => {
+        await page.setContent('<button data-testid="foo">click me</button>');
+
+      });
+    `,
+  });
+
+  await testController.expandTestItems(/test.spec/);
+  await expect(await testController.run()).toHaveOutput('1 passed');
+
+  await vscode.openEditors('**/test.spec.ts');
+  const editor = vscode.window.activeTextEditor;
+  expect(editor.document.uri.path).toContain('test.spec.ts');
+  editor.selection = new vscode.Selection(4, 0, 4, 0);
+
+  const webView = vscode.webViews.get('pw.extension.settingsView')!;
+  await webView.getByText('Record at cursor').click();
+  await expect.poll(() => vscode.lastWithProgressData, { timeout: 0 }).toEqual({ message: 'recording\u2026' });
+
+  const browser = await connectToSharedBrowser(vscode);
+  const page = await waitForPage(browser);
+  await page.getByRole('button', { name: 'click me' }).click();
+  await expect.poll(() => editor.edits.length).toBe(1);
+
+  // A dialog is shown and auto-dismissed by the recorder, which re-renders
+  // the last recorded action with a dialog signal. Since the user has not
+  // edited the file, the update should still land in the editor.
+  await page.evaluate('setTimeout(() => alert("hi"), 0)');
+  await expect.poll(() => ({
+    clicks: editor.document.text.match(/getByTestId\('foo'\)\.click\(\)/g)?.length,
+    hasDialogHandler: editor.document.text.includes("page.once('dialog'"),
+  })).toEqual({ clicks: 1, hasDialogHandler: true });
+
+  vscode.lastWithProgressToken!.cancel();
+});
+
+test('should not insert stale actions when editing file during recording', async ({ activate, showBrowser }) => {
+  test.skip(!showBrowser);
+
+  const { vscode, testController } = await activate({
+    'playwright.config.js': `module.exports = {}`,
+    'tests/test.spec.ts': `
+      import { test } from '@playwright/test';
+      test('should pass', async ({ page }) => {
+        await page.setContent('<button data-testid="foo">click me</button>');
+
+      });
+    `,
+  });
+
+  await testController.expandTestItems(/test.spec/);
+  await expect(await testController.run()).toHaveOutput('1 passed');
+
+  await vscode.openEditors('**/test.spec.ts');
+  const editor = vscode.window.activeTextEditor;
+  expect(editor.document.uri.path).toContain('test.spec.ts');
+  editor.selection = new vscode.Selection(4, 0, 4, 0);
+
+  const webView = vscode.webViews.get('pw.extension.settingsView')!;
+  await webView.getByText('Record at cursor').click();
+  await expect.poll(() => vscode.lastWithProgressData, { timeout: 0 }).toEqual({ message: 'recording\u2026' });
+
+  const browser = await connectToSharedBrowser(vscode);
+  const page = await waitForPage(browser);
+  await page.getByRole('button', { name: 'click me' }).click();
+  // Wait for the insert to settle: the document contains the action and the
+  // selection spans it, i.e. the extension is done updating the editor.
+  await expect.poll(() => ({
+    clicks: editor.document.text.match(/getByTestId\('foo'\)\.click\(\)/g)?.length,
+    selectionSpansAction: !editor.selection.isEmpty,
+  })).toEqual({ clicks: 1, selectionSpansAction: true });
+
+  // The user edits the file while recording, moving the cursor away
+  // from the recorded action.
+  editor.document.lines.splice(4, 0, '      // tidy up');
+  editor.selection = new vscode.Selection(4, 0, 4, 0);
+
+  // A dialog is shown and auto-dismissed by the recorder, which re-renders
+  // the last recorded action with a dialog signal. It should not land at
+  // the user's cursor.
+  await page.evaluate('setTimeout(() => alert("hi"), 0)');
+
+  // One more action is recorded. The stale re-render must not have landed at
+  // the user's cursor: only the two real clicks are in the document and the
+  // dialog handler wrapper was never inserted.
+  await page.getByRole('button', { name: 'click me' }).click();
+  await expect.poll(() => ({
+    clicks: editor.document.text.match(/getByTestId\('foo'\)\.click\(\)/g)?.length,
+    hasDialogHandler: editor.document.text.includes("page.once('dialog'"),
+  })).toEqual({ clicks: 2, hasDialogHandler: false });
+
+  vscode.lastWithProgressToken!.cancel();
+});
+
+test('should not insert stale actions from the previous recording session', async ({ activate, showBrowser }) => {
+  test.skip(!showBrowser);
+  // Requires the core-side reset of the recorded actions between sessions.
+  test.skip(!hasCrossSessionReset, 'needs playwright with https://github.com/microsoft/playwright/pull/42458');
+
+  const { vscode, testController } = await activate({
+    'playwright.config.js': `module.exports = {}`,
+    'tests/test.spec.ts': `
+      import { test } from '@playwright/test';
+      test('should pass', async ({ page }) => {
+        await page.setContent('<button data-testid="foo">click me</button>');
+
+      });
+    `,
+  });
+
+  await testController.expandTestItems(/test.spec/);
+  await expect(await testController.run()).toHaveOutput('1 passed');
+
+  await vscode.openEditors('**/test.spec.ts');
+  const editor = vscode.window.activeTextEditor;
+  expect(editor.document.uri.path).toContain('test.spec.ts');
+
+  const webView = vscode.webViews.get('pw.extension.settingsView')!;
+  const browser = await connectToSharedBrowser(vscode);
+  const page = await waitForPage(browser);
+
+  // Session 1: record a click at the cursor.
+  editor.selection = new vscode.Selection(4, 0, 4, 0);
+  await webView.getByText('Record at cursor').click();
+  await expect.poll(() => vscode.lastWithProgressData, { timeout: 0 }).toEqual({ message: 'recording\u2026' });
+  await page.getByRole('button', { name: 'click me' }).click();
+  await expect.poll(() => editor.document.text.match(/getByTestId\('foo'\)\.click\(\)/g)?.length).toBe(1);
+
+  // Stop recording, then start a new session on the same page, like the
+  // user toggling "Record at cursor" off and on after moving the cursor.
+  vscode.lastWithProgressToken!.cancel();
+  editor.selection = new vscode.Selection(6, 0, 6, 0);
+  await webView.getByText('Record at cursor').click();
+  await expect.poll(() => vscode.lastWithProgressData, { timeout: 0 }).toEqual({ message: 'recording\u2026' });
+
+  // A late signal (a dialog auto-dismissed by the recorder) used to re-render
+  // the stale last action of the previous session and insert it at the new
+  // cursor position. The new session must start from a clean action list.
+  await page.evaluate('setTimeout(() => alert("hi"), 0)');
+  await page.waitForTimeout(1000);
+  expect(editor.document.text.match(/getByTestId\('foo'\)\.click\(\)/g)?.length).toBe(1);
+  expect(editor.document.text.includes("page.once('dialog'")).toBe(false);
+
+  vscode.lastWithProgressToken!.cancel();
+});