fix e2e test flakiness due to workshop page fixture #9288

fungairino · 2024-10-16T18:56:45Z

What does this PR do?

Hopefully resolves the flakiness we are seeing in the copyMod.spec
https://github.com/pixiebrix/pixiebrix-extension/actions/runs/11366379656/job/31626687612?pr=9287

Discussion

I have a theory that this might be caused by mishandling concurrent network requests where one might be aborted. According to the trace, we abort a request to GET /api/bricks loading the workshop page at the same time as we load the mod activation page.
- This didn't seem to be exactly the case, but I was able to resolve the flakiness by avoiding loading the workshop page fixture at the beginning of the test alongside the other extension console page.

Checklist

Leave all that are relevant and check off as completed

This PR requires a security review
This PR introduces a new library: double check it's MIT/Apache2/permissively licensed
This PR requires a node/npm version update: let the team know on #engineering
This PR requires a documentation change (link to old docs)
This PR requires a tutorial update (link to old tutorials)
This PR requires a feature flag
This PR requires a environment variable change
Added jest or playwright tests and/or storybook stories

For more information on our expectations for the PR process, see the
code review principles doc

codecov · 2024-10-16T19:05:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.23%. Comparing base (8318d74) to head (41e4553).
Report is 382 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9288      +/-   ##
==========================================
+ Coverage   74.24%   75.23%   +0.98%     
==========================================
  Files        1332     1363      +31     
  Lines       40817    42085    +1268     
  Branches     7634     7834     +200     
==========================================
+ Hits        30306    31661    +1355     
+ Misses      10511    10424      -87

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-10-16T19:13:35Z

Playwright test results

139 passed
1 flaky
2 skipped

Details

Open report ↗︎
142 tests across 46 suites
10 minutes, 2 seconds
41e4553
For more information on how to debug and view this report, see our readme

Flaky tests

chrome › tests/extensionConsole/activation.spec.ts › can activate a mod with built-in integration

Skipped tests

chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
msedge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

fungairino · 2024-10-16T20:23:48Z

My guess unfortunately was not right, but I'm trying making some slight tweaks to visibility checks in fixtures

This reverts commit 55b9e58.

fungairino · 2024-10-17T19:32:47Z

end-to-end-tests/fixtures/modDefinitions.ts

