openverse icon indicating copy to clipboard operation
openverse copied to clipboard

Lower playwright default timeout

Open sarayourfriend opened this issue 4 years ago • 2 comments

Problem

Our current default playwright timeout is very high (1 minute) and makes debugging new tests annoying. It's also potentially too flexible if we want to rely on playwright tests to give us some indication of the general performance of the application.

Description

Our current end to end tests should be very fast under most conditions. Specifically, unless a new page is being accessed (in which case a real API request will have to be made) the every request and navigation never leaves the local loopback. With our current high timeout of 1 minute, we're probably losing some insight into when parts of the application slow down, but we're also making debugging new tests annoying. If a selector you write doesn't work, then you have to wait a full minute for it to fail unless you remembered specifically to pass a shorter timeout to the selector function.

Curious what @WordPress/openverse-frontend folks think about this and what the lowered timeout should be if we did do it. Most of the time I find that 100 works fine, but it might be too low for certain things like page navigation. In those cases, should we expect the test writer to manually raise the timeout only for those cases when it is necessary? Or should we choose a middle ground like 1 or 2 seconds? Those still seem very high if we expect our app to be performant (especially considering we're taking network latency out of the equation except when running update-tapes).

If we do lower it, we'd also want to default to a higher value when updating tapes, so that we do account for the network latency.

Implementation

  • [ ] 🙋 I would be interested in implementing this feature.

sarayourfriend avatar Apr 11 '22 13:04 sarayourfriend

Having fast Playwright tests would be awesome. I am, however, worried that it might also make them flaky in CI if for whatever reason some tests become slower.

obulat avatar Apr 13 '22 03:04 obulat

@WordPress/openverse-frontend and @sarayourfriend I looked into this a bit today. Lowering the timeout to 1s led to only two repeatable and consistent test failures:

  • e2e/report-media.spec.ts:138:9
  • e2e/load-more.spec.ts:75:11

Some other observations:

  • Playwright's default test timeout is 30s, so 50% less than ours.
  • This did not meaningfully change the time to run ov just frontend/run test:playwright locally it still took ~3 minutes regardless of the timeout. This is because the vast majority of tests run quite quickly and the ones that do exceed short timeouts are running concurrently with others, and still finish before the entire suite does.
  • The timing out tests may have configuration issues, or improperly aren't awaiting certain actions, or other issues causing the slowdown.

Conclusions

I think we can pretty easily reduce this number to 1s and add specific increased timeouts to the relevant tests. Or, even better, fix the likely issues with those tests.

zackkrida avatar Sep 04 '24 16:09 zackkrida