Skip to content

Commit 4d3cb57

Browse files
authored
fix(LoginHelper): increase GitHub login timeout and handle GitHub popup re-authorization (#105)
* fix(common): increase GitHub login timeout Signed-off-by: Dominik Augustín <daugusti@redhat.com> * chore: include built dist for branch reference * fix: increase GitHub login timeout from 3s to 30s Signed-off-by: Dominik Augustín <daugusti@redhat.com> * fix: loginAsGithubUser case when user might already be logged in before Signed-off-by: Dominik Augustín <daugusti@redhat.com> * chore: removed built dist files Signed-off-by: Dominik Augustín <daugusti@redhat.com> * refactor: extract GitHub popup reauthorization logic into a separate method Signed-off-by: Dominik Augustín <daugusti@redhat.com> * chore: bump version to 1.1.41 Signed-off-by: Dominik Augustín <daugusti@redhat.com> * chore: update changelog for version 1.1.41 with fixes, additions, and changes Signed-off-by: Dominik Augustín <daugusti@redhat.com> * fix: link GitHub login timeout to waiting for the repositories header Signed-off-by: Dominik Augustín <daugusti@redhat.com> * fix: update GitHub login timeout selector to wait for the Home header Signed-off-by: Dominik Augustín <daugusti@redhat.com> * chore: update version to 1.1.43 in package.json and changelog.md Signed-off-by: Dominik Augustín <daugusti@redhat.com> * fix: update GitHub login timeout selector to use role-based query Signed-off-by: Dominik Augustín <daugusti@redhat.com> --------- Signed-off-by: Dominik Augustín <daugusti@redhat.com>
1 parent 606bb4a commit 4d3cb57

3 files changed

Lines changed: 62 additions & 18 deletions

File tree

docs/changelog.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,22 @@
22

33
All notable changes to this project will be documented in this file.
44

5-
## [1.1.42] - Current
5+
## [1.1.43] - Current
6+
7+
### Fixed
8+
9+
- **GitHub login null-race guard**: `loginAsGithubUser` now throws an explicit error when neither the sidebar nav nor the GitHub popup appears after clicking Sign In, instead of silently continuing with a stale session file.
10+
- Increased GitHub popup wait timeouts.
11+
12+
### Added
13+
14+
- **GitHub session reuse auto-login/reauth race**: In case of existing session, handle both auto-login and reauthorization flows by racing a `nav` selector against the `popup` event.
15+
16+
### Changed
17+
18+
- **Deduplicate GitHub popup reauth logic**: Extracted shared popup reauthorization code from `loginAsGithubUser` and `checkAndReauthorizeGithubApp` into a private `handleGithubPopupReauth` method to eliminate duplication.
19+
20+
## [1.1.42]
621

722
### Changed
823

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@red-hat-developer-hub/e2e-test-utils",
3-
"version": "1.1.42",
3+
"version": "1.1.43",
44
"description": "Test utilities for RHDH E2E tests",
55
"license": "Apache-2.0",
66
"repository": {

src/playwright/helpers/common.ts

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export class LoginHelper {
6161

6262
await this.page.click('[value="Sign in"]');
6363
await this.page.fill("#app_totp", this.getGitHub2FAOTP(userid));
64-
test.setTimeout(130000);
64+
test.setTimeout(260_000);
6565
if (
6666
(await this.uiHelper.isTextVisible(
6767
"The two-factor code you entered has already been used",
@@ -75,7 +75,9 @@ export class LoginHelper {
7575
await this.page.fill("#app_totp", this.getGitHub2FAOTP(userid));
7676
}
7777

78-
await this.page.waitForTimeout(3_000);
78+
await this.page
79+
.getByRole("heading", { name: "Home" })
80+
.waitFor({ timeout: 30_000 });
7981
}
8082

8183
async logintoKeycloak(popup: Page, userid: string, password: string) {
@@ -114,7 +116,30 @@ export class LoginHelper {
114116
await this.page.goto("/");
115117
await this.uiHelper.waitForLoad(12000);
116118
await this.uiHelper.clickButton("Sign In");
117-
await this.checkAndReauthorizeGithubApp();
119+
120+
// Wait for either: sidebar appears (auto-login) or popup opens (needs auth)
121+
const navPromise = this.page
122+
.waitForSelector("nav a", { timeout: 15_000 })
123+
.then(() => "nav" as const)
124+
.catch(() => null);
125+
126+
const popupPromise = this.page
127+
.waitForEvent("popup", { timeout: 15_000 })
128+
.then((popup) => ({ popup }))
129+
.catch(() => null);
130+
131+
const result = await Promise.race([navPromise, popupPromise]);
132+
133+
if (result === null) {
134+
throw new Error(
135+
"GitHub login failed: neither sidebar nor popup appeared after Sign In — session file may be stale",
136+
);
137+
}
138+
139+
if (typeof result === "object" && "popup" in result) {
140+
// Popup opened — handle reauthorization
141+
await this.handleGithubPopupReauth(result.popup);
142+
}
118143
} else {
119144
// Perform login if no session file exists, then save the state
120145
await this.logintoGithub(userid);
@@ -131,24 +156,28 @@ export class LoginHelper {
131156
async checkAndReauthorizeGithubApp() {
132157
await new Promise<void>((resolve) => {
133158
this.page.once("popup", async (popup) => {
134-
await popup.waitForLoadState();
135-
136-
// Check for popup closure for up to 10 seconds before proceeding
137-
for (let attempts = 0; attempts < 10 && !popup.isClosed(); attempts++) {
138-
await this.page.waitForTimeout(1000); // Using page here because if the popup closes automatically, it throws an error during the wait
139-
}
140-
141-
const locator = popup.locator("button.js-oauth-authorize-btn");
142-
if (!popup.isClosed() && (await locator.isVisible())) {
143-
await popup.locator("body").click();
144-
await locator.waitFor();
145-
await locator.click();
146-
}
159+
await this.handleGithubPopupReauth(popup);
147160
resolve();
148161
});
149162
});
150163
}
151164

165+
private async handleGithubPopupReauth(popup: Page) {
166+
await popup.waitForLoadState();
167+
168+
// Check for popup closure for up to 10 seconds before proceeding
169+
for (let attempts = 0; attempts < 10 && !popup.isClosed(); attempts++) {
170+
await this.page.waitForTimeout(1000); // Using page here because if the popup closes automatically, it throws an error during the wait
171+
}
172+
173+
const locator = popup.locator("button.js-oauth-authorize-btn");
174+
if (!popup.isClosed() && (await locator.isVisible())) {
175+
await popup.locator("body").click();
176+
await locator.waitFor();
177+
await locator.click();
178+
}
179+
}
180+
152181
async googleSignIn(email: string) {
153182
await new Promise<void>((resolve) => {
154183
this.page.once("popup", async (popup) => {

0 commit comments

Comments
 (0)