sadpandajoe commented on code in PR #36196:
URL: https://github.com/apache/superset/pull/36196#discussion_r2583844860
##########
superset-frontend/playwright/tests/auth/login.spec.ts:
##########
@@ -20,69 +20,74 @@
import { test, expect } from '@playwright/test';
import { AuthPage } from '../../pages/AuthPage';
import { URL } from '../../utils/urls';
+import { TIMEOUT } from '../../utils/constants';
-test.describe('Login view', () => {
- let authPage: AuthPage;
+/**
+ * Auth/login tests use per-test navigation via beforeEach.
+ * Each test starts fresh on the login page without global authentication.
+ * This follows the Cypress pattern for auth testing - simple and isolated.
+ */
+
+test.beforeEach(async ({ page }) => {
+ // Navigate to login page before each test (ensures clean state)
+ const authPage = new AuthPage(page);
+ await authPage.goto();
+ await authPage.waitForLoginForm();
+});
+
+test('should redirect to login with incorrect username and password', async ({
+ page,
+}) => {
+ // Create page object (already on login page from beforeEach)
+ const authPage = new AuthPage(page);
+
+ // Setup request interception before login attempt
+ const loginRequestPromise = authPage.waitForLoginRequest();
- test.beforeEach(async ({ page }: any) => {
- authPage = new AuthPage(page);
- await authPage.goto();
- await authPage.waitForLoginForm();
+ // Attempt login with incorrect credentials
+ await authPage.loginWithCredentials('admin', 'wrongpassword');
+
+ // Wait for login request and verify response
+ const loginResponse = await loginRequestPromise;
+ // Failed login returns 401 Unauthorized or 302 redirect to login
+ expect([401, 302]).toContain(loginResponse.status());
+
+ // Wait for redirect to complete before checking URL
+ await page.waitForURL(url => url.pathname.endsWith(URL.LOGIN), {
+ timeout: TIMEOUT.PAGE_LOAD,
});
- test('should redirect to login with incorrect username and password', async
({
- page,
- }: any) => {
- // Setup request interception before login attempt
- const loginRequestPromise = authPage.waitForLoginRequest();
+ // Verify we stay on login page
+ const currentUrl = await authPage.getCurrentUrl();
+ expect(currentUrl).toContain(URL.LOGIN);
- // Attempt login with incorrect credentials
- await authPage.loginWithCredentials('admin', 'wrongpassword');
+ // Verify error message is shown
+ const hasError = await authPage.hasLoginError();
+ expect(hasError).toBe(true);
+});
- // Wait for login request and verify response
- const loginResponse = await loginRequestPromise;
- // Failed login returns 401 Unauthorized or 302 redirect to login
- expect([401, 302]).toContain(loginResponse.status());
+test('should login with correct username and password', async ({ page }) => {
+ // Create page object (already on login page from beforeEach)
+ const authPage = new AuthPage(page);
- // Wait for redirect to complete before checking URL
- await page.waitForURL((url: any) => url.pathname.endsWith('login/'), {
- timeout: 10000,
- });
+ // Setup request interception before login attempt
+ const loginRequestPromise = authPage.waitForLoginRequest();
- // Verify we stay on login page
- const currentUrl = await authPage.getCurrentUrl();
- expect(currentUrl).toContain(URL.LOGIN);
+ // Login with correct credentials
+ await authPage.loginWithCredentials('admin', 'general');
Review Comment:
Already addressed - credentials use environment variables with test defaults:
```typescript
const adminUsername = process.env.PLAYWRIGHT_ADMIN_USERNAME || 'admin';
const adminPassword = process.env.PLAYWRIGHT_ADMIN_PASSWORD || 'general';
```
##########
superset-frontend/playwright/global-setup.ts:
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import {
+ chromium,
+ FullConfig,
+ Browser,
+ BrowserContext,
+} from '@playwright/test';
+import { mkdir } from 'fs/promises';
+import { dirname } from 'path';
+import { AuthPage } from './pages/AuthPage';
+
+/**
+ * Global setup function that runs once before all tests.
+ * Authenticates as admin user and saves the authentication state
+ * to be reused by tests in the 'chromium' project (E2E tests).
+ *
+ * Auth tests (chromium-unauth project) don't use this - they login
+ * per-test via beforeEach for isolation and simplicity.
+ */
+async function globalSetup(config: FullConfig) {
+ // Get baseURL with fallback to default
+ // FullConfig.use doesn't exist in the type - baseURL is only in
projects[0].use
+ const baseURL = config.projects[0]?.use?.baseURL || 'http://localhost:8088';
+
+ console.log('[Global Setup] Authenticating as admin user...');
+
+ let browser: Browser | null = null;
+ let context: BrowserContext | null = null;
+
+ try {
+ // Launch browser
+ browser = await chromium.launch();
+ } catch (error) {
+ console.error('[Global Setup] Failed to launch browser:', error);
+ throw new Error('Browser launch failed - check Playwright installation');
+ }
+
+ try {
+ context = await browser.newContext({ baseURL });
+ const page = await context.newPage();
+
+ // Use AuthPage to handle login logic (DRY principle)
+ const authPage = new AuthPage(page);
+ await authPage.goto();
+ await authPage.waitForLoginForm();
+ await authPage.loginWithCredentials('admin', 'general');
Review Comment:
Already addressed - credentials use environment variables with test defaults:
```typescript
const adminUsername = process.env.PLAYWRIGHT_ADMIN_USERNAME || 'admin';
const adminPassword = process.env.PLAYWRIGHT_ADMIN_PASSWORD || 'general';
```
--
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]
