sadpandajoe commented on code in PR #36196: URL: https://github.com/apache/superset/pull/36196#discussion_r2583888041
########## superset-frontend/playwright/tests/experimental/dataset/dataset-list.spec.ts: ########## @@ -0,0 +1,243 @@ +/** + * 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, + ENDPOINTS, +} from '../../../helpers/api/dataset'; + +/** + * Test data constants + * These reference example datasets loaded via --load-examples in CI + */ +const TEST_DATASETS = { + EXAMPLE_DATASET: 'members_channels_2', +} as const; + +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(error => { + // Log cleanup failures to avoid silent resource leaks + console.warn( + `[Cleanup] Failed to delete dataset ${datasetId}:`, + String(error), + ); + }), + ); + } + 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 = TEST_DATASETS.EXAMPLE_DATASET; + const dataset = await getDatasetByName(page, datasetName); + expect(dataset).not.toBeNull(); + + // Verify dataset is visible in list (uses page object + Playwright auto-wait) + await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible(); + + // Click on dataset name to navigate to Explore + await datasetListPage.clickDatasetName(datasetName); + + // Wait for Explore page to load (validates URL + datasource control) + await explorePage.waitForPageLoad(); + + // Verify correct dataset is loaded in datasource control + const loadedDatasetName = await explorePage.getDatasetName(); + expect(loadedDatasetName).toContain(datasetName); + + // Verify visualization switcher shows default viz type (indicates full page load) + await expect(explorePage.getVizSwitcher()).toBeVisible(); + await expect(explorePage.getVizSwitcher()).toContainText('Table'); + }); + + test('should delete a dataset with confirmation', async ({ page }) => { + // Get example dataset to duplicate + const originalName = TEST_DATASETS.EXAMPLE_DATASET; + const originalDataset = await getDatasetByName(page, originalName); + expect(originalDataset).not.toBeNull(); + + // Create throwaway copy for deletion (hermetic - uses UI duplication) + const datasetName = `test_delete_${Date.now()}`; + + // Verify original dataset is visible in list + await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible(); + + // Set up response intercept to capture duplicate dataset ID + const duplicateResponsePromise = page.waitForResponse( + response => + response.url().includes(`${ENDPOINTS.DATASET}duplicate`) && + response.status() === 201, + ); + + // Click duplicate action button + await datasetListPage.clickDuplicateAction(originalName); + + // Duplicate modal should appear and be ready for interaction + const duplicateModal = new DuplicateDatasetModal(page); + await duplicateModal.waitForReady(); + + // Fill in new dataset name + await duplicateModal.fillDatasetName(datasetName); + + // Click the Duplicate button + await duplicateModal.clickDuplicate(); + + // Get the duplicate dataset ID from response and track immediately + const duplicateResponse = await duplicateResponsePromise; + const duplicateData = await duplicateResponse.json(); + const duplicateId = duplicateData.id; + + // Track duplicate for cleanup immediately (before any operations that could fail) + testResources = { datasetIds: [duplicateId] }; Review Comment: YAGNI - these are just 2 tests. Will extract helpers when we have more tests that need this pattern. ########## superset-frontend/playwright/tests/experimental/dataset/dataset-list.spec.ts: ########## @@ -0,0 +1,243 @@ +/** + * 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, + ENDPOINTS, +} from '../../../helpers/api/dataset'; + +/** + * Test data constants + * These reference example datasets loaded via --load-examples in CI + */ +const TEST_DATASETS = { + EXAMPLE_DATASET: 'members_channels_2', +} as const; + +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(error => { + // Log cleanup failures to avoid silent resource leaks + console.warn( + `[Cleanup] Failed to delete dataset ${datasetId}:`, + String(error), + ); + }), + ); + } + 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 = TEST_DATASETS.EXAMPLE_DATASET; + const dataset = await getDatasetByName(page, datasetName); + expect(dataset).not.toBeNull(); + + // Verify dataset is visible in list (uses page object + Playwright auto-wait) + await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible(); + + // Click on dataset name to navigate to Explore + await datasetListPage.clickDatasetName(datasetName); + + // Wait for Explore page to load (validates URL + datasource control) + await explorePage.waitForPageLoad(); + + // Verify correct dataset is loaded in datasource control + const loadedDatasetName = await explorePage.getDatasetName(); + expect(loadedDatasetName).toContain(datasetName); + + // Verify visualization switcher shows default viz type (indicates full page load) + await expect(explorePage.getVizSwitcher()).toBeVisible(); + await expect(explorePage.getVizSwitcher()).toContainText('Table'); + }); + + test('should delete a dataset with confirmation', async ({ page }) => { + // Get example dataset to duplicate + const originalName = TEST_DATASETS.EXAMPLE_DATASET; + const originalDataset = await getDatasetByName(page, originalName); + expect(originalDataset).not.toBeNull(); + + // Create throwaway copy for deletion (hermetic - uses UI duplication) + const datasetName = `test_delete_${Date.now()}`; + + // Verify original dataset is visible in list + await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible(); + + // Set up response intercept to capture duplicate dataset ID + const duplicateResponsePromise = page.waitForResponse( + response => + response.url().includes(`${ENDPOINTS.DATASET}duplicate`) && + response.status() === 201, + ); + + // Click duplicate action button + await datasetListPage.clickDuplicateAction(originalName); + + // Duplicate modal should appear and be ready for interaction + const duplicateModal = new DuplicateDatasetModal(page); + await duplicateModal.waitForReady(); + + // Fill in new dataset name + await duplicateModal.fillDatasetName(datasetName); + + // Click the Duplicate button + await duplicateModal.clickDuplicate(); + + // Get the duplicate dataset ID from response and track immediately + const duplicateResponse = await duplicateResponsePromise; + const duplicateData = await duplicateResponse.json(); + const duplicateId = duplicateData.id; + + // Track duplicate for cleanup immediately (before any operations that could fail) + testResources = { datasetIds: [duplicateId] }; + + // Modal should close + await duplicateModal.waitForHidden(); + + // Refresh page to see new dataset + await datasetListPage.goto(); + await datasetListPage.waitForTableLoad(); + Review Comment: Same - YAGNI for now. Only 2 occurrences doesn't justify a helper. ########## 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; +// Test credentials - can be overridden via environment variables +const adminUsername = process.env.PLAYWRIGHT_ADMIN_USERNAME || 'admin'; +const adminPassword = process.env.PLAYWRIGHT_ADMIN_PASSWORD || 'general'; - test.beforeEach(async ({ page }: any) => { - authPage = new AuthPage(page); - await authPage.goto(); - await authPage.waitForLoginForm(); - }); +/** + * 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('should redirect to login with incorrect username and password', async ({ - page, - }: any) => { - // Setup request interception before login attempt - const loginRequestPromise = authPage.waitForLoginRequest(); +let authPage: AuthPage; - // Attempt login with incorrect credentials - await authPage.loginWithCredentials('admin', 'wrongpassword'); +test.beforeEach(async ({ page }) => { + // Navigate to login page before each test (ensures clean state) + authPage = new AuthPage(page); + await authPage.goto(); + await authPage.waitForLoginForm(); +}); - // 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 redirect to login with incorrect username and password', async ({ + page, +}) => { + // Setup request interception before login attempt + const loginRequestPromise = authPage.waitForLoginRequest(); - // Wait for redirect to complete before checking URL - await page.waitForURL((url: any) => url.pathname.endsWith('login/'), { - timeout: 10000, - }); + // Attempt login with incorrect credentials (both username and password invalid) + await authPage.loginWithCredentials('wronguser', 'wrongpassword'); Review Comment: The test name and comment are now aligned - both indicate invalid username AND password. ########## superset-frontend/playwright/tests/experimental/dataset/dataset-list.spec.ts: ########## @@ -0,0 +1,243 @@ +/** + * 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, + ENDPOINTS, +} from '../../../helpers/api/dataset'; + +/** + * Test data constants + * These reference example datasets loaded via --load-examples in CI + */ +const TEST_DATASETS = { + EXAMPLE_DATASET: 'members_channels_2', +} as const; + +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(error => { + // Log cleanup failures to avoid silent resource leaks + console.warn( + `[Cleanup] Failed to delete dataset ${datasetId}:`, + String(error), + ); + }), + ); + } + 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 = TEST_DATASETS.EXAMPLE_DATASET; + const dataset = await getDatasetByName(page, datasetName); + expect(dataset).not.toBeNull(); + + // Verify dataset is visible in list (uses page object + Playwright auto-wait) + await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible(); + + // Click on dataset name to navigate to Explore + await datasetListPage.clickDatasetName(datasetName); + + // Wait for Explore page to load (validates URL + datasource control) + await explorePage.waitForPageLoad(); + + // Verify correct dataset is loaded in datasource control + const loadedDatasetName = await explorePage.getDatasetName(); + expect(loadedDatasetName).toContain(datasetName); + + // Verify visualization switcher shows default viz type (indicates full page load) + await expect(explorePage.getVizSwitcher()).toBeVisible(); + await expect(explorePage.getVizSwitcher()).toContainText('Table'); + }); + + test('should delete a dataset with confirmation', async ({ page }) => { + // Get example dataset to duplicate + const originalName = TEST_DATASETS.EXAMPLE_DATASET; + const originalDataset = await getDatasetByName(page, originalName); + expect(originalDataset).not.toBeNull(); + + // Create throwaway copy for deletion (hermetic - uses UI duplication) + const datasetName = `test_delete_${Date.now()}`; Review Comment: `Date.now()` has millisecond precision - parallel test collisions are practically impossible. Tests run in separate browser contexts anyway. -- 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]
