Skip to content

Commit 35cb20d

Browse files
dgozmandgozman
authored
test: unflake recorder tests (#2808)
We ensure that recorder is installed in the main frame before running the test.
1 parent baaa654 commit 35cb20d

File tree

4 files changed

+82
-42
lines changed

4 files changed

+82
-42
lines changed

‎src/debug/debugController.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,37 @@ import { Writable } from 'stream';
1818
import { BrowserContextBase } from '../browserContext';
1919
import { Events } from '../events';
2020
import * as frames from '../frames';
21+
import * as js from '../javascript';
2122
import { Page } from '../page';
2223
import { RecorderController } from './recorderController';
24+
import DebugScript from './injected/debugScript';
2325

2426
export class DebugController {
27+
private _options: { recorderOutput?: Writable | undefined };
28+
2529
constructor(context: BrowserContextBase, options: { recorderOutput?: Writable | undefined }) {
26-
const installInFrame = async (frame: frames.Frame) => {
27-
try {
28-
const mainContext = await frame._mainContext();
29-
await mainContext.createDebugScript({ console: true, record: !!options.recorderOutput });
30-
} catch (e) {
31-
}
32-
};
30+
this._options = options;
3331

3432
if (options.recorderOutput)
3533
new RecorderController(context, options.recorderOutput);
3634

3735
context.on(Events.BrowserContext.Page, (page: Page) => {
3836
for (const frame of page.frames())
39-
installInFrame(frame);
40-
page.on(Events.Page.FrameNavigated, installInFrame);
37+
this.ensureInstalledInFrame(frame);
38+
page.on(Events.Page.FrameNavigated, frame => this.ensureInstalledInFrame(frame));
4139
});
4240
}
41+
42+
private async ensureInstalledInFrame(frame: frames.Frame): Promise<js.JSHandle<DebugScript> | undefined> {
43+
try {
44+
const mainContext = await frame._mainContext();
45+
return await mainContext.createDebugScript({ console: true, record: !!this._options.recorderOutput });
46+
} catch (e) {
47+
}
48+
}
49+
50+
async ensureInstalledInFrameForTest(frame: frames.Frame): Promise<void> {
51+
const handle = await this.ensureInstalledInFrame(frame);
52+
await handle!.evaluate(debugScript => debugScript.recorder!.refreshListeners());
53+
}
4354
}

‎src/debug/injected/debugScript.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@ export default class DebugScript {
2222
consoleAPI: ConsoleAPI | undefined;
2323
recorder: Recorder | undefined;
2424

25-
constructor() {
26-
}
27-
2825
initialize(injectedScript: InjectedScript, options: { console?: boolean, record?: boolean }) {
2926
if (options.console)
3027
this.consoleAPI = new ConsoleAPI(injectedScript);

‎src/debug/injected/recorder.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,27 @@ declare global {
2828
export class Recorder {
2929
private _injectedScript: InjectedScript;
3030
private _performingAction = false;
31+
readonly refreshListeners: () => void;
3132

3233
constructor(injectedScript: InjectedScript) {
3334
this._injectedScript = injectedScript;
3435

35-
document.addEventListener('click', event => this._onClick(event), true);
36-
document.addEventListener('input', event => this._onInput(event), true);
37-
document.addEventListener('keydown', event => this._onKeyDown(event), true);
36+
const onClick = this._onClick.bind(this);
37+
const onInput = this._onInput.bind(this);
38+
const onKeyDown = this._onKeyDown.bind(this);
39+
this.refreshListeners = () => {
40+
document.removeEventListener('click', onClick, true);
41+
document.removeEventListener('input', onInput, true);
42+
document.removeEventListener('keydown', onKeyDown, true);
43+
document.addEventListener('click', onClick, true);
44+
document.addEventListener('input', onInput, true);
45+
document.addEventListener('keydown', onKeyDown, true);
46+
};
47+
this.refreshListeners();
48+
// Document listeners are cleared upon document.open,
49+
// so we refresh them periodically in a best-effort manner.
50+
// Note: keep in sync with the same constant in the test.
51+
setInterval(this.refreshListeners, 1000);
3852
}
3953

4054
private async _onClick(event: MouseEvent) {

‎test/recorder.spec.js

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
const {USES_HOOKS} = require('./utils').testOptions(browserType);
17+
const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = require('./utils').testOptions(browserType);
1818

1919
class WritableBuffer {
2020
constructor() {
@@ -56,16 +56,41 @@ describe.skip(USES_HOOKS)('Recorder', function() {
5656
state.context = await state.browser.newContext();
5757
state.output = new WritableBuffer();
5858
const debugController = state.context._initDebugModeForTest({ recorderOutput: state.output });
59+
state.page = await state.context.newPage();
60+
state.setContent = async (content) => {
61+
await state.page.setContent(content);
62+
await debugController.ensureInstalledInFrameForTest(state.page.mainFrame());
63+
};
5964
});
60-
65+
6166
afterEach(async state => {
6267
await state.context.close();
6368
});
64-
65-
it('should click', async function({context, output, server}) {
66-
const page = await context.newPage();
69+
70+
it('should click', async function({page, output, setContent, server}) {
71+
await page.goto(server.EMPTY_PAGE);
72+
await setContent(`<button onclick="console.log('click')">Submit</button>`);
73+
const [message] = await Promise.all([
74+
page.waitForEvent('console'),
75+
output.waitFor('click'),
76+
page.dispatchEvent('button', 'click', { detail: 1 })
77+
]);
78+
expect(output.text()).toContain(`
79+
// Click text="Submit"
80+
await page.click('text="Submit"');`);
81+
expect(message.text()).toBe('click');
82+
});
83+
84+
it('should click after document.open', async function({page, output, setContent, server}) {
6785
await page.goto(server.EMPTY_PAGE);
68-
await page.setContent(`<button onclick="console.log('click')">Submit</button>`);
86+
await setContent(``);
87+
await page.evaluate(() => {
88+
document.open();
89+
document.write(`<button onclick="console.log('click')">Submit</button>`);
90+
document.close();
91+
// Give it time to refresh. See Recorder for details.
92+
return new Promise(f => setTimeout(f, 1000));
93+
});
6994
const [message] = await Promise.all([
7095
page.waitForEvent('console'),
7196
output.waitFor('click'),
@@ -77,10 +102,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
77102
expect(message.text()).toBe('click');
78103
});
79104

80-
it('should fill', async function({context, output, server}) {
81-
const page = await context.newPage();
105+
it('should fill', async function({page, output, setContent, server}) {
82106
await page.goto(server.EMPTY_PAGE);
83-
await page.setContent(`<input id="input" name="name" oninput="console.log(input.value)"></input>`);
107+
await setContent(`<input id="input" name="name" oninput="console.log(input.value)"></input>`);
84108
const [message] = await Promise.all([
85109
page.waitForEvent('console'),
86110
output.waitFor('fill'),
@@ -92,10 +116,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
92116
expect(message.text()).toBe('John');
93117
});
94118

95-
it('should press', async function({context, output, server}) {
96-
const page = await context.newPage();
119+
it('should press', async function({page, output, setContent, server}) {
97120
await page.goto(server.EMPTY_PAGE);
98-
await page.setContent(`<input name="name" onkeypress="console.log('press')"></input>`);
121+
await setContent(`<input name="name" onkeypress="console.log('press')"></input>`);
99122
const [message] = await Promise.all([
100123
page.waitForEvent('console'),
101124
output.waitFor('press'),
@@ -107,10 +130,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
107130
expect(message.text()).toBe('press');
108131
});
109132

110-
it('should check', async function({context, output, server}) {
111-
const page = await context.newPage();
133+
it('should check', async function({page, output, setContent, server}) {
112134
await page.goto(server.EMPTY_PAGE);
113-
await page.setContent(`<input id="checkbox" type="checkbox" name="accept" onchange="console.log(checkbox.checked)"></input>`);
135+
await setContent(`<input id="checkbox" type="checkbox" name="accept" onchange="console.log(checkbox.checked)"></input>`);
114136
const [message] = await Promise.all([
115137
page.waitForEvent('console'),
116138
output.waitFor('check'),
@@ -123,10 +145,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
123145
expect(message.text()).toBe("true");
124146
});
125147

126-
it('should uncheck', async function({context, output, server}) {
127-
const page = await context.newPage();
148+
it('should uncheck', async function({page, output, setContent, server}) {
128149
await page.goto(server.EMPTY_PAGE);
129-
await page.setContent(`<input id="checkbox" type="checkbox" checked name="accept" onchange="console.log(checkbox.checked)"></input>`);
150+
await setContent(`<input id="checkbox" type="checkbox" checked name="accept" onchange="console.log(checkbox.checked)"></input>`);
130151
const [message] = await Promise.all([
131152
page.waitForEvent('console'),
132153
output.waitFor('uncheck'),
@@ -138,10 +159,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
138159
expect(message.text()).toBe("false");
139160
});
140161

141-
it('should select', async function({context, output, server}) {
142-
const page = await context.newPage();
162+
it('should select', async function({page, output, setContent, server}) {
143163
await page.goto(server.EMPTY_PAGE);
144-
await page.setContent('<select id="age" onchange="console.log(age.selectedOptions[0].value)"><option value="1"><option value="2"></select>');
164+
await setContent('<select id="age" onchange="console.log(age.selectedOptions[0].value)"><option value="1"><option value="2"></select>');
145165
const [message] = await Promise.all([
146166
page.waitForEvent('console'),
147167
output.waitFor('select'),
@@ -153,10 +173,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
153173
expect(message.text()).toBe("2");
154174
});
155175

156-
it('should await popup', async function({context, output, server}) {
157-
const page = await context.newPage();
176+
it('should await popup', async function({context, page, output, setContent, server}) {
158177
await page.goto(server.EMPTY_PAGE);
159-
await page.setContent('<a target=_blank rel=noopener href="/popup/popup.html">link</a>');
178+
await setContent('<a target=_blank rel=noopener href="/popup/popup.html">link</a>');
160179
const [popup] = await Promise.all([
161180
context.waitForEvent('page'),
162181
output.waitFor('waitForEvent'),
@@ -171,10 +190,9 @@ describe.skip(USES_HOOKS)('Recorder', function() {
171190
expect(popup.url()).toBe(`${server.PREFIX}/popup/popup.html`);
172191
});
173192

174-
it('should await navigation', async function({context, output, server}) {
175-
const page = await context.newPage();
193+
it('should await navigation', async function({page, output, setContent, server}) {
176194
await page.goto(server.EMPTY_PAGE);
177-
await page.setContent(`<a onclick="setTimeout(() => window.location.href='${server.PREFIX}/popup/popup.html', 1000)">link</a>`);
195+
await setContent(`<a onclick="setTimeout(() => window.location.href='${server.PREFIX}/popup/popup.html', 1000)">link</a>`);
178196
await Promise.all([
179197
page.waitForNavigation(),
180198
output.waitFor('waitForNavigation'),

0 commit comments

Comments
 (0)