-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fixes 4065: TestCase reindex preserves testCaseStatus + lenient time-series reads #28061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
mohityadav766
wants to merge
17
commits into
main
Choose a base branch
from
fix-deleted-field
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
317d324
Entity Time Series
mohityadav766 2c2eb41
test case results
mohityadav766 dde647e
Add Playwright
mohityadav766 382198c
Merge branch 'main' into fix-deleted-field
mohityadav766 082f1e9
fix: declare 'summary' on TestSuiteIndex so reindex preserves lastRes…
mohityadav766 eb75c50
Add tag/tier/classification/Tags/glossaryTags
mohityadav766 af7ee8e
Merge branch 'main' into fix-deleted-field
mohityadav766 e8faf2c
Revert "Add tag/tier/classification/Tags/glossaryTags"
mohityadav766 d8a9586
Fix for reindex since recreate is removed
mohityadav766 5bdb130
fix(testCase): keep tier tag in `tags` array for index parity with pr…
mohityadav766 e300646
chore: untrack accidentally-committed empty liveUpdate.txt / withRein…
mohityadav766 4487946
chore: actually remove liveUpdate.txt / withReindex.txt from the repo
mohityadav766 b6b88dd
Spotless
mohityadav766 da9f0d2
Revert to add all entity tags
mohityadav766 084c5c2
Fix Stats false status in case of empty stats
mohityadav766 1bbb849
Add Fields back to API, Worksheet and Topic
mohityadav766 1d80086
test: align finalizeAllEntityReindex missing-stats expectation with 0…
mohityadav766 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
122 changes: 122 additions & 0 deletions
122
.../main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseStatusAfterReindex.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| /** | ||
| * Regression for the 1.12.7 selective-reindex bug | ||
| * (https://github.com/open-metadata/OpenMetadata/pull/27723): | ||
| * TestCaseIndex.getRequiredReindexFields() omitted `testCaseResult` and | ||
| * `incidentId`, both of which are stripped from the storage JSON. On reindex, | ||
| * TestCaseRepository.setFieldsInBulk skipped fetching them and the resulting | ||
| * ES doc had no `testCaseStatus` — wiping status from search/UI until a | ||
| * per-case write re-populated it. | ||
| * | ||
| * This test creates a test case, writes a result, forces an entity reindex | ||
| * with `recreate=true` (delete + re-add), and asserts the status survives. | ||
| */ | ||
|
|
||
| import test, { expect } from '@playwright/test'; | ||
| import { TableClass } from '../../../support/entity/TableClass'; | ||
| import { createNewPage } from '../../../utils/common'; | ||
|
|
||
| test.use({ storageState: 'playwright/.auth/admin.json' }); | ||
|
|
||
| const TEST_CASE_STATUS = 'Failed' as const; | ||
|
|
||
| test('Test case status survives a full entity reindex', async ({ browser }) => { | ||
| const { apiContext, afterAction } = await createNewPage(browser); | ||
|
|
||
| const table = new TableClass(); | ||
|
|
||
| try { | ||
| await table.create(apiContext); | ||
|
|
||
| const testCase = await table.createTestCase(apiContext); | ||
| const testCaseFqn = testCase.fullyQualifiedName as string; | ||
| const testCaseId = testCase.id as string; | ||
|
|
||
| await table.addTestCaseResult(apiContext, testCaseFqn, { | ||
| result: 'Reindex regression check', | ||
| testCaseStatus: TEST_CASE_STATUS, | ||
| timestamp: Date.now(), | ||
| }); | ||
|
|
||
| // Wait for the search doc to settle and assert the status is indexed. | ||
| await expect | ||
| .poll( | ||
| async () => { | ||
| const res = await apiContext.get( | ||
| `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( | ||
| testCaseFqn | ||
| )}%22&index=test_case_search_index` | ||
| ); | ||
| if (res.status() !== 200) { | ||
| return undefined; | ||
| } | ||
| const body = await res.json(); | ||
| const hits = body?.hits?.hits ?? []; | ||
| return hits[0]?._source?.testCaseResult?.testCaseStatus; | ||
| }, | ||
| { | ||
| message: | ||
| 'pre-reindex: test case search doc must include testCaseResult.testCaseStatus', | ||
| timeout: 30_000, | ||
| } | ||
|
mohityadav766 marked this conversation as resolved.
|
||
| ) | ||
| .toBe(TEST_CASE_STATUS); | ||
|
|
||
| // Force a recreate-style reindex of the test case — this is the exact | ||
| // path that drops the status before the fix. | ||
| const reindexRes = await apiContext.post( | ||
| '/api/v1/search/reindexEntities?recreate=true', | ||
| { | ||
| data: [ | ||
| { | ||
| id: testCaseId, | ||
| type: 'testCase', | ||
| fullyQualifiedName: testCaseFqn, | ||
| }, | ||
| ], | ||
| } | ||
| ); | ||
|
|
||
| expect(reindexRes.status()).toBeLessThan(400); | ||
|
Comment on lines
+79
to
+92
|
||
|
|
||
|
mohityadav766 marked this conversation as resolved.
|
||
| // Assert the status is still there after reindex. Before the fix, the | ||
| // recreated doc had no testCaseResult and this poll would time out. | ||
| await expect | ||
| .poll( | ||
| async () => { | ||
| const res = await apiContext.get( | ||
| `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( | ||
| testCaseFqn | ||
| )}%22&index=test_case_search_index` | ||
| ); | ||
| if (res.status() !== 200) { | ||
| return undefined; | ||
| } | ||
| const body = await res.json(); | ||
| const hits = body?.hits?.hits ?? []; | ||
| return hits[0]?._source?.testCaseResult?.testCaseStatus; | ||
| }, | ||
| { | ||
| message: | ||
| 'post-reindex: test case search doc must still include testCaseResult.testCaseStatus', | ||
|
mohityadav766 marked this conversation as resolved.
|
||
| timeout: 30_000, | ||
| } | ||
| ) | ||
| .toBe(TEST_CASE_STATUS); | ||
| } finally { | ||
| await table.delete(apiContext); | ||
| await afterAction(); | ||
| } | ||
|
gitar-bot[bot] marked this conversation as resolved.
|
||
| }); | ||
106 changes: 106 additions & 0 deletions
106
...ain/resources/ui/playwright/e2e/Features/DataQuality/TestSuiteSummaryAfterReindex.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. | ||
| */ | ||
|
|
||
| /** | ||
| * Regression for the selective-reindex refactor (PR 27723): | ||
| * | ||
| * TestSuiteIndex.buildSearchIndexDocInternal computes `lastResultTimestamp` | ||
| * from `testSuite.getTestCaseResultSummary()`. TestSuiteRepository registers | ||
| * a fetcher for that under the field name `"summary"`. The reindex path only | ||
| * runs fetchers whose field is in `getRequiredReindexFields()`. Without | ||
| * `"summary"` declared, the fetcher does not run, the Index falls through to | ||
| * `doc.put("lastResultTimestamp", 0L)` (TestSuiteIndex.java:41), and the DQ | ||
| * `/data-quality/test-suites` list page — which sorts by that exact field | ||
| * (`TestSuites.component.tsx:175`) — collapses every reindexed suite to the | ||
| * 1970 epoch, breaking "most recently run first" ordering. | ||
| * | ||
| * The test creates a basic suite, writes a result, asserts the field is | ||
| * non-zero on the live-write path, forces a `recreate=true` reindex of the | ||
| * testSuite, and asserts the field is still non-zero. Before the fix this | ||
| * dropped back to 0 — provably the data the UI sort depends on. | ||
| */ | ||
|
mohityadav766 marked this conversation as resolved.
|
||
|
|
||
| import test, { expect } from '@playwright/test'; | ||
| import { TableClass } from '../../../support/entity/TableClass'; | ||
| import { createNewPage } from '../../../utils/common'; | ||
|
|
||
| test.use({ storageState: 'playwright/.auth/admin.json' }); | ||
|
|
||
| test('Test suite lastResultTimestamp survives a full entity reindex', async ({ | ||
| browser, | ||
| }) => { | ||
| const { apiContext, afterAction } = await createNewPage(browser); | ||
|
|
||
| const table = new TableClass(); | ||
|
|
||
| try { | ||
| await table.create(apiContext); | ||
| const testCase = await table.createTestCase(apiContext); | ||
|
|
||
| const resultTimestamp = Date.now(); | ||
| await table.addTestCaseResult(apiContext, testCase.fullyQualifiedName, { | ||
| result: 'Reindex regression check', | ||
| testCaseStatus: 'Success', | ||
| timestamp: resultTimestamp, | ||
| }); | ||
|
|
||
| const suite = testCase.testSuite as { | ||
| id: string; | ||
| fullyQualifiedName: string; | ||
| }; | ||
|
|
||
| const reindexRes = await apiContext.post( | ||
| '/api/v1/search/reindexEntities?recreate=true', | ||
| { | ||
| data: [ | ||
| { | ||
| fullyQualifiedName: suite.fullyQualifiedName, | ||
| id: suite.id, | ||
| type: 'testSuite', | ||
| }, | ||
| ], | ||
| } | ||
| ); | ||
|
|
||
| expect(reindexRes.status()).toBeLessThan(400); | ||
|
mohityadav766 marked this conversation as resolved.
Comment on lines
+62
to
+75
|
||
|
|
||
| // Before the fix, the recreated doc has lastResultTimestamp=0 because the | ||
| // "summary" fetcher never ran and the Index hit its 0L fallback branch. | ||
| // After the fix it is the millisecond timestamp of the most recent result. | ||
| await expect | ||
| .poll( | ||
| async () => { | ||
| const res = await apiContext.get( | ||
| `/api/v1/search/query?q=fullyQualifiedName:%22${encodeURIComponent( | ||
| suite.fullyQualifiedName | ||
| )}%22&index=test_suite_search_index` | ||
| ); | ||
| if (res.status() !== 200) { | ||
| return 0; | ||
| } | ||
| const body = await res.json(); | ||
|
|
||
| return body?.hits?.hits?.[0]?._source?.lastResultTimestamp ?? 0; | ||
| }, | ||
| { | ||
| message: | ||
| 'post-reindex: test suite doc must still include a non-zero lastResultTimestamp', | ||
| timeout: 30_000, | ||
| } | ||
| ) | ||
| .toBeGreaterThan(0); | ||
| } finally { | ||
| await table.delete(apiContext); | ||
| await afterAction(); | ||
| } | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.