chore: move auto-generated expect computation to injected code (microsoft#36894)

Author: dgozman

packages/injected/src/ariaSnapshot.ts

@@ -77,7 +77,7 @@ function toInternalOptions(options: AriaTreeOptions): InternalOptions {
   }
   if (options.mode === 'autoexpect') {
     // To auto-generate assertions on visible elements.
-    return { visibility: 'ariaAndVisible', refs: 'all' };
+    return { visibility: 'ariaAndVisible', refs: 'none' };
   }
   if (options.mode === 'codegen') {
     // To generate aria assertion with regex heurisitcs.

packages/injected/src/injectedScript.ts

@@ -107,6 +107,7 @@ export class InjectedScript {
     isInsideScope,
     normalizeWhiteSpace,
     parseAriaSnapshot,
+    generateAriaTree,
     // Builtins protect injected code from clock emulation.
     builtins: null as unknown as Builtins,
   };

packages/injected/src/recorder/recorder.ts

@@ -23,6 +23,7 @@ import type { ElementText } from '../selectorUtils';
 import type * as actions from '@recorder/actions';
 import type { ElementInfo, Mode, OverlayState, UIState } from '@recorder/recorderTypes';
 import type { Language } from '@isomorphic/locatorGenerators';
+import type { AriaNode, AriaSnapshot } from '@injected/ariaSnapshot';
 
 const HighlightColors = {
   multiple: '#f6b26b7f',
@@ -543,21 +544,13 @@ class RecordActionTool implements RecorderTool {
       consumeEvent(event);
   }
 
-  private _captureAriaSnapshotForAction(action: actions.Action) {
-    const documentElement = this._recorder.injectedScript.document.documentElement;
-    if (documentElement)
-      action.ariaSnapshot = this._recorder.injectedScript.ariaSnapshot(documentElement, { mode: 'autoexpect' });
-  }
-
   private _recordAction(action: actions.Action) {
-    this._captureAriaSnapshotForAction(action);
     this._recorder.recordAction(action);
   }
 
   private _performAction(action: actions.PerformOnRecordAction) {
     this._recorder.updateHighlight(null, false);
 
-    this._captureAriaSnapshotForAction(action);
     this._performingActions.add(action);
 
     const promise = this._recorder.performAction(action).then(() => {
@@ -1267,6 +1260,7 @@ export class Recorder {
   private _tools: Record<Mode, RecorderTool>;
   private _lastHighlightedSelector: string | undefined = undefined;
   private _lastHighlightedAriaTemplateJSON: string = 'undefined';
+  private _lastActionAutoexpectSnapshot: AriaSnapshot | undefined;
   readonly highlight: Highlight;
   readonly overlay: Overlay | undefined;
   private _stylesheet: CSSStyleSheet;
@@ -1585,11 +1579,25 @@ export class Recorder {
     void this._delegate.setMode?.(mode);
   }
 
+  private _captureAutoExpectSnapshot() {
+    const documentElement = this.injectedScript.document.documentElement;
+    return documentElement ? this.injectedScript.utils.generateAriaTree(documentElement, { mode: 'autoexpect' }) : undefined;
+  }
+
   async performAction(action: actions.PerformOnRecordAction) {
+    const previousSnapshot = this._lastActionAutoexpectSnapshot;
+    this._lastActionAutoexpectSnapshot = this._captureAutoExpectSnapshot();
+    if (!isAssertAction(action) && this._lastActionAutoexpectSnapshot) {
+      const element = findNewElement(previousSnapshot, this._lastActionAutoexpectSnapshot);
+      action.preconditionSelector = element ? this.injectedScript.generateSelector(element, { testIdAttributeName: this.state.testIdAttributeName }).selector : undefined;
+      if (action.preconditionSelector === action.selector)
+        action.preconditionSelector = undefined;
+    }
     await this._delegate.performAction?.(action).catch(() => {});
   }
 
   recordAction(action: actions.Action) {
+    this._lastActionAutoexpectSnapshot = this._captureAutoExpectSnapshot();
     void this._delegate.recordAction?.(action);
   }
 
@@ -1794,3 +1802,56 @@ function createSvgElement(doc: Document, { tagName, attrs, children }: SvgJson):
 
   return elem;
 }
+
+function isAssertAction(action: actions.Action): action is actions.AssertAction {
+  return action.name.startsWith('assert');
+}
+
+function findNewElement(from: AriaSnapshot | undefined, to: AriaSnapshot): Element | undefined {
+  type ByRoleAndName = Map<string, Map<string, { node: AriaNode, sizeAndPosition: number }>>;
+
+  function fillMap(root: AriaNode, map: ByRoleAndName, position: number) {
+    let size = 1;
+    let childPosition = position + size;
+    for (const child of root.children || []) {
+      if (typeof child === 'string') {
+        size++;
+        childPosition++;
+      } else {
+        size += fillMap(child, map, childPosition);
+        childPosition += size;
+      }
+    }
+    if (!['none', 'presentation', 'fragment', 'iframe', 'generic'].includes(root.role) && root.name) {
+      let byRole = map.get(root.role);
+      if (!byRole) {
+        byRole = new Map();
+        map.set(root.role, byRole);
+      }
+      const existing = byRole.get(root.name);
+      // This heuristic prioritizes elements at the top of the page, even if somewhat smaller.
+      const sizeAndPosition = size * 100 - position;
+      if (!existing || existing.sizeAndPosition < sizeAndPosition)
+        byRole.set(root.name, { node: root, sizeAndPosition });
+    }
+    return size;
+  }
+
+  const fromMap: ByRoleAndName = new Map();
+  if (from)
+    fillMap(from.root, fromMap, 0);
+
+  const toMap: ByRoleAndName = new Map();
+  fillMap(to.root, toMap, 0);
+
+  const result: { node: AriaNode, sizeAndPosition: number }[] = [];
+  for (const [role, byRole] of toMap) {
+    for (const [name, byName] of byRole) {
+      const inFrom = fromMap.get(role)?.get(name);
+      if (!inFrom)
+        result.push(byName);
+    }
+  }
+  result.sort((a, b) => b.sizeAndPosition - a.sizeAndPosition);
+  return result[0]?.node.element;
+}

packages/playwright-core/src/server/codegen/language.ts

@@ -22,11 +22,33 @@ import type * as actions from '@recorder/actions';
 export function generateCode(actions: actions.ActionInContext[], languageGenerator: LanguageGenerator, options: LanguageGeneratorOptions) {
   const header = languageGenerator.generateHeader(options);
   const footer = languageGenerator.generateFooter(options.saveStorage);
-  const actionTexts = actions.map(a => languageGenerator.generateAction(a)).filter(Boolean);
+  const actionTexts = actions.map(a => generateActionText(languageGenerator, a)).filter(Boolean) as string[];
   const text = [header, ...actionTexts, footer].join('\n');
   return { header, footer, actionTexts, text };
 }
 
+function generateActionText(generator: LanguageGenerator, action: actions.ActionInContext): string | undefined {
+  let text = generator.generateAction(action);
+  if (!text)
+    return;
+  if (action.action.preconditionSelector) {
+    const expectAction: actions.ActionInContext = {
+      frame: action.frame,
+      startTime: action.startTime,
+      endTime: action.startTime,
+      action: {
+        name: 'assertVisible',
+        selector: action.action.preconditionSelector,
+        signals: [],
+      },
+    };
+    const expectText = generator.generateAction(expectAction);
+    if (expectText)
+      text = expectText + '\n' + text;
+  }
+  return text;
+}
+
 export function sanitizeDeviceOptions(device: any, options: BrowserContextOptions): BrowserContextOptions {
   // Filter out all the properties from the device descriptor.
   const cleanedOptions: Record<string, any> = {};

packages/playwright-core/src/server/recorder.ts

@@ -20,7 +20,7 @@ import fs from 'fs';
 import { isUnderTest } from '../utils';
 import { BrowserContext } from './browserContext';
 import { Debugger } from './debugger';
-import { buildFullSelector, generateFrameSelector, isAssertAction, metadataToCallLog, shouldMergeAction } from './recorder/recorderUtils';
+import { buildFullSelector, generateFrameSelector, metadataToCallLog } from './recorder/recorderUtils';
 import { locatorOrSelectorAsSelector } from '../utils/isomorphic/locatorParser';
 import { stringifySelector } from '../utils/isomorphic/selectorParser';
 import { ProgressController } from './progress';
@@ -31,8 +31,6 @@ import { eventsHelper, monotonicTime } from './../utils';
 import { Frame } from './frames';
 import { Page } from './page';
 import { performAction } from './recorder/recorderRunner';
-import { findNewElementRef } from '../utils/isomorphic/ariaSnapshot';
-import { yaml } from '../utilsBundle';
 
 import type { Language } from './codegen/types';
 import type { CallMetadata, InstrumentationListener, SdkObject } from './instrumentation';
@@ -522,60 +520,9 @@ export class Recorder extends EventEmitter<RecorderEventMap> implements Instrume
     return actionInContext;
   }
 
-  private async _maybeGenerateAssertAction(frame: Frame, actionInContext: actions.ActionInContext) {
-    const lastAction = getLastFrameAction(frame);
-    if (isAssertAction(actionInContext.action))
-      return;
-    if (lastAction && (isAssertAction(lastAction.action) || shouldMergeAction(actionInContext, lastAction)))
-      return;
-    const newSnapshot = actionInContext.action.ariaSnapshot;
-    if (!newSnapshot)
-      return;
-    const lastSnapshot = lastAction?.action.ariaSnapshot || `- document [ref=e1]\n`;
-    if (!lastSnapshot)
-      return;
-    const callMetadata = serverSideCallMetadata();
-    const controller = new ProgressController(callMetadata, frame);
-    const selector = await controller.run(async progress => {
-      const ref = findNewElementRef(yaml, lastSnapshot, newSnapshot);
-      if (!ref)
-        return;
-      // Note: recorder runs in the main world, so we need to resolve the ref in the main world.
-      // Note: resolveSelector always returns a |page|-based selector, not |frame|-based.
-      const { resolvedSelector } = await frame.resolveSelector(progress, `aria-ref=${ref}`, { mainWorld: true }).catch(() => ({ resolvedSelector: undefined }));
-      if (!resolvedSelector)
-        return;
-      const isVisible = await frame._page.mainFrame().isVisible(progress, resolvedSelector, { strict: true }).catch(() => false);
-      return isVisible ? resolvedSelector : undefined;
-    }).catch(() => undefined);
-    if (!selector)
-      return;
-    if (!actionInContext.frame.framePath.length && 'selector' in actionInContext.action && actionInContext.action.selector === selector)
-      return;  // Do not assert the action target, auto-waiting takes care of it.
-    const assertActionInContext: actions.ActionInContext = {
-      frame: {
-        pageGuid: actionInContext.frame.pageGuid,
-        pageAlias: actionInContext.frame.pageAlias,
-        framePath: [],
-      },
-      action: {
-        name: 'assertVisible',
-        selector,
-        signals: [],
-      },
-      startTime: actionInContext.startTime,
-      endTime: actionInContext.startTime,
-    };
-    return assertActionInContext;
-  }
-
   private async _performAction(frame: Frame, action: actions.PerformOnRecordAction) {
     const actionInContext = await this._createActionInContext(frame, action);
-    const assertActionInContext = await this._maybeGenerateAssertAction(frame, actionInContext);
-    if (assertActionInContext)
-      this._signalProcessor.addAction(assertActionInContext);
     this._signalProcessor.addAction(actionInContext);
-    setLastFrameAction(frame, actionInContext);
     if (actionInContext.action.name !== 'openPage' && actionInContext.action.name !== 'closePage')
       await performAction(this._pageAliases, actionInContext);
     actionInContext.endTime = monotonicTime();
@@ -584,7 +531,6 @@ export class Recorder extends EventEmitter<RecorderEventMap> implements Instrume
   private async _recordAction(frame: Frame, action: actions.Action) {
     const actionInContext = await this._createActionInContext(frame, action);
     this._signalProcessor.addAction(actionInContext);
-    setLastFrameAction(frame, actionInContext);
   }
 
   private _onFrameNavigated(frame: Frame, page: Page) {
@@ -624,11 +570,3 @@ function languageForFile(file: string) {
     return 'csharp';
   return 'javascript';
 }
-
-const kLastFrameActionSymbol = Symbol('lastFrameAction');
-function getLastFrameAction(frame: Frame): actions.ActionInContext | undefined {
-  return (frame as any)[kLastFrameActionSymbol];
-}
-function setLastFrameAction(frame: Frame, action: actions.ActionInContext | undefined) {
-  (frame as any)[kLastFrameActionSymbol] = action;
-}

packages/playwright-core/src/server/recorder/recorderUtils.ts

@@ -73,10 +73,6 @@ export async function frameForAction(pageAliases: Map<Page, string>, actionInCon
   return result.frame;
 }
 
-export function isAssertAction(action: actions.Action): action is actions.AssertAction {
-  return action.name.startsWith('assert');
-}
-
 function isSameAction(a: actions.ActionInContext, b: actions.ActionInContext): boolean {
   return a.action.name === b.action.name && a.frame.pageAlias === b.frame.pageAlias && a.frame.framePath.join('|') === b.frame.framePath.join('|');
 }

packages/playwright-core/src/utils/isomorphic/ariaSnapshot.ts

@@ -32,7 +32,6 @@ export type AriaProps = {
   level?: number;
   pressed?: boolean | 'mixed';
   selected?: boolean;
-  ref?: string;
 };
 
 // We pass parsed template between worlds using JSON, make it easy.
@@ -65,7 +64,7 @@ type YamlLibrary = {
 };
 
 type ParsedYamlPosition = { line: number; col: number; };
-type ParsingOptions = yamlTypes.ParseOptions & { allowRef?: boolean, allowUnknownAttributes?: boolean };
+type ParsingOptions = yamlTypes.ParseOptions;
 
 export type ParsedYamlError = {
   message: string;
@@ -434,10 +433,6 @@ export class KeyParser {
   }
 
   private _applyAttribute(node: AriaTemplateRoleNode, key: string, value: string, errorPos: number) {
-    if (this._options.allowRef && key === 'ref') {
-      node.ref = value;
-      return;
-    }
     if (key === 'checked') {
       this._assert(value === 'true' || value === 'false' || value === 'mixed', 'Value of "checked\" attribute must be a boolean or "mixed"', errorPos);
       node.checked = value === 'true' ? true : value === 'false' ? false : 'mixed';
@@ -473,8 +468,6 @@ export class KeyParser {
       node.selected = value === 'true';
       return;
     }
-    if (this._options.allowUnknownAttributes)
-      return;
     this._assert(false, `Unsupported attribute [${key}]`, errorPos);
   }
 
@@ -492,55 +485,3 @@ export class ParserError extends Error {
     this.pos = pos;
   }
 }
-
-export function findNewElementRef(yaml: YamlLibrary, fromSnapshot: string, toSnapshot: string): string | undefined {
-  type ByRoleAndName = Map<string, Map<string, { node: AriaTemplateRoleNode, sizeAndPosition: number }>>;
-
-  function fillMap(root: AriaTemplateRoleNode, map: ByRoleAndName, position: number) {
-    let size = 1;
-    let childPosition = position + size;
-    for (const child of root.children || []) {
-      if (child.kind === 'role') {
-        size += fillMap(child, map, childPosition);
-        childPosition += size;
-      } else {
-        size++;
-        childPosition++;
-      }
-    }
-    if (!['none', 'presentation', 'fragment', 'iframe', 'generic'].includes(root.role) && typeof root.name === 'string' && root.name) {
-      let byRole = map.get(root.role);
-      if (!byRole) {
-        byRole = new Map();
-        map.set(root.role, byRole);
-      }
-      const existing = byRole.get(root.name);
-      // This heuristic prioritizes elements at the top of the page, even if somewhat smaller.
-      const sizeAndPosition = size * 100 - position;
-      if (!existing || existing.sizeAndPosition < sizeAndPosition)
-        byRole.set(root.name, { node: root, sizeAndPosition });
-    }
-    return size;
-  }
-
-  const fromMap: ByRoleAndName = new Map();
-  const from = parseAriaSnapshotUnsafe(yaml, fromSnapshot, { allowRef: true, allowUnknownAttributes: true });
-  if (from.kind === 'role')
-    fillMap(from, fromMap, 0);
-
-  const toMap: ByRoleAndName = new Map();
-  const to = parseAriaSnapshotUnsafe(yaml, toSnapshot, { allowRef: true, allowUnknownAttributes: true });
-  if (to.kind === 'role')
-    fillMap(to, toMap, 0);
-
-  const result: { node: AriaTemplateRoleNode, sizeAndPosition: number }[] = [];
-  for (const [role, byRole] of toMap) {
-    for (const [name, byName] of byRole) {
-      const inFrom = fromMap.get(role)?.get(name);
-      if (!inFrom)
-        result.push(byName);
-    }
-  }
-  result.sort((a, b) => b.sizeAndPosition - a.sizeAndPosition);
-  return result.find(r => r.node.ref)?.node.ref;
-}

packages/recorder/src/actions.d.ts

@@ -37,6 +37,7 @@ export type ActionBase = {
   name: ActionName,
   signals: Signal[],
   ariaSnapshot?: string,
+  preconditionSelector?: string,
 };
 
 export type ActionWithSelector = ActionBase & {