DR-3790: Fix timeouts on homepage localhost tests#465
Conversation
jbdalton
requested review from
Toxiapo,
clarissarichard,
samanthaandrews and
sarangj
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
samanthaandrews
left a comment
samanthaandrews
left a comment
There was a problem hiding this comment.
One small change; otherwise this looks good. Once the change is made, you can re-request review from me so I'm notified to look at the PR again.
| await page.route("**/default.jpg", (route) => route.abort()); | ||
|
|
||
| // Navigate to the page after setting up the routing rules. | ||
| await page.goto("/", { waitUntil: "load" }); |
There was a problem hiding this comment.
The default behavior for Playwright's .goto() method is waitUntil: "load", so this option can be removed.
There was a problem hiding this comment.
removed the explicit "load"
| await page.route(/.*googletagmanager\.com.*/, (route) => route.abort()); | ||
| await page.route(/.*demdex\.net.*/, (route) => route.abort()); | ||
| await page.route(/.*adobedtm\.com.*/, (route) => route.abort()); | ||
| await page.route(/.*everesttech\.net.*/, (route) => route.abort()); | ||
| await page.route(/.*ipify\.org.*/, (route) => route.abort()); | ||
| await page.route(/.*google\.com.*/, (route) => route.abort()); | ||
| await page.route(/.*omappapi\.com.*/, (route) => route.abort()); | ||
| await page.route(/.*google-analytics\.com.*/, (route) => route.abort()); |
There was a problem hiding this comment.
nice! if we find this helpful on the homepage, we could make a change to abort these calls for all tests on all pages using a global setup file. definitely doesn't need to happen on this PR.
|
Not sure if this will be helpful, but when I try to debug local versus virtual environment tests, I try to match the configurations as closely as possible. For instance, I noticed that the worker settings run with 2 workers in GH but 0 locally (which, I believe, will turns off parallelism). To make the environments more consistent, I would set a constant number of workers locally to match the GH setup. |
This is good insight. I think our config currently is set to use 2 workers on GH Actions and "undefined" in local testing. In this context "undefined" actually means "use the max number of workers" which is pretty confusing. For me has been defaulting to 4. I think due to GH Actions concurrency limitations, 2 is what Playwright recommends because if you default to |
samanthaandrews
changed the title
|
Yes, on my machine the undefined seems to default to 6 workers. |
|
I see at the top of the changelog an unreleased entry for "Fix Playwright test timeouts." Should I be adding another entry for this specific merge, like "DR-3790-Fix timeouts ...." or does that previous entry cover it? |
3 participants
Ticket:
This PR does the following:
I've added a few things, after trying several alternatives, to try to address some of the continuing timeout issues, especially for the DC homepage tests.
The two main changes I've made are:
This builds on the improvements made recently that allow us to run our playwright tests on localhost against production builds for better performance and stability during testing.
Open questions
When this is run in CI, I assume that New Relic, google, adobe etc. are not being called against the prod site. Or maybe this is a minimal overhead in that case. I found removing it from localhost calls against the prod build keeps it from being part of our analytics and runs faster, at least when the waitUntil-load method is used on the page.
How has this been tested? How should a reviewer test this?
I found that currently, while homepage.spec.ts would sometimes run without failing, if it were run a few times -- or especially if run within the entire suite, we could replicate failures. These were usually on the "verify feedback form" scenario in the homepage test.
I compared several different configurations until I found a combination that seems to be 0% flakey, at least during my testing. Specifically I've found that the visual overlay that uses IIIF's "main-feature" splash image is loaded twice (a 900 px and 1600 px version) to produce the main visual element on the home page. And whenever it fails (especially on localhost/dev, but even when run against a prod build) the feedback-form tests also fail. Overall, this is about 10-20% of the time. My theory is it's bottlenecking other assertions that Playwright might be waiting for on the "verify feedback form" functionality, which does happen to appear at the bottom of a DC homepage-load. Even after changing to
{ waitUntil: "domcontentloaded" }I found that the feedback-form test, no matter how large the timeout, would still sometimes fail.My goal was to find some combination that would run quickly, at least 7 times in a row, without a homepage.spec.ts fail, both as part of the entire test suite and when run individually.
The results of my testing look like this (if something failed over 30% of the time, I stopped before 7 tries). "Timeout" here refers to the timeout set in the specific feedback scenario via the "test" and "page" set methods. Default.jpg is the visual overlay from IIIF. There is a route-filter on it, as well as the 3rd-party calls.
Running suite:
Single test only: homepage.spec.ts
Accessibility concerns or updates
Checklist: