Using HTTPS causes the About dialog e2e license link tests to fail

[cghague]

We noticed during the recent release of TinyPilot Pro 2.6.3 that the tests that verify the license links on the About dialog are correct fail when testing against an actual TinyPilot device.

The cause of the issue is that we're connecting to the TinyPilot device over HTTPS but without a valid certificate installed. The Playwright navigation functions ignore this, but the fetch function used to test the links fails due to the missing certificate.

We should ideally update the tests to use the Playwright navigation tools, or failing that; we should skip the tests when running in this fashion as part of our pre-release tests.

[db39]

I noticed that the documentation around e2e tests isn't comprehensive. For instance, I didn't know that our e2e tests require Node 18 or greater (for fetch()), so I ended up spending unnecessary time troubleshooting that. Additionally, our process in Notion shows this command:

./dev-scripts/run-e2e-tests --base-url https://tinypilot.local

However, the script doesn't use --base-url as an argument.

Our CONTRIBUTING file mentions to turn off "Require encrypted connection (HTTPS)" to run the e2e tests. If we follow that instruction, we could probably create a workaround replacing the https base url with http so the fetch() call doesn't run into the https/certificate issue.

test("checks that all license URLs are valid and reachable", async ({
  page,
  baseURL,
}) => {
  const links = await page.locator("a.license").all();
  const paths = await Promise.all(
    links.map((link) => link.getAttribute("href"))
  );
  const http_baseURL = baseURL.replace('https://', 'http://');
  const failedUrls = [];
  await Promise.all(
    paths
      .map((path) => `${http_baseURL}${path}`)
      .map((url) =>
        fetch(url, { signal: AbortSignal.timeout(10000) })
          .then((res) => {
            if (res.status !== 200) {
              failedUrls.push(url);
            }
          })
          .catch(() => failedUrls.push(url))
      )
  );
  expect(
    failedUrls.length,
    `License link broken for URLs: ${failedUrls.join(", ")}`
  ).toBe(0);
});

I've tested the workaround, and the tests pass using it. If we use this workaround, we don't have to use navigation functions or skip the tests. Does that seem like a reasonable solution?

[cghague]

Thanks @db39.

However, the script doesn't use --base-url as an argument.

The --base-url argument is present in the run-e2e-tests script in the tinypilot and tinypilot-pro repositories, so I think there may be some confusion here.

Our CONTRIBUTING file mentions to turn off "Require encrypted connection (HTTPS)" to run the e2e tests.

I believe CONTRIBUTING.md is outdated, as before leaving, Michael updated our internal testing process to remove it explicitly. The rationale was that we wanted the tests to be completed with the "Require encrypted connection (HTTPS)" setting enabled, as that mimics the default configuration of a fresh TinyPilot Pro installation.

If we follow that instruction, we could probably create a workaround replacing the https base url with http so the fetch() call doesn't run into the https/certificate issue.

The purpose of e2e tests is to interact with the app precisely as a user would, so manipulating the addresses like this would invalidate the test results. We should be using Playwright to "click" the links.

I've tested the workaround, and the tests pass using it. If we use this workaround, we don't have to use navigation functions or skip the tests. Does that seem like a reasonable solution?

Based on the above reasoning, we should implement a fix using the Playwright navigation tools. I suspect that will be sufficient to fix the bug and will make the tests more realistic.