[Bug] Hoist page objects to journey level in the synthetics generator

[vigneshshanmugam]

Bug summary

When multiple page objects are created in the recorder session either by

• clicking on links that moves from a.com to b.com
• Opening popups from a.com to b.com

Playwright creates page objects for each of those pages, The Synthetics JS generator then creates the necessary code to express those sessions. Take an example below

step('Go to https://vigneshh.in/', async () => {
  await page.goto('https://vigneshh.in/');
});
step('Click text=Tailor', async () => {
  const [page1] = await Promise.all([
    page.waitForEvent('popup'),
    page.click('text=Tailor')
  ]);
  await page1.click('text=Packages 0');
  expect(page1.url()).toBe('https://github.com/orgs/zalando/packages?repo_name=tailor');
  await page1.close();
});
step('Click text=Babel Minify', async () => {
  const [page2] = await Promise.all([
    page.waitForEvent('popup'),
    page.click('text=Babel Minify')
  ]);
  await page2.close();
});

The problem here is we want the variable declaration for page1 and page2 and their close method to be on their dedicated steps or grouped on a single step. This issue was mitigated before as we controlled the time when steps gets created which was only on clicks that triggered new navigations.

But with the introduction of Step divider UI, Its way easier for our users to create steps for each action and run in to the variable not being defined issue. Creating step before page1 close would yield this code.

step('Go to https://vigneshh.in/', async () => {
  await page.goto('https://vigneshh.in/');
});
step('Click text=Tailor', async () => {
  const [page1] = await Promise.all([
    page.waitForEvent('popup'),
    page.click('text=Tailor')
  ]);
  await page1.click('text=Packages 0');
  expect(page1.url()).toBe('https://github.com/orgs/zalando/packages?repo_name=tailor');
});
step('Close page', async () => {
  await page1.close();
});
step('Click text=Babel Minify', async () => {
  const [page2] = await Promise.all([
    page.waitForEvent('popup'),
    page.click('text=Babel Minify')
  ]);
  await page2.close();
});

Recorder Version

main branch

Steps to reproduce

1. Start the recorder and type any page url.
2. Taking www.vigneshh.in for now as the test was performed on that page.
3. Click on a links that opens new page objects.
4. Try creating step dividers and run Test button.

Expected behavior

• Synthetics tests should run without any JS errors.

Additional information

The easiest way to solve this would be to hoist the page variables before the step function declaration and adds guards when functions are invoked on them.

let page1;
step('Click text=Tailor', async () => {
  page1 = await Promise.all([
    page.waitForEvent('popup'),
    page.click('text=Tailor')
  ]);
  await page1.click('text=Packages 0');
  expect(page1.url()).toBe('https://github.com/orgs/zalando/packages?repo_name=tailor');
});
step('Close page', async () => {
  await page1.close();
});

Another approaches, Google chrome recorder tries to create separate blocks for each of the step, but requires you to be on the recorder page to work correctly.

{
    const targetPage = page;
    const element = await waitForSelectors([["aria/Tailor"],["body > div > div.profile > div.content.left > p:nth-child(5) > a:nth-child(1)"]], targetPage, { timeout, visible: true });
    await scrollIntoViewIfNeeded(element, timeout);
    await element.click({ offset: { x: 32.0390625, y: 8.90625} });
}
{
    const target = await browser.waitForTarget(t => t.url() === "https://github.com/zalando/tailor", { timeout });
    const targetPage = await target.page();
    targetPage.setDefaultTimeout(timeout);
    await targetPage.keyboard.down("Meta");
}

Open to new ideas as well, If anyone wants to tackle, I am more than happy to pair on this one.

[justinkambic]

@vigneshshanmugam do you think this blocks the next release?

[vigneshshanmugam]

@justinkambic I dont think so, For now if we can educate people about this edge case that should be good enough while we figure out a fix on the generator side.

[shahzad31]

POST FF Testing LGTM

Works as expected with following example

let page1;
step('Go to https://vigneshh.in/', async () => {
  await page.goto('https://vigneshh.in/');
  await page.click('text=Blogs & Talks');
  await page.click('html');
  await page.press('body:has-text("Vignesh Shanmugam Web Performance, JavaScript Fanatic, Front End Enthusiast, Moz")', 'Meta+f');
});
step('Click text=Tailor', async () => {
  [page1] = await Promise.all([
    page.waitForEvent('popup'),
    page.click('text=Tailor')
  ]);
  await page1.click('summary:has-text("Code")');
  await page1.click('text=GitHub CLI');
  await page1.click(':nth-match([aria-label="Copy to clipboard"], 2)');
  await page1.click('summary:has-text("Code")');
  await Promise.all([
    page1.waitForNavigation(/*{ url: 'https://github.com/zalando/tailor/pulls' }*/),
    page1.click('text=Pull requests 12')
  ]);
});