sadpandajoe commented on code in PR #36196:
URL: https://github.com/apache/superset/pull/36196#discussion_r2583872588
##########
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:
Already addressed - error message now includes current URL for debugging
context.
##########
superset-frontend/playwright/tests/experimental/dataset/dataset-list.spec.ts:
##########
@@ -0,0 +1,201 @@
+/**
+ * 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 { test, expect } from '@playwright/test';
+import { DatasetListPage } from '../../../pages/DatasetListPage';
+import { ExplorePage } from '../../../pages/ExplorePage';
+import { DeleteConfirmationModal } from
'../../../components/modals/DeleteConfirmationModal';
+import { DuplicateDatasetModal } from
'../../../components/modals/DuplicateDatasetModal';
+import { Toast } from '../../../components/core/Toast';
+import {
+ apiDeleteDataset,
+ apiGetDataset,
+ getDatasetByName,
+ duplicateDataset,
+ ENDPOINTS,
+} from '../../../helpers/api/dataset';
+
+test.describe('Dataset List', () => {
+ let datasetListPage: DatasetListPage;
+ let explorePage: ExplorePage;
+ let testResources: { datasetIds: number[] } = { datasetIds: [] };
+
+ test.beforeEach(async ({ page }) => {
+ datasetListPage = new DatasetListPage(page);
+ explorePage = new ExplorePage(page);
+ testResources = { datasetIds: [] }; // Reset for each test
+
+ // Navigate to dataset list page
+ await datasetListPage.goto();
+ await datasetListPage.waitForTableLoad();
+ });
+
+ test.afterEach(async ({ page }) => {
+ // Cleanup any resources created during the test
+ const promises = [];
+ for (const datasetId of testResources.datasetIds) {
+ promises.push(
+ apiDeleteDataset(page, datasetId, {
+ failOnStatusCode: false,
+ }).catch(() => {}),
+ );
+ }
+ await Promise.all(promises);
+ });
+
+ test('should navigate to Explore when dataset name is clicked', async ({
+ page,
+ }) => {
+ // Use existing example dataset (hermetic - loaded in CI via
--load-examples)
+ const datasetName = 'members_channels_2';
Review Comment:
Already addressed - extracted to `TEST_DATASETS.EXAMPLE_DATASET` constant.
##########
superset-frontend/playwright/components/core/Table.ts:
##########
@@ -0,0 +1,83 @@
+/**
+ * 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 { Locator, Page } from '@playwright/test';
+
+/**
+ * Table component for Superset ListView tables.
+ */
+export class Table {
+ private readonly page: Page;
+ private readonly tableSelector: string;
+
+ private static readonly SELECTORS = {
+ TABLE_ROW: '[data-test="table-row"]',
+ };
+
+ constructor(page: Page, tableSelector = '[data-test="listview-table"]') {
+ this.page = page;
+ this.tableSelector = tableSelector;
+ }
+
+ /**
+ * Gets the table element locator
+ */
+ get element(): Locator {
+ return this.page.locator(this.tableSelector);
+ }
+
+ /**
+ * Gets a table row by exact text match in the first cell (dataset name
column).
+ * Uses exact match to avoid substring collisions (e.g.,
'members_channels_2' vs 'duplicate_members_channels_2_123').
+ * @param rowText - Exact text to find in the row's first cell
+ */
+ getRow(rowText: string): Locator {
+ return this.element.locator(Table.SELECTORS.TABLE_ROW).filter({
+ has: this.page.getByRole('cell', { name: rowText, exact: true }),
+ });
+ }
+
+ /**
+ * Clicks a link within a specific row
+ * @param rowText - Text to identify the row
+ * @param linkSelector - Selector for the link within the row
+ */
+ async clickRowLink(rowText: string, linkSelector: string): Promise<void> {
+ const row = this.getRow(rowText);
+ await row.locator(linkSelector).click();
+ }
+
+ /**
+ * Waits for the table to be visible
+ * @param options - Optional wait options
+ */
+ async waitForVisible(options?: { timeout?: number }): Promise<void> {
+ await this.element.waitFor({ state: 'visible', ...options });
+ }
+
+ /**
+ * Clicks an action button in a row by selector
+ * @param rowText - Text to identify the row
+ * @param selector - CSS selector for the action element
+ */
+ async clickRowAction(rowText: string, selector: string): Promise<void> {
+ const row = this.getRow(rowText);
+ await row.locator(selector).first().click();
Review Comment:
Already addressed - now validates count and throws clear errors on 0 or
multiple matches.
##########
superset-frontend/playwright/helpers/api/dataset.ts:
##########
@@ -0,0 +1,160 @@
+/**
+ * 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 { Page, APIResponse, expect } from '@playwright/test';
+import { apiGet, apiPost, apiDelete, ApiRequestOptions } from './requests';
+
+export const ENDPOINTS = {
+ DATASET: 'api/v1/dataset/',
+} as const;
+
+/**
+ * TypeScript interface for dataset creation API payload
+ * Provides compile-time safety for required fields
+ */
+export interface DatasetCreatePayload {
+ database: number;
+ catalog: string | null;
+ schema: string;
+ table_name: string;
+}
+
+/**
+ * TypeScript interface for dataset API response
+ * Represents the shape of dataset data returned from the API
+ */
+export interface DatasetResult {
+ id: number;
+ table_name: string;
+ sql?: string;
+ schema?: string;
+ database: {
+ id: number;
+ database_name: string;
+ };
+ owners?: Array<{ id: number }>;
+ dataset_type?: 'physical' | 'virtual';
+}
+
+/**
+ * POST request to create a dataset
+ * @param page - Playwright page instance (provides authentication context)
+ * @param requestBody - Dataset configuration object (database, schema,
table_name)
+ * @returns API response from dataset creation
+ */
+export async function apiPostDataset(
+ page: Page,
+ requestBody: DatasetCreatePayload,
+): Promise<APIResponse> {
+ return apiPost(page, ENDPOINTS.DATASET, requestBody);
+}
+
+/**
+ * Get a dataset by its table name
+ * @param page - Playwright page instance (provides authentication context)
+ * @param tableName - The table_name to search for
+ * @returns Dataset object if found, null if not found
+ */
+export async function getDatasetByName(
+ page: Page,
+ tableName: string,
+): Promise<DatasetResult | null> {
+ // Use Superset's filter API to search by table_name
+ const filter = {
+ filters: [
+ {
+ col: 'table_name',
+ opr: 'eq',
+ value: tableName,
+ },
+ ],
+ };
+ const queryParam = encodeURIComponent(JSON.stringify(filter));
+ const response = await apiGet(page, `${ENDPOINTS.DATASET}?q=${queryParam}`);
+
+ if (!response.ok()) {
+ return null;
+ }
+
+ const body = await response.json();
+ if (body.result && body.result.length > 0) {
+ return body.result[0] as DatasetResult;
+ }
+
+ return null;
+}
+
+/**
+ * Duplicates a dataset via API and returns the new dataset info
+ * @param page - Playwright page instance (provides authentication context)
+ * @param datasetId - ID of dataset to duplicate
+ * @param newName - Name for the duplicated dataset
+ * @returns Promise resolving to new dataset info
+ */
+export async function duplicateDataset(
+ page: Page,
+ datasetId: number,
+ newName: string,
+): Promise<DatasetResult> {
+ const response = await apiPost(page, `${ENDPOINTS.DATASET}duplicate`, {
+ base_model_id: datasetId,
+ table_name: newName,
+ });
+
+ expect(response.status()).toBe(201);
Review Comment:
Already addressed - now throws `Error` instead of using `expect()`.
--
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]
