From f86e042f8155032a3030360db3cd8cd6281fe2a5 Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Fri, 11 Sep 2026 10:25:21 +0530 Subject: [PATCH 1/3] fix(playwright): select own pipelines in bulk re-deploy test TestSuitePipelineRedeploy selected the first two rows of the Data Observability grid, which lists every test-suite pipeline in the deployment ordered by name. Data Contract DQ pipelines are named with a bare UUID, so they sort ahead of the spec's own `pw-*` pipelines: the test was redeploying pipelines it does not own, and failed whenever one of those did not deploy (merge-queue run 34475633731). - select the spec's own pipelines by name via getRowByName - widen the page with ?pageSize=100 so both are always listed - key deploy statuses by pipeline id so a failure names the pipeline - add afterAll cleanup; the spec leaked both services into the shard - prune the now-stale no-positional-locator suppression - regenerate impact-map.generated.json Co-Authored-By: Claude Opus 5 --- .github/playwright/impact-map.generated.json | 1 + .../resources/ui/eslint-suppressions.json | 5 - .../TestSuitePipelineRedeploy.spec.ts | 91 +++++++++++++------ 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/.github/playwright/impact-map.generated.json b/.github/playwright/impact-map.generated.json index 30cf432dfec5..23aa93d3df1c 100644 --- a/.github/playwright/impact-map.generated.json +++ b/.github/playwright/impact-map.generated.json @@ -4079,6 +4079,7 @@ "specs": [ "playwright/e2e/Features/BulkEditEntity.spec.ts", "playwright/e2e/Features/OnlineUsers.spec.ts", + "playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts", "playwright/e2e/Pages/Teams.spec.ts" ] }, diff --git a/openmetadata-ui/src/main/resources/ui/eslint-suppressions.json b/openmetadata-ui/src/main/resources/ui/eslint-suppressions.json index 625f29270b5a..ae8cf9f9b140 100644 --- a/openmetadata-ui/src/main/resources/ui/eslint-suppressions.json +++ b/openmetadata-ui/src/main/resources/ui/eslint-suppressions.json @@ -518,11 +518,6 @@ "count": 3 } }, - "playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts": { - "om-playwright/no-positional-locator": { - "count": 1 - } - }, "playwright/e2e/Features/Topic.spec.ts": { "om-playwright/no-positional-locator": { "count": 3 diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts index 09036deb4b3c..a3b27200fcce 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TestSuitePipelineRedeploy.spec.ts @@ -19,6 +19,7 @@ import { redirectToHomePage, toastNotification, } from '../../utils/common'; +import { getRowByName } from '../../utils/scopedLocators'; import { settingClick } from '../../utils/sidebar'; // use the admin user to login @@ -27,6 +28,12 @@ test.use({ storageState: 'playwright/.auth/admin.json' }); const table1 = new TableClass(); const table2 = new TableClass(); +// The grid lists every test-suite pipeline in the deployment, not just this spec's, and the +// default page holds 15. Widen it so both pipelines below are always on the page the test +// reads -- the listing is name-ordered and pipelines named with a bare UUID (how +// DataContractRepository names a contract's DQ pipeline) sort ahead of every `pw-*` one. +const PIPELINE_PAGE_SIZE = 100; + test.describe('Bulk Re-Deploy pipelines ', PLAYWRIGHT_INGESTION_TAG_OBJ, () => { test.beforeAll('Setup pre-requests', async ({ browser }) => { const { afterAction, apiContext } = await createNewPage(browser); @@ -40,6 +47,17 @@ test.describe('Bulk Re-Deploy pipelines ', PLAYWRIGHT_INGESTION_TAG_OBJ, () => { await afterAction(); }); + // Without this the two services -- and the test-suite pipelines under them -- outlive the + // spec and stay in the shared Data Observability listing for every later test in the shard. + test.afterAll('Cleanup', async ({ browser }) => { + const { afterAction, apiContext } = await createNewPage(browser); + + await table1.delete(apiContext); + await table2.delete(apiContext); + + await afterAction(); + }); + test.beforeEach('Visit home page', async ({ page }) => { await redirectToHomePage(page); }); @@ -52,32 +70,36 @@ test.describe('Bulk Re-Deploy pipelines ', PLAYWRIGHT_INGESTION_TAG_OBJ, () => { test('Re-deploy all test-suite ingestion pipelines', async ({ page }) => { await settingClick(page, GlobalSettingOptions.DATA_OBSERVABILITY); + // usePaging seeds pageSize from the URL on first render, so widening the page is a + // navigation rather than a click through the (conditionally rendered) size selector. + const listUrl = new URL(page.url()); + listUrl.searchParams.set('pageSize', String(PIPELINE_PAGE_SIZE)); + await page.goto(listUrl.toString()); + await expect( page.getByRole('button', { name: 'Re Deploy' }) ).not.toBeEnabled(); await expect(page.getByTestId('ingestion-list-table')).toBeVisible(); - // beforeAll creates one test-suite pipeline per table, and there are two - // tables -- so this is the fixture's count, not an arbitrary number. One - // source for it, so the deploy assertion below cannot drift from the - // selection here. - const selectedPipelineCount = 2; - // TableV2 selection: the sr-only checkbox input is pointer-intercepted, so - // target the pressable label slot rather than the raw input. - const rowCheckboxes = page.locator('td label[slot="selection"]'); - - // The listing is global and can lag behind the pipelines this spec just - // created. Wait for enough rows first: nth() on a shorter list auto-waits - // and would spend the whole budget instead of saying what was missing. - await expect - .poll(() => rowCheckboxes.count(), { - message: `Wait for at least ${selectedPipelineCount} test-suite pipelines to be listed`, - timeout: 30_000, - }) - .toBeGreaterThanOrEqual(selectedPipelineCount); - - for (let index = 0; index < selectedPipelineCount; index++) { - await rowCheckboxes.nth(index).click(); + // Select this spec's own pipelines by name. Selecting by row position instead meant the + // test never touched them: the listing is global, so the top rows belong to whatever else + // exists in the deployment, and the assertion below then tracked a foreign pipeline whose + // deployability this spec does not control. + const pipelines = [ + table1.testSuitePipelineResponseData[0], + table2.testSuitePipelineResponseData[0], + ]; + + for (const pipeline of pipelines) { + const row = getRowByName(page, pipeline.name); + + // hasText is a substring match, so pin it to exactly one row before selecting it -- + // otherwise a near-miss silently selects the wrong pipeline, or several. + await expect(row).toHaveCount(1); + // TableV2 selection: the sr-only checkbox input is pointer-intercepted, so + // target the pressable label slot rather than the raw input. + await row.locator('label[slot="selection"]').click(); + await expect(row.getByRole('checkbox')).toBeChecked(); } await expect(page.getByRole('button', { name: 'Re Deploy' })).toBeEnabled(); @@ -88,13 +110,20 @@ test.describe('Bulk Re-Deploy pipelines ', PLAYWRIGHT_INGESTION_TAG_OBJ, () => { // toast instead, and the test then waits out its whole budget for a success // toast that can never arrive. Collect every deploy and report the real // status, so a genuine deploy failure fails fast and says why. - const deployStatuses: number[] = []; + // + // Keyed by pipeline id rather than pushed onto a list: an id says which pipeline failed + // straight from the assertion diff, and a deploy this test did not ask for cannot pad the + // count into passing. + const deployStatuses: Record = {}; const collectDeploy = (response: Response) => { - if ( - response.request().method() === 'POST' && - response.url().includes('/api/v1/services/ingestionPipelines/deploy') - ) { - deployStatuses.push(response.status()); + const deployedId = response + .url() + .match( + /\/api\/v1\/services\/ingestionPipelines\/deploy\/([^/?]+)/ + )?.[1]; + + if (response.request().method() === 'POST' && deployedId) { + deployStatuses[deployedId] = response.status(); } }; page.on('response', collectDeploy); @@ -103,16 +132,18 @@ test.describe('Bulk Re-Deploy pipelines ', PLAYWRIGHT_INGESTION_TAG_OBJ, () => { await page.getByRole('button', { name: 'Re Deploy' }).click(); await expect - .poll(() => deployStatuses.length, { + .poll(() => Object.keys(deployStatuses).length, { message: 'Wait for every selected pipeline to report a deploy result', timeout: 30_000, }) - .toBe(selectedPipelineCount); + .toBe(pipelines.length); expect( deployStatuses, 'every selected pipeline must deploy for the success toast to appear' - ).toEqual(Array(selectedPipelineCount).fill(200)); + ).toEqual( + Object.fromEntries(pipelines.map((pipeline) => [pipeline.id, 200])) + ); } finally { // Scope the listener to the action it observes: left attached it would // keep collecting for the page's lifetime, and a second test in this From 67606f01aa62f9ce667c736a79ca92e5c7925d27 Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Fri, 11 Sep 2026 11:54:33 +0530 Subject: [PATCH 2/3] fix(playwright): lower recorded no-positional-locator baseline to 1297 Pruning the TestSuitePipelineRedeploy suppression dropped the no-positional-locator total from 1298 to 1297, but corpus.test.mjs still recorded 1298, failing "the suppressions baseline matches its recorded state exactly" in UI Checkstyle. Co-Authored-By: Claude Opus 5 --- .../resources/ui/playwright/eslint-rules/tests/corpus.test.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/eslint-rules/tests/corpus.test.mjs b/openmetadata-ui/src/main/resources/ui/playwright/eslint-rules/tests/corpus.test.mjs index 24a9f4492826..b60c407dd602 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/eslint-rules/tests/corpus.test.mjs +++ b/openmetadata-ui/src/main/resources/ui/playwright/eslint-rules/tests/corpus.test.mjs @@ -41,7 +41,7 @@ test('the suppressions baseline matches its recorded state exactly', () => { // of the same rule in the same file stays invisible here. const EXPECTED = { 'om-playwright/justified-rule-disable': 12, - 'om-playwright/no-positional-locator': 1298, + 'om-playwright/no-positional-locator': 1297, 'om-playwright/require-assertion-per-test': 1, 'playwright/no-skipped-test': 4, 'playwright/no-wait-for-selector': 35, From c3dd36cf74c27499d0de15e8b81d3d7c20c9edea Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Fri, 11 Sep 2026 11:51:43 +0530 Subject: [PATCH 3/3] fix(playwright): retry entity-type selection in knowledge rule test The Add Rule drawer can close the entity-type popover right after it opens, while the match preview and filter builder are still re-rendering. The pending click on the glossaryTerm option then waited out the whole test budget for a listbox that never came back (merge-queue run 34521159069, both attempts). Use the existing selectOptionWithRetry helper, which reopens the popover and retries the option click, as the entity-type switch test in the same spec already does. Co-Authored-By: Claude Opus 5 --- .../e2e/Features/PersonaAIContextRules.spec.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/PersonaAIContextRules.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/PersonaAIContextRules.spec.ts index cc423d02056d..44f6c351a651 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/PersonaAIContextRules.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/PersonaAIContextRules.spec.ts @@ -452,14 +452,18 @@ test.describe.serial('Persona AI Context — Rule Builder', () => { await openAddRuleDrawer(page); await test.step('switch to a knowledge entity type', async () => { - await page.getByTestId('context-rule-entity-type').click(); + // The popover can close on its own right after opening, while the drawer + // is still settling (the match preview and filter builder re-render as + // their requests land). The pending option click then waits out the test + // budget for a listbox that never comes back. selectOptionWithRetry + // reopens the popover and retries, as the entity-type switch test does. // Each option carries its EntityType as data-key. Matching the // "Knowledge" supporting text instead would match all three knowledge // types and leave DOM order to decide which one the test exercises. - await page - .getByRole('listbox') - .locator('[data-key="glossaryTerm"]') - .click(); + await selectOptionWithRetry( + page.getByTestId('context-rule-entity-type'), + page.getByRole('listbox').locator('[data-key="glossaryTerm"]') + ); }); await test.step('Fully rendered switch must be checked and disabled', async () => {