Repository navigation
fix failing searchIndexApplication - #30577
Merged
Merged
Conversation
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
Contributor
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ ApprovedUpdates SearchIndexApplication.spec.ts to reuse the SUCCESSFUL_RUN_STATUS constant for run-status assertions, fixing flaky test failures caused by stale regex matching. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source |
Contributor
✅ Playwright Results — workflow succeededValidated commit ✅ 47 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 3 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 39m 34s ⏱️ Max setup 1m 32s · max shard execution 5m 46s · max shard-job elapsed before upload 9m 8s · reporting 3s 🌐 134.04 requests/attempt · 1.35 app boots/UI scenario · 0.00% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
shrabantipaul-collate
added
the
safe to test
shrabantipaul-collate
temporarily deployed
to
test
July 28, 2026 11:22 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
aniketkatkar97
approved these changes
Jul 28, 2026
shrabantipaul-collate
added a commit
that referenced
this pull request
Jul 29, 2026
Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
shrabantipaul-collate
added a commit
that referenced
this pull request
Jul 31, 2026
Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local> (cherry picked from commit 43df581)
1 task done
ShaileshParmar11
added a commit
that referenced
this pull request
Aug 1, 2026
…cation spec
SearchIndexApplication.spec.ts fails deterministically on both 1.13 AUT
runs (MySQL and PostgreSQL), 3/3 attempts:
ReferenceError: SUCCESSFUL_RUN_STATUS is not defined
at verifyLastExecutionRun (SearchIndexApplication.spec.ts:158:31)
214e5c5 cherry-picked #30577 onto 1.13, replacing the inline regex
/success|activeError/g with the identifier SUCCESSFUL_RUN_STATUS at two
call sites. That identifier is declared by #27971 (ed5e68c), which
`git branch -r --contains` places on main and 2.0 only -- not 1.13. The
pick applied cleanly because the surrounding context matched, and
introduced a guaranteed ReferenceError.
Declare the constant with the same value it carries on main and 2.0.
Verified there is no second undeclared identifier: SEARCH_INDEX_APP_NAME
exists on main but is never referenced by the 1.13 copy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
1 task done
ShaileshParmar11
added a commit
that referenced
this pull request
Aug 1, 2026
… fixtures, tour badge (#30787) * test(playwright): create beforeAll fixtures concurrently on 1.13 Two specs fail on the 1.13/MySQL AUT run with: "beforeAll" hook timeout of 60000ms exceeded Both hooks are pure API setup with no UI interaction, and both pass on every other branch/database -- this is latency accumulation under load, not a defect in the code under test. Pagination.spec.ts creates 20 users one at a time in a for loop, each awaited. CustomProperties.spec.ts does the same with 5 users on the entity_table path, which is exactly the case that failed, and with two topics on the entity_dashboard path. Twenty sequential round-trips exhaust a 60s budget on latency alone once the backend is loaded. Construct the instances first, then create them concurrently with Promise.all. This removes the accumulation rather than tolerating it -- deliberately not a timeout increase, which would only move the cliff. Each instance still generates its own uuid-suffixed name, so concurrent creation cannot collide. CustomProperties.spec.ts is describe.serial, so its hook failure also skipped 29 downstream tests; those execute again once the hook fits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): declare SUCCESSFUL_RUN_STATUS in the 1.13 SearchIndexApplication spec SearchIndexApplication.spec.ts fails deterministically on both 1.13 AUT runs (MySQL and PostgreSQL), 3/3 attempts: ReferenceError: SUCCESSFUL_RUN_STATUS is not defined at verifyLastExecutionRun (SearchIndexApplication.spec.ts:158:31) 214e5c5 cherry-picked #30577 onto 1.13, replacing the inline regex /success|activeError/g with the identifier SUCCESSFUL_RUN_STATUS at two call sites. That identifier is declared by #27971 (ed5e68c), which `git branch -r --contains` places on main and 2.0 only -- not 1.13. The pick applied cleanly because the surrounding context matched, and introduced a guaranteed ReferenceError. Declare the constant with the same value it carries on main and 2.0. Verified there is no second undeclared identifier: SEARCH_INDEX_APP_NAME exists on main but is never referenced by the 1.13 copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(playwright): assert the tour badge with a single web-first assertion Tour.spec.ts fails on 1.13/MySQL (flaky on 1.13/PostgreSQL) with TimeoutError: locator.waitFor: Timeout 10000ms exceeded waiting for locator('[data-tour-elem="badge"]') to be visible at expectTourBadge (Tour.spec.ts:48) The failing call is mid-tour (validateTourSteps line 82), not the first one -- waitForTourBadgeWithRetry has already established the badge by then. The badge re-renders on each step transition, and expectTourBadge gave that transition two independent budgets to lose against: a visibility waitFor, then a separate text poll, each capped at 10s -- the tightest timeout in any failing spec. Replace both with one toHaveText assertion. It auto-waits for the badge to attach *and* carry the expected step under a single budget, and reports the actual step on failure instead of "not visible". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(playwright): keep partially-created fixtures reachable for teardown Addresses review on #30787. With Promise.all, a rejection from any one create skips the users.push(...newUsers) that follows it, so users created successfully by the siblings never reach the afterAll loop that deletes them and leak into the environment. Use Promise.allSettled, register every fulfilled instance for teardown, then rethrow the first rejection. The hook still fails on a genuine create error -- nothing is swallowed -- but cleanup no longer depends on all five succeeding. Pagination.spec.ts needs no equivalent change: it has no afterAll for its users, before or after this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(playwright): register fixtures by persisted id, not promise outcome Addresses the second review round on #30787. UserClass.create() persists the user through /users/signup and sets responseData before it assigns roles, so a rejection from the role patch still leaves a real user behind. Registering only the fulfilled results therefore still leaked exactly the case allSettled was added to cover. Register every instance that came back with an id, regardless of how its promise settled, and rethrow the first rejection as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This branch was previously deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Describe your changes:
Fixes #
I worked on ... because ...
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Greptile Summary
This PR fixes SearchIndexApplication Playwright status assertions by:
SUCCESSFUL_RUN_STATUSconstant at both duplicated assertion sites.completedstatus.Confidence Score: 5/5
The PR appears safe to merge because the assertions now consistently recognize the backend’s valid completed status.
The changed test assertions reuse a non-global shared matcher that covers the established successful terminal statuses without altering production behavior.
Important Files Changed
Reviews (1): Last reviewed commit: "fix flaky searchIndexApplication" | Re-trigger Greptile