sadpandajoe commented on code in PR #36196:
URL: https://github.com/apache/superset/pull/36196#discussion_r2583843736
##########
superset-frontend/playwright/components/core/index.ts:
##########
@@ -21,3 +21,5 @@
export { Button } from './Button';
export { Form } from './Form';
export { Input } from './Input';
+export { Modal } from './Modal';
Review Comment:
This is incorrect - `Modal.ts` exists in the directory:
```
$ ls superset-frontend/playwright/components/core/
Button.ts Form.ts index.ts Input.ts Modal.ts Table.ts Toast.ts
```
The export is valid and tests run successfully.
##########
superset-frontend/playwright/pages/AuthPage.ts:
##########
@@ -83,6 +84,54 @@ export class AuthPage {
await loginButton.click();
}
+ /**
+ * Wait for successful login by verifying the login response and session
cookie.
+ * Call this after loginWithCredentials to ensure authentication completed.
+ *
+ * This does NOT assume a specific landing page (which is configurable).
+ * Instead it:
+ * 1. Checks if session cookie already exists (guards against race condition)
+ * 2. Waits for POST /login/ response with redirect status
+ * 3. Polls for session cookie to appear
+ *
+ * @param options - Optional wait options
+ */
+ async waitForLoginSuccess(options?: { timeout?: number }): Promise<void> {
+ const timeout = options?.timeout || TIMEOUT.PAGE_LOAD;
+ const startTime = Date.now();
+
+ // 1. Guard: Check if session cookie already exists (race condition
protection)
+ const existingCookie = await this.getSessionCookie();
+ if (existingCookie?.value) {
+ // Already authenticated - login completed before we started waiting
+ return;
+ }
+
+ // 2. Wait for POST /login/ response
+ const loginResponse = await this.waitForLoginRequest();
+
+ // 3. Verify it's a redirect (3xx status code indicates successful login)
+ const status = loginResponse.status();
+ if (status < 300 || status >= 400) {
+ throw new Error(`Login failed: expected redirect (3xx), got ${status}`);
+ }
+
+ // 4. Poll for session cookie to appear (may take a moment after redirect)
+ const pollInterval = TIMEOUT.API_POLL_INTERVAL;
+ while (Date.now() - startTime < timeout) {
+ const sessionCookie = await this.getSessionCookie();
+ if (sessionCookie && sessionCookie.value) {
+ // Success - session cookie has landed
+ return;
+ }
+ await this.page.waitForTimeout(pollInterval);
+ }
+
+ throw new Error(
+ `Login timeout: session cookie did not appear within ${timeout}ms`,
+ );
Review Comment:
The suggested `document.cookie.includes('session=')` approach won't work
here - Superset's session cookie is `HttpOnly`, so it's not accessible via
`document.cookie` in the browser context.
The current implementation uses `page.context().cookies()` which is the
correct Playwright API for reading HttpOnly cookies.
Polling interval was already increased from 100ms to 500ms in a previous
commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
