feat(codegen): prefer frame name over url when unique (#5175)

This commit is contained in:
Dmitry Gozman 2021-01-27 13:19:36 -08:00 committed by GitHub
parent 35baf335d8
commit 527286683f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 85 additions and 21 deletions

View file

@ -18,10 +18,13 @@ import type { BrowserContextOptions, LaunchOptions } from '../../../..';
import { Frame } from '../../frames';
import { LanguageGenerator } from './language';
import { Action, Signal } from './recorderActions';
import { describeFrame } from './utils';
export type ActionInContext = {
pageAlias: string;
frame: Frame;
frameName?: string;
frameUrl: string;
isMainFrame: boolean;
action: Action;
committed?: boolean;
}
@ -124,7 +127,7 @@ export class CodeGenerator {
if (signal.name === 'navigation') {
this.addAction({
pageAlias,
frame,
...describeFrame(frame),
committed: true,
action: {
name: 'navigate',

View file

@ -24,7 +24,7 @@ import deviceDescriptors = require('../../deviceDescriptors');
export class CSharpLanguageGenerator implements LanguageGenerator {
generateAction(actionInContext: ActionInContext, performingAction: boolean): string {
const { action, pageAlias, frame } = actionInContext;
const { action, pageAlias } = actionInContext;
const formatter = new CSharpFormatter(0);
formatter.newLine();
formatter.add('// ' + actionTitle(action));
@ -36,8 +36,10 @@ export class CSharpLanguageGenerator implements LanguageGenerator {
return formatter.format();
}
const subject = !frame.parentFrame() ? pageAlias :
`${pageAlias}.GetFrame(url: '${frame.url()}')`;
const subject = actionInContext.isMainFrame ? pageAlias :
(actionInContext.frameName ?
`${pageAlias}.GetFrame(name: ${quote(actionInContext.frameName)})` :
`${pageAlias}.GetFrame(url: ${quote(actionInContext.frameUrl)})`);
let navigationSignal: NavigationSignal | undefined;
let popupSignal: PopupSignal | undefined;

View file

@ -24,7 +24,7 @@ import deviceDescriptors = require('../../deviceDescriptors');
export class JavaScriptLanguageGenerator implements LanguageGenerator {
generateAction(actionInContext: ActionInContext, performingAction: boolean): string {
const { action, pageAlias, frame } = actionInContext;
const { action, pageAlias } = actionInContext;
const formatter = new JavaScriptFormatter(2);
formatter.newLine();
formatter.add('// ' + actionTitle(action));
@ -36,8 +36,10 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator {
return formatter.format();
}
const subject = !frame.parentFrame() ? pageAlias :
`${pageAlias}.frame(${formatObject({ url: frame.url() })})`;
const subject = actionInContext.isMainFrame ? pageAlias :
(actionInContext.frameName ?
`${pageAlias}.frame(${formatObject({ name: actionInContext.frameName })})` :
`${pageAlias}.frame(${formatObject({ url: actionInContext.frameUrl })})`);
let navigationSignal: NavigationSignal | undefined;
let popupSignal: PopupSignal | undefined;

View file

@ -33,7 +33,7 @@ export class PythonLanguageGenerator implements LanguageGenerator {
}
generateAction(actionInContext: ActionInContext, performingAction: boolean): string {
const { action, pageAlias, frame } = actionInContext;
const { action, pageAlias } = actionInContext;
const formatter = new PythonFormatter(4);
formatter.newLine();
formatter.add('# ' + actionTitle(action));
@ -45,8 +45,10 @@ export class PythonLanguageGenerator implements LanguageGenerator {
return formatter.format();
}
const subject = !frame.parentFrame() ? pageAlias :
`${pageAlias}.frame(${formatOptions({ url: frame.url() }, false)})`;
const subject = actionInContext.isMainFrame ? pageAlias :
(actionInContext.frameName ?
`${pageAlias}.frame(${formatOptions({ name: actionInContext.frameName }, false)})` :
`${pageAlias}.frame(${formatOptions({ url: actionInContext.frameUrl }, false)})`);
let navigationSignal: NavigationSignal | undefined;
let popupSignal: PopupSignal | undefined;

View file

@ -46,3 +46,13 @@ export function toModifiers(modifiers: number): ('Alt' | 'Control' | 'Meta' | 'S
result.push('Shift');
return result;
}
export function describeFrame(frame: Frame): { frameName?: string, frameUrl: string, isMainFrame: boolean } {
const page = frame._page;
if (page.mainFrame() === frame)
return { isMainFrame: true, frameUrl: frame.url() };
const frames = page.frames().filter(f => f.name() === frame.name());
if (frames.length === 1 && frames[0] === frame)
return { isMainFrame: false, frameUrl: frame.url(), frameName: frame.name() };
return { isMainFrame: false, frameUrl: frame.url() };
}

View file

@ -17,7 +17,7 @@
import * as actions from './recorder/recorderActions';
import type * as channels from '../../protocol/channels';
import { CodeGenerator, ActionInContext } from './recorder/codeGenerator';
import { toClickOptions, toModifiers } from './recorder/utils';
import { describeFrame, toClickOptions, toModifiers } from './recorder/utils';
import { Page } from '../page';
import { Frame } from '../frames';
import { BrowserContext } from '../browserContext';
@ -151,7 +151,7 @@ export class RecorderSupplement {
this._pageAliases.delete(page);
this._generator.addAction({
pageAlias,
frame: page.mainFrame(),
...describeFrame(page.mainFrame()),
committed: true,
action: {
name: 'closePage',
@ -174,7 +174,7 @@ export class RecorderSupplement {
if (!isPopup) {
this._generator.addAction({
pageAlias,
frame: page.mainFrame(),
...describeFrame(page.mainFrame()),
committed: true,
action: {
name: 'openPage',
@ -190,7 +190,7 @@ export class RecorderSupplement {
const controller = new ProgressController();
const actionInContext: ActionInContext = {
pageAlias: this._pageAliases.get(page)!,
frame,
...describeFrame(frame),
action
};
this._generator.willPerformAction(actionInContext);
@ -218,10 +218,9 @@ export class RecorderSupplement {
}
private async _recordAction(frame: Frame, action: actions.Action) {
// We are lacking frame.page() in
this._generator.addAction({
pageAlias: this._pageAliases.get(frame._page)!,
frame,
...describeFrame(frame),
action
});
}

View file

@ -597,4 +597,45 @@ describe('cli codegen', (test, { browserName, headful }) => {
page.click('input[id=checkbox]')
]);
});
it('should prefer frame name', async ({ page, recorder, server }) => {
await recorder.setContentAndWait(`
<iframe src='./frames/frame.html' name='one'></iframe>
<iframe src='./frames/frame.html' name='two'></iframe>
<iframe src='./frames/frame.html'></iframe>
`, server.EMPTY_PAGE, 4);
const frameOne = page.frame({ name: 'one' });
const frameTwo = page.frame({ name: 'two' });
const otherFrame = page.frames().find(f => f !== page.mainFrame() && !f.name());
await Promise.all([
recorder.waitForOutput('one'),
frameOne.click('div'),
]);
expect(recorder.output()).toContain(`
// Click text="Hi, I'm frame"
await page.frame({
name: 'one'
}).click('text="Hi, I\\'m frame"');`);
await Promise.all([
recorder.waitForOutput('two'),
frameTwo.click('div'),
]);
expect(recorder.output()).toContain(`
// Click text="Hi, I'm frame"
await page.frame({
name: 'two'
}).click('text="Hi, I\\'m frame"');`);
await Promise.all([
recorder.waitForOutput('url: \''),
otherFrame.click('div'),
]);
expect(recorder.output()).toContain(`
// Click text="Hi, I'm frame"
await page.frame({
url: '${otherFrame.url()}'
}).click('text="Hi, I\\'m frame"');`);
});
});

View file

@ -127,15 +127,20 @@ class Recorder {
this._actionPerformedCallback = () => { };
}
async setContentAndWait(content: string, url: string = 'about:blank') {
await this.setPageContentAndWait(this.page, content, url);
async setContentAndWait(content: string, url: string = 'about:blank', frameCount: number = 1) {
await this.setPageContentAndWait(this.page, content, url, frameCount);
}
async setPageContentAndWait(page: Page, content: string, url: string = 'about:blank') {
async setPageContentAndWait(page: Page, content: string, url: string = 'about:blank', frameCount: number = 1) {
let callback;
const result = new Promise(f => callback = f);
await page.goto(url);
await page.exposeBinding('_recorderScriptReadyForTest', (source, arg) => callback(arg));
const frames = new Set<any>();
await page.exposeBinding('_recorderScriptReadyForTest', (source, arg) => {
frames.add(source.frame);
if (frames.size === frameCount)
callback(arg);
});
await Promise.all([
result,
page.setContent(content)