Reduce number of network reqs in visual regression

[Rigellute]

On my machine, this cuts down the test run time from ~5 mins to ~3 mins!

Closes #249

[wgolledge]

One small comment but looks great!

[wgolledge]

I think it could be nice still having these split into separate landscape and portrait tests with a beforeAll or something.

[Rigellute]

I would prefer the same, however I kept running into an issue using beforeAll where the page would close after the first run - then all the remaining tests would fail.

[Rigellute]

This is the error I get when using beforeAll

    Protocol error (Page.navigate): Session closed. Most likely the page has been closed.

      72 |   describe("Go to page, test landscape and portrait viewport", () => {
      73 |     beforeAll(async () => {
    > 74 |       await page.goto(rendererUrl, { waitUntil: "load" });
         |                  ^
      75 |       await page.mouse.move(0, 0);
      76 |       await delay(500);
      77 |     });

[wgolledge]

Ah - that's happening because of the execution order, https://jestjs.io/docs/en/setup-teardown.html. beforeAll getting triggered before the beforeEach page.close()

[wgolledge]

A (crude) fix would be to force the navigation to run after the page.close()

      let pageRendered = false;

      beforeEach(async () => {
        if (pageRendered) {
          return;
        }
        pageRendered = true;
        await page.goto(rendererUrl, { waitUntil: "load" });
  
        await page.mouse.move(0, 0);
      })

  
      it("matches landscape", async () => {
        await page.setViewport({ width: 1200, height: 600 });
        await delay(500);
        await matchSnapshot({ viewport: "landscape" });
      });
      it("matches portrait", async () => {
        await page.setViewport({ width: 600, height: 1200 });
        await delay(500);
        await matchSnapshot({ viewport: "portrait" });
      });
    })

[wgolledge]

Running with if (page.url() != rendererUrl) was surprisingly ~30% slower than the above (or by just rendering the render in the landscape test)

[Rigellute]

Ah okay cool. I'll go with your suggestion. Shame that the more idiomatic beforeAll fails due the somewhat obscure execution order.

Although I now see the afterEach in setupTests.ts that closes the page - so now it makes sense.

[Rigellute]

On reflection - it looks like the page is setup and torn down in setupTests but then the page is only ever used in the visual regression test?

Perhaps we should move page setup and teardown to visual-regression?

[andyrichardson]

Perhaps we should move page setup and teardown to visual-regression?

We did have that in the past but from what I remember, using jest-puppeteer was solving some determinism issues we were having during tests.

Totally open to having the setup configured in this test alone if we get a performance benefit! Going to ask some Formidafolks today about getting something like Percy setup