@@ -70,7 +70,6 @@ export const test = pageContextFixture.extend<{
  async _workshopPage({ context, extensionId }, use) {
    const newPage = await context.newPage();
    const workshopPage = new WorkshopPage(newPage, extensionId);
-    await workshopPage.goto();


since we navigate and refresh the workshop each time we need to use it, we don't initially need to load it. Avoiding this goto avoids some weird behavior we are observing when we load the extension console twice in quick succession

fungairino · 2024-10-17T19:33:46Z

end-to-end-tests/pageObjects/extensionConsole/modsPage.ts

+    await expect(async () => {
+      await this.getByRole("heading", { name: "Activate " }).waitFor({
+        timeout: 10_000,
+      });
+      try {
+        await this.getByText(this.modId).waitFor({ timeout: 10_000 });
+      } catch (error) {
+        await this.page.reload();
+        throw error;
+      }
+    }).toPass({ timeout: 30_000 });


I'm not actually sure we need this additional handling any more... I added this when I was seeing issues loading the activation page where the page actually just crashed, but not seeing that any more with the changes to the workshop page fixture

This reload cycle will happen ever 10s, so I'd be in favor of removing if you don't think it's needed especially w.r.t. this being on a POM

I'll try this in the follow-up right after this is merged

fungairino · 2024-10-17T19:35:10Z

end-to-end-tests/tests/pageEditor/copyMod.spec.ts

-  test.fixme(
-    process.env.GITHUB_WORKFLOW === PRE_RELEASE_BROWSER_WORKFLOW_NAME &&
-      isMsEdge(chromiumChannel),
-    "Skipping test for MS Edge in pre-release workflow, see https://github.com/pixiebrix/pixiebrix-extension/issues/9125",
-  );


I think the edge flakiness has been resolved by this PR change.

fungairino · 2024-10-18T15:07:27Z

end-to-end-tests/pageObjects/pageEditor/pageEditorPage.ts

-    const modsPage = new ModsPage(this.page, this.extensionId);
-    await modsPage.goto();


this modsPage wasn't being used.

fungairino · 2024-10-18T15:24:37Z

There's some additional msedge flakiness that I'll address in a follow-up

mnholtz · 2024-10-18T18:33:43Z

end-to-end-tests/pageObjects/pageEditor/pageEditorPage.ts

@@ -300,8 +299,9 @@ export class PageEditorPage extends BasePageObject {
   * @see newPageEditorPage in fixtures/testBase.ts
   */
  async cleanup() {
-    const modsPage = new ModsPage(this.page, this.extensionId);
-    await modsPage.goto();
+    if (this.savedModIds.length === 0) {


opportunity/future work: Do we want to consider removing the cleanup portion of our tests in favor of the server cron job?

Yea, I think that would be a good endeavor to pursue! Would save us some time on test runs

ensure workshop page is done loading in fixture

6325c93

fungairino self-assigned this Oct 16, 2024

fungairino requested a review from grahamlangford October 16, 2024 19:00

fungairino marked this pull request as ready for review October 16, 2024 19:00

twschiller added the testing label Oct 16, 2024

Use waitFor to ensure element is actually visible before continuing

1f23857

vercel bot deployed to Preview October 16, 2024 20:10 View deployment

debugging flakiness

55b9e58

vercel bot deployed to Preview October 16, 2024 20:59 View deployment

fungairino added 2 commits October 16, 2024 17:12

try to handle page loading flakiness

47d02b6

Revert "debugging flakiness"

f635105

This reverts commit 55b9e58.

vercel bot deployed to Preview October 16, 2024 21:14 View deployment

try reloading

70ada8c

vercel bot deployed to Preview October 16, 2024 21:37 View deployment

fix missing navigation call

56b26a3

vercel bot deployed to Preview October 17, 2024 14:32 View deployment

config changes to debug flakiness

769f325

vercel bot deployed to Preview October 17, 2024 15:30 View deployment

navigate directly

8799487

vercel bot deployed to Preview October 17, 2024 17:55 View deployment

wip

49796b5

vercel bot deployed to Preview October 17, 2024 18:18 View deployment

wip

1df39eb

fungairino commented Oct 17, 2024

View reviewed changes

vercel bot deployed to Preview October 17, 2024 19:32 View deployment

fungairino commented Oct 17, 2024

View reviewed changes

wip

fa3b1e1

vercel bot deployed to Preview October 18, 2024 00:33 View deployment

wip

a5131a7

vercel bot deployed to Preview October 18, 2024 14:47 View deployment

fungairino commented Oct 18, 2024

View reviewed changes

wip

f891334

vercel bot deployed to Preview October 18, 2024 15:09 View deployment

fungairino mentioned this pull request Oct 18, 2024

Fix Pre-release Browser workflow #9283

Closed

fungairino changed the title ~~ensure workshop page fixture is done loading before being used~~ fix e2e test flakiness due to workshop page fixture Oct 18, 2024

Merge branch 'main' into activation-flakiness

be836da

vercel bot deployed to Preview October 18, 2024 16:32 View deployment

pr feedback

41e4553

vercel bot deployed to Preview October 18, 2024 18:25 View deployment

fungairino enabled auto-merge (squash) October 18, 2024 18:26

fungairino requested a review from mnholtz October 18, 2024 18:27

mnholtz reviewed Oct 18, 2024

View reviewed changes

grahamlangford approved these changes Oct 18, 2024

View reviewed changes

fungairino merged commit 4c6ca51 into main Oct 18, 2024
35 checks passed

fungairino deleted the activation-flakiness branch October 18, 2024 18:42

grahamlangford added this to the 2.1.6 milestone Oct 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix e2e test flakiness due to workshop page fixture #9288

fix e2e test flakiness due to workshop page fixture #9288

fungairino commented Oct 16, 2024 •

edited

Loading

codecov bot commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading

fungairino commented Oct 16, 2024

fungairino Oct 17, 2024 •

edited

Loading

fungairino Oct 17, 2024 •

edited

Loading

mnholtz Oct 18, 2024

fungairino Oct 18, 2024

fungairino Oct 17, 2024

fungairino Oct 18, 2024

fungairino commented Oct 18, 2024

mnholtz Oct 18, 2024

fungairino Oct 18, 2024

grahamlangford Oct 18, 2024

		const modsPage = new ModsPage(this.page, this.extensionId);
		await modsPage.goto();

fix e2e test flakiness due to workshop page fixture #9288

fix e2e test flakiness due to workshop page fixture #9288

Conversation

fungairino commented Oct 16, 2024 • edited Loading

What does this PR do?

Discussion

Checklist

codecov bot commented Oct 16, 2024 • edited Loading

Codecov Report

github-actions bot commented Oct 16, 2024 • edited Loading

Playwright test results

Details

fungairino commented Oct 16, 2024

fungairino Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

fungairino Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

mnholtz Oct 18, 2024

Choose a reason for hiding this comment

fungairino Oct 18, 2024

Choose a reason for hiding this comment

fungairino Oct 17, 2024

Choose a reason for hiding this comment

fungairino Oct 18, 2024

Choose a reason for hiding this comment

fungairino commented Oct 18, 2024

mnholtz Oct 18, 2024

Choose a reason for hiding this comment

fungairino Oct 18, 2024

Choose a reason for hiding this comment

grahamlangford Oct 18, 2024

Choose a reason for hiding this comment

fungairino commented Oct 16, 2024 •

edited

Loading

codecov bot commented Oct 16, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading

fungairino Oct 17, 2024 •

edited

Loading

fungairino Oct 17, 2024 •

edited

Loading