-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix e2e test flakiness due to workshop page fixture #9288
Changes from 11 commits
6325c93
1f23857
55b9e58
47d02b6
f635105
70ada8c
56b26a3
769f325
8799487
49796b5
1df39eb
fa3b1e1
a5131a7
f891334
be836da
41e4553
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ | |
import { expect, type Page } from "@playwright/test"; | ||
import { getBaseExtensionConsoleUrl } from "../constants"; | ||
import { BasePageObject } from "../basePageObject"; | ||
import { ensureVisibility } from "../../utils"; | ||
import { validateRegistryId } from "@/types/helpers"; | ||
import { API_PATHS, UI_PATHS } from "@/data/service/urlPaths"; | ||
import { DEFAULT_TIMEOUT } from "../../../playwright.config"; | ||
|
@@ -169,12 +168,19 @@ export class ActivateModPage extends BasePageObject { | |
|
||
async goto() { | ||
await this.page.goto(this.activateModUrl); | ||
|
||
await expect( | ||
this.getByRole("heading", { name: "Activate " }), | ||
).toBeVisible(); | ||
// Loading the mod details may take a long time. Using ensureVisibility because the modId may be attached and hidden | ||
await ensureVisibility(this.getByText(this.modId)); | ||
// Wrapped in toPass due to flakiness with the page not loading ex: | ||
// https://github.com/pixiebrix/pixiebrix-extension/actions/runs/11373118427?pr=9286 | ||
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 }); | ||
Comment on lines
+173
to
+183
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll try this in the follow-up right after this is merged |
||
} | ||
|
||
async getIntegrationConfigField(index: number) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,12 +23,7 @@ import { uuidv4 } from "@/types/helpers"; | |
import { type Serializable } from "playwright-core/types/structs"; | ||
import path from "node:path"; | ||
import { FloatingActionButton } from "end-to-end-tests/pageObjects/floatingActionButton"; | ||
import { | ||
getSidebarPage, | ||
runModViaQuickBar, | ||
isMsEdge, | ||
PRE_RELEASE_BROWSER_WORKFLOW_NAME, | ||
} from "end-to-end-tests/utils"; | ||
import { getSidebarPage, runModViaQuickBar } from "end-to-end-tests/utils"; | ||
import { VALID_UUID_REGEX } from "@/types/stringTypes"; | ||
|
||
test("copying a mod that uses the PixieBrix API is copied correctly", async ({ | ||
|
@@ -86,14 +81,7 @@ test("run a copied mod with a built-in integration", async ({ | |
context, | ||
newPageEditorPage, | ||
verifyModDefinitionSnapshot, | ||
chromiumChannel, | ||
}) => { | ||
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", | ||
); | ||
Comment on lines
-91
to
-95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the edge flakiness has been resolved by this PR change. |
||
|
||
let giphyRequestPostData: Serializable; | ||
// The giphy search request is proxied through the PixieBrix server, which is kicked off in the background/service | ||
// worker. Playwright experimentally supports mocking service worker requests, see | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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