From c3a28727604b67c7351a4efbe1790e6f1958b663 Mon Sep 17 00:00:00 2001 From: Harsh Vador Date: Fri, 21 Aug 2026 15:53:25 +0530 Subject: [PATCH 1/2] Improvement: lint against ambiguous search/aggregate waits in Playwright MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A facet dropdown aggregates the same field twice — once when it opens (`value=.*`) and once per typed search. A wait naming only the endpoint or the field matches both, so it can resolve on the open request while the typed search is still in flight; the test then runs ahead of the request it queued. That is how #31859 stayed hidden behind a passing assertion. - playwright/utils/searchAggregation.ts: `waitForAggregation(page, { field, value, deleted })` matches on parsed URL params instead of a glob. `value` is required and takes `null` (or `''`) for the request fired on open, so the intent is stated at the call site. The search text is compared exactly after unwrapping the API's `.*text.*`, so a wait for `service` cannot resolve on an in-flight response for `service-name`; backslashes are normalised away because the API escapes ES reserved characters. - eslint-rules/openmetadata-playwright.mjs: `require-aggregation-wait-helper` flags raw waits on the endpoint — string, template literal or URL predicate — and follows an identifier to its declaration so hoisting the URL to a local const does not silence it. Registered at `warn` while the remaining 27 call sites are migrated, matching the convention used for the other aspirational Playwright rules. - Migrated the sites where the ambiguity is a live latent bug: ExploreDiscovery (4), utils/explore.ts (2), utils/glossary.ts, DataProductCertificationFilter. The explore-tree POST aggregate carries no field/value to discriminate on and is suppressed with a reason. Verified against a local stack: 45/46 of the affected specs pass; the one failure (EntitySummaryPanel display-name modal) contains no aggregation wait and passes in isolation. Co-Authored-By: Claude Opus 5 (1M context) --- .../eslint-rules/openmetadata-playwright.mjs | 90 +++++++++++++++ .../openmetadata-playwright.test.mjs | 106 ++++++++++++++++++ .../src/main/resources/ui/eslint.config.mjs | 10 ++ .../e2e/Flow/ExploreDiscovery.spec.ts | 44 ++++---- .../DataProductCertificationFilter.spec.ts | 10 +- .../resources/ui/playwright/utils/explore.ts | 20 ++-- .../resources/ui/playwright/utils/glossary.ts | 5 +- .../ui/playwright/utils/searchAggregation.ts | 78 +++++++++++++ 8 files changed, 325 insertions(+), 38 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs create mode 100644 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts diff --git a/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs new file mode 100644 index 000000000000..70efdd4fd98e --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs @@ -0,0 +1,90 @@ +/* + * Copyright 2026 Collate. + * Licensed 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. + */ + +const AGGREGATE_ENDPOINT = 'search/aggregate'; +const HELPER_MODULE = 'searchAggregation'; + +/** + * A wait naming only the endpoint or the field matches both aggregations a + * dropdown fires — the one on open and the typed search — so it can resolve on + * the wrong one (#31859). `waitForAggregation` requires the value that tells + * them apart. + */ +/** + * Source text of the matcher, following an identifier to its declaration in this + * file. A matcher built in another module is out of reach — ESLint sees one file + * at a time — so this narrows the gap rather than closing it. + */ +const resolveMatcherText = (argument, sourceCode) => { + if (argument.type !== 'Identifier') { + return sourceCode.getText(argument); + } + + const variable = sourceCode + .getScope(argument) + .references.find( + (reference) => reference.identifier === argument + )?.resolved; + + const initialisers = (variable?.defs ?? []) + .map((def) => def.node?.init) + .filter(Boolean); + + return initialisers.map((init) => sourceCode.getText(init)).join('\n'); +}; + +const requireAggregationWaitHelper = { + meta: { + messages: { + rawAggregationWait: + 'Use waitForAggregation from playwright/utils/searchAggregation instead of waiting on search/aggregate directly — a wait that names only the endpoint or field also matches the dropdown-open request and can resolve early.', + }, + schema: [], + type: 'problem', + }, + create(context) { + const { sourceCode } = context; + + if (context.filename.includes(HELPER_MODULE)) { + return {}; + } + + return { + CallExpression(node) { + const isWaitForResponse = + node.callee.type === 'MemberExpression' && + !node.callee.computed && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'waitForResponse'; + + if (!isWaitForResponse || node.arguments.length === 0) { + return; + } + + // The matcher may be a string, template literal or URL predicate, so + // match on source text rather than evaluating each form. + const matcherText = resolveMatcherText(node.arguments[0], sourceCode); + + if (matcherText.includes(AGGREGATE_ENDPOINT)) { + context.report({ node, messageId: 'rawAggregationWait' }); + } + }, + }; + }, +}; + +export default { + rules: { + 'require-aggregation-wait-helper': requireAggregationWaitHelper, + }, +}; diff --git a/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs new file mode 100644 index 000000000000..1e682d0f9267 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs @@ -0,0 +1,106 @@ +/* + * Copyright 2026 Collate. + * Licensed 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 assert from 'node:assert/strict'; +import test, { describe, it } from 'node:test'; +import { RuleTester } from 'eslint'; +import tseslint from 'typescript-eslint'; + +RuleTester.describe = describe; +RuleTester.it = it; + +const playwrightPlugin = (await import('./openmetadata-playwright.mjs')) + .default; + +test('exports the aggregation wait helper rule', () => { + assert.ok(playwrightPlugin.rules['require-aggregation-wait-helper']); +}); + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 'latest', + parser: tseslint.parser, + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + sourceType: 'module', + }, +}); + +ruleTester.run( + 'require-aggregation-wait-helper', + playwrightPlugin.rules['require-aggregation-wait-helper'], + { + valid: [ + { + code: "const res = waitForAggregation(page, { field: 'domains.displayName.keyword', value: 'sales' });", + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + code: "const res = page.waitForResponse('/api/v1/search/query?*deleted=true*');", + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + code: "const res = page.waitForResponse((response) => response.url().includes('/api/v1/tables'));", + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + // The helper itself owns the only raw wait on the endpoint. + code: "const res = page.waitForResponse((response) => response.url().includes('/api/v1/search/aggregate'));", + filename: 'playwright/utils/searchAggregation.ts', + }, + { + code: ` + const queryUrl = '/api/v1/search/query?*index=dataAsset*'; + const res = page.waitForResponse(queryUrl); + `, + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + ], + invalid: [ + { + code: "const res = page.waitForResponse('/api/v1/search/aggregate?*');", + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + code: 'const res = page.waitForResponse(`/api/v1/search/aggregate?index=dataAsset&field=${field}*`);', + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + code: "const res = page.waitForResponse((response) => response.url().includes('/api/v1/search/aggregate') && response.url().includes(field));", + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/utils/glossary.ts', + }, + { + // Hoisting the URL to a local const is the realistic accidental evasion. + code: ` + const aggregateUrl = '/api/v1/search/aggregate?*'; + const res = page.waitForResponse(aggregateUrl); + `, + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + code: ` + const aggregateUrl = \`/api/v1/search/aggregate?index=dataAsset&field=\${field}*\`; + const res = page.waitForResponse(aggregateUrl); + `, + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/utils/explore.ts', + }, + ], + } +); diff --git a/openmetadata-ui/src/main/resources/ui/eslint.config.mjs b/openmetadata-ui/src/main/resources/ui/eslint.config.mjs index 67e94969ccf4..4dc77a9ebb67 100644 --- a/openmetadata-ui/src/main/resources/ui/eslint.config.mjs +++ b/openmetadata-ui/src/main/resources/ui/eslint.config.mjs @@ -26,6 +26,7 @@ import jsoncParser from 'jsonc-eslint-parser'; import tseslint from 'typescript-eslint'; import openMetadataImports from './eslint-rules/openmetadata-imports.mjs'; import openMetadataPerformance from './eslint-rules/openmetadata-performance.mjs'; +import openMetadataPlaywright from './eslint-rules/openmetadata-playwright.mjs'; export default [ // Base recommended configs @@ -428,6 +429,7 @@ export default [ { files: ['**/playwright/**/*.{js,jsx,ts,tsx}'], plugins: { + 'openmetadata-playwright': openMetadataPlaywright, playwright, }, rules: { @@ -491,6 +493,14 @@ export default [ 'playwright/no-page-pause': 'error', 'playwright/no-focused-test': 'error', + // A facet aggregation wait must name the value it is waiting for, not just + // the endpoint or field: a dropdown fires one aggregation when it opens and + // one per typed search, so a wait that names neither can resolve off the + // wrong one and run the test ahead of the request it queued (#31859). Warn + // rather than error while the remaining 27 call sites are migrated to + // playwright/utils/searchAggregation.ts. + 'openmetadata-playwright/require-aggregation-wait-helper': 'warn', + // Playwright rules — aspirational (warn): existing violations to fix over time 'playwright/missing-playwright-await': 'warn', 'playwright/valid-expect': 'warn', diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ExploreDiscovery.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ExploreDiscovery.spec.ts index 29de7bce146b..082b3abc747a 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ExploreDiscovery.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/ExploreDiscovery.spec.ts @@ -22,6 +22,7 @@ import { } from '../../utils/entity'; import { clickUpdateButtonIfVisible } from '../../utils/explore'; import { getJsonTreeObject } from '../../utils/exploreDiscovery'; +import { waitForAggregation } from '../../utils/searchAggregation'; import { sidebarClick } from '../../utils/sidebar'; // use the admin user to login @@ -257,13 +258,11 @@ test.describe('Explore Assets Discovery', () => { // The user should not be visible in the owners filter when the deleted switch is off await page.click('[data-testid="search-dropdown-Owners"]'); - // Match on the typed value: opening the dropdown fires its own aggregation - // for the same field, and a glob matching both races ahead of the search. - const searchResOwner = page.waitForResponse( - `/api/v1/search/aggregate?index=dataAsset&field=ownerDisplayName&value=*${encodeURIComponent( - user.responseData.displayName - )}*deleted=false*` - ); + const searchResOwner = waitForAggregation(page, { + deleted: false, + field: 'ownerDisplayName', + value: user.responseData.displayName, + }); await page.fill( '[data-testid="search-input"]', @@ -284,11 +283,11 @@ test.describe('Explore Assets Discovery', () => { // The domain should not be visible in the domains filter when the deleted switch is off await page.click('[data-testid="search-dropdown-Domains"]'); - const searchResDomain = page.waitForResponse( - `/api/v1/search/aggregate?index=dataAsset&field=domains.displayName.keyword&value=*${encodeURIComponent( - domain.responseData.displayName - )}*deleted=false*` - ); + const searchResDomain = waitForAggregation(page, { + deleted: false, + field: 'domains.displayName.keyword', + value: domain.responseData.displayName, + }); await page.fill( '[data-testid="search-input"]', @@ -323,12 +322,11 @@ test.describe('Explore Assets Discovery', () => { const ownerSearchText = user.responseData.displayName.toLowerCase(); await page.click('[data-testid="search-dropdown-Owners"]'); - // Match on the typed value, not the dropdown's own initial aggregation. - const searchResOwner = page.waitForResponse( - `/api/v1/search/aggregate?index=dataAsset&field=ownerDisplayName&value=*${encodeURIComponent( - ownerSearchText - )}*deleted=true*` - ); + const searchResOwner = waitForAggregation(page, { + deleted: true, + field: 'ownerDisplayName', + value: ownerSearchText, + }); await page.fill('[data-testid="search-input"]', ownerSearchText); await searchResOwner; @@ -364,11 +362,11 @@ test.describe('Explore Assets Discovery', () => { const domainSearchText = domain.responseData.displayName.toLowerCase(); await page.click('[data-testid="search-dropdown-Domains"]'); - const searchResDomain = page.waitForResponse( - `/api/v1/search/aggregate?index=dataAsset&field=domains.displayName.keyword&value=*${encodeURIComponent( - domainSearchText - )}*deleted=true*` - ); + const searchResDomain = waitForAggregation(page, { + deleted: true, + field: 'domains.displayName.keyword', + value: domainSearchText, + }); await page.fill('[data-testid="search-input"]', domainSearchText); await searchResDomain; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataProductCertificationFilter.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataProductCertificationFilter.spec.ts index 35bc76b262c7..00d173f4c1b5 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataProductCertificationFilter.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataProductCertificationFilter.spec.ts @@ -19,6 +19,7 @@ import { TagClass } from '../../support/tag/TagClass'; import { createNewPage, redirectToHomePage } from '../../utils/common'; import { waitForAllLoadersToDisappear } from '../../utils/entity'; import { clickUpdateButtonIfVisible } from '../../utils/explore'; +import { waitForAggregation } from '../../utils/searchAggregation'; import { sidebarClick } from '../../utils/sidebar'; test.use({ storageState: 'playwright/.auth/admin.json' }); @@ -78,11 +79,10 @@ const resolveCertificationOptionKey = async ( await menu.waitFor({ state: 'visible' }); } - const aggregateResponse = page.waitForResponse( - (response) => - response.url().includes('/api/v1/search/aggregate') && - response.url().includes(CERTIFICATION_FIELD) - ); + const aggregateResponse = waitForAggregation(page, { + field: CERTIFICATION_FIELD, + value: searchText, + }); await menu.getByTestId('search-input').fill(searchText); const body = await (await aggregateResponse).json(); const buckets: Array<{ key: string }> = diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/explore.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/explore.ts index 227124f4b102..ec5d955d6bef 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/explore.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/explore.ts @@ -18,6 +18,7 @@ import { TableClass } from '../support/entity/TableClass'; import { getApiContext, redirectToExplorePage } from './common'; import { waitForAllLoadersToDisappear } from './entity'; import { openEntitySummaryPanel } from './entityPanel'; +import { waitForAggregation } from './searchAggregation'; export interface Bucket { key: string; @@ -42,10 +43,11 @@ export const searchAndClickOnOption = async ( checkedAfterClick: boolean ) => { let testId = (filter.value ?? '').toLowerCase(); - // Filtering for tiers is done on client side, so no API call will be triggered - const searchRes = page.waitForResponse( - `/api/v1/search/aggregate?index=dataAsset&field=${filter.key}**` - ); + + const searchRes = waitForAggregation(page, { + field: filter.key, + value: filter.value ?? null, + }); await page.fill('[data-testid="search-input"]', filter.value ?? ''); await searchRes; @@ -143,9 +145,10 @@ export const selectDataAssetFilter = async ( '/api/v1/search/query?*index=dataAsset&from=0&size=0*' ); await page.getByRole('button', { name: 'Data Assets' }).click(); - const dataAssetDropdownRequest = page.waitForResponse( - '/api/v1/search/aggregate?index=dataAsset&field=entityType.keyword*' - ); + const dataAssetDropdownRequest = waitForAggregation(page, { + field: 'entityType.keyword', + value: filterValue, + }); await page .getByTestId('drop-down-menu') .getByTestId('search-input') @@ -198,6 +201,9 @@ export const expandServiceInExploreTree = async ( // Expanding the serviceType groups its services. The service drill-down // goes through the aggregate API (POST /search/aggregate) so the buckets // carry service.style top hits for custom service icons. + // Not a facet dropdown: the tree drill-down is a POST aggregate with no + // field/value pair for waitForAggregation to discriminate on. + // eslint-disable-next-line openmetadata-playwright/require-aggregation-wait-helper const serviceNameRes = page.waitForResponse( (response) => response.url().endsWith('/api/v1/search/aggregate') && diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts index 7f5b25a80259..f2f385e15882 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts @@ -50,6 +50,7 @@ import { getEntityDisplayName, waitForAllLoadersToDisappear, } from './entity'; +import { waitForAggregation } from './searchAggregation'; import { sidebarClick } from './sidebar'; import { TaskDetails, @@ -940,9 +941,7 @@ const testFilterWithSpecificOption = async ( await page.getByTestId('drop-down-menu').waitFor(); if (searchText) { - const aggregateResponse = page.waitForResponse( - '/api/v1/search/aggregate?*' - ); + const aggregateResponse = waitForAggregation(page, { value: searchText }); await page .getByRole('textbox', { name: 'Search Service Type...' }) .fill(searchText); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts new file mode 100644 index 000000000000..19b7c8ebb7a3 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts @@ -0,0 +1,78 @@ +/* + * Copyright 2026 Collate. + * Licensed 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, Response } from '@playwright/test'; + +const AGGREGATE_PATH = '/api/v1/search/aggregate'; + +/** + * A dropdown aggregates the same field twice — once on open, once per typed + * search — so `value` is what tells the two apart and is therefore required. + */ +export type AggregationWait = { + /** Aggregated field, e.g. `domains.displayName.keyword`. Omit to match any. */ + field?: string; + /** Typed search text; `null` or `''` for the request fired on open. */ + value: string | null; + /** Match only when the request carries this `deleted` flag. */ + deleted?: boolean; +}; + +const WRAPPED_SEARCH_TEXT = /^\.\*(.*)\.\*$/; + +// The API escapes ES reserved characters, so `service-name` arrives as +// `service\-name`; dropping the backslashes keeps those values comparable. +const normalize = (text: string): string => + text.toLowerCase().replace(/\\/g, ''); + +const matches = (response: Response, wait: AggregationWait): boolean => { + const url = new URL(response.url()); + + if (!url.pathname.endsWith(AGGREGATE_PATH)) { + return false; + } + + const params = url.searchParams; + + if (wait.field && params.get('field') !== wait.field) { + return false; + } + + if ( + wait.deleted !== undefined && + params.get('deleted') !== String(wait.deleted) + ) { + return false; + } + + const value = params.get('value'); + + // The API sends `.*` when there is no search text, which is the open request. + if (!wait.value) { + return value === null || value === '.*'; + } + + const searchText = value?.match(WRAPPED_SEARCH_TEXT)?.[1]; + + // Exact, not substring: a wait for `service` must not resolve on an in-flight + // response for `service-name`. + if (searchText !== undefined) { + return normalize(searchText) === normalize(wait.value); + } + + // Unwrapped value: outside the documented shape, so stay permissive. + return normalize(value ?? '').includes(normalize(wait.value)); +}; + +/** Arm before the action that triggers the request, as with `waitForResponse`. */ +export const waitForAggregation = (page: Page, wait: AggregationWait) => + page.waitForResponse((response) => matches(response, wait)); From 2cc0b7f804531440bd030e7c9f51796f6922e94f Mon Sep 17 00:00:00 2001 From: Harsh Vador Date: Fri, 21 Aug 2026 16:55:50 +0530 Subject: [PATCH 2/2] Address review: widen the rule's matcher resolution, unescape rather than strip - resolveMatcherText now walks the scope chain via the same findVariable helper openmetadata-performance.mjs uses, and reads later assignments as well as declarations, so a matcher declared at module scope or assigned to a `let` no longer escapes the rule. Quotes and `+` are dropped before the endpoint check so a path split across concatenated literals still matches. - searchAggregation compares against an unescaped URL value instead of stripping backslashes from both sides, which kept `foo\bar` and `foobar` distinct rather than collapsing them onto each other. - Restores the rule's own doc block, which an earlier comment trim had left sitting above resolveMatcherText. Rule tests cover the module-scope, reassigned-let and concatenated forms: 99 pass. The affected specs still pass against a local stack (27/27). Co-Authored-By: Claude Opus 5 (1M context) --- .../eslint-rules/openmetadata-playwright.mjs | 62 ++++++++++++------- .../openmetadata-playwright.test.mjs | 27 ++++++++ .../ui/playwright/utils/searchAggregation.ts | 11 ++-- 3 files changed, 74 insertions(+), 26 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs index 70efdd4fd98e..2c08455e48c7 100644 --- a/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs +++ b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs @@ -14,35 +14,50 @@ const AGGREGATE_ENDPOINT = 'search/aggregate'; const HELPER_MODULE = 'searchAggregation'; +/** Mirrors the resolver in openmetadata-performance.mjs. */ +const findVariable = (sourceCode, identifier) => { + let scope = sourceCode.getScope(identifier); + + while (scope) { + const variable = scope.set.get(identifier.name); + + if (variable) { + return variable; + } + + scope = scope.upper; + } + + return null; +}; + /** - * A wait naming only the endpoint or the field matches both aggregations a - * dropdown fires — the one on open and the typed search — so it can resolve on - * the wrong one (#31859). `waitForAggregation` requires the value that tells - * them apart. - */ -/** - * Source text of the matcher, following an identifier to its declaration in this - * file. A matcher built in another module is out of reach — ESLint sees one file - * at a time — so this narrows the gap rather than closing it. + * Source text of the matcher, following an identifier to every value assigned to + * it in this file — declaration or later assignment, at any scope. A matcher + * built in another module is out of reach, since ESLint sees one file at a time. */ const resolveMatcherText = (argument, sourceCode) => { if (argument.type !== 'Identifier') { return sourceCode.getText(argument); } - const variable = sourceCode - .getScope(argument) - .references.find( - (reference) => reference.identifier === argument - )?.resolved; + const variable = findVariable(sourceCode, argument); + const assigned = [ + ...(variable?.defs ?? []).map((def) => def.node?.init), + ...(variable?.references ?? []) + .filter((reference) => reference.writeExpr) + .map((reference) => reference.writeExpr), + ].filter(Boolean); - const initialisers = (variable?.defs ?? []) - .map((def) => def.node?.init) - .filter(Boolean); - - return initialisers.map((init) => sourceCode.getText(init)).join('\n'); + return assigned.map((node) => sourceCode.getText(node)).join('\n'); }; +/** + * A wait naming only the endpoint or the field matches both aggregations a + * dropdown fires — the one on open and the typed search — so it can resolve on + * the wrong one (#31859). `waitForAggregation` requires the value that tells + * them apart. + */ const requireAggregationWaitHelper = { meta: { messages: { @@ -72,8 +87,13 @@ const requireAggregationWaitHelper = { } // The matcher may be a string, template literal or URL predicate, so - // match on source text rather than evaluating each form. - const matcherText = resolveMatcherText(node.arguments[0], sourceCode); + // match on source text rather than evaluating each form. Quotes and + // concatenation come out first so a path split across literals still + // reads as one string. + const matcherText = resolveMatcherText( + node.arguments[0], + sourceCode + ).replace(/['"`+\s]/g, ''); if (matcherText.includes(AGGREGATE_ENDPOINT)) { context.report({ node, messageId: 'rawAggregationWait' }); diff --git a/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs index 1e682d0f9267..5acb45ee70e2 100644 --- a/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs +++ b/openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs @@ -101,6 +101,33 @@ ruleTester.run( errors: [{ messageId: 'rawAggregationWait' }], filename: 'playwright/utils/explore.ts', }, + { + // Declared at module scope, used inside a test callback. + code: ` + const aggregateUrl = '/api/v1/search/aggregate?*'; + test('example', async ({ page }) => { + const res = page.waitForResponse(aggregateUrl); + }); + `, + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + // Assigned after declaration, so the variable has no initialiser. + code: ` + let aggregateUrl; + aggregateUrl = '/api/v1/search/aggregate?*'; + const res = page.waitForResponse(aggregateUrl); + `, + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, + { + // Path split across concatenated literals. + code: "const res = page.waitForResponse('/api/v1/search/' + 'aggregate?*');", + errors: [{ messageId: 'rawAggregationWait' }], + filename: 'playwright/e2e/Flow/Example.spec.ts', + }, ], } ); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts index 19b7c8ebb7a3..216b7ed45fc2 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/searchAggregation.ts @@ -30,9 +30,10 @@ export type AggregationWait = { const WRAPPED_SEARCH_TEXT = /^\.\*(.*)\.\*$/; // The API escapes ES reserved characters, so `service-name` arrives as -// `service\-name`; dropping the backslashes keeps those values comparable. -const normalize = (text: string): string => - text.toLowerCase().replace(/\\/g, ''); +// `service\-name`. Unescaping (rather than stripping backslashes from both +// sides) keeps `foo\bar` distinguishable from `foobar`. +const unescapeReserved = (text: string): string => + text.replace(/\\(.)/g, '$1').toLowerCase(); const matches = (response: Response, wait: AggregationWait): boolean => { const url = new URL(response.url()); @@ -66,11 +67,11 @@ const matches = (response: Response, wait: AggregationWait): boolean => { // Exact, not substring: a wait for `service` must not resolve on an in-flight // response for `service-name`. if (searchText !== undefined) { - return normalize(searchText) === normalize(wait.value); + return unescapeReserved(searchText) === wait.value.toLowerCase(); } // Unwrapped value: outside the documented shape, so stay permissive. - return normalize(value ?? '').includes(normalize(wait.value)); + return unescapeReserved(value ?? '').includes(wait.value.toLowerCase()); }; /** Arm before the action that triggers the request, as with `waitForResponse`. */