test(playwright): E2E coverage for Persona AI Context rule builder - #31581
test(playwright): E2E coverage for Persona AI Context rule builder#31581anuj-kumary wants to merge 4 commits into
Conversation
Adds 11 Playwright tests covering the full rule CRUD flow, filter validation, and individual field behaviours for the AI Context tab in Persona settings. The most important addition is a regression test for #31564: before PR #31565 a fully-completed single-value condition (e.g. Description Contains "…") was incorrectly rejected with the "unfinished condition" error because `elasticSearchFormat` passed raw `config` instead of `extendedConfig` to `buildEsRule`, causing widget resolution to fail and `hasUnfinishedRule` to return true. Tests added: - Rule CRUD serial group: empty state, create, edit, delete - Filter validation serial group: incomplete condition blocks save, regression #31564 (Description Contains allows save), entity-type switch clears filter, duplicate name rejected - Rule editor fields: max-assets clamping, toggle visibility, knowledge entity-type forces Fully rendered on Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
❌ 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 |
|
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! |
anuj-kumary
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…ion step
Live browser testing revealed two issues with the Persona AI Context
regression spec:
1. `page.locator('.rule').first()` finds the GROUP container which can
be hidden in some builds — go directly to `.rule--field .ant-select`
at page level instead.
2. The Description field uses `match_phrase` operators. The text widget
(`.rule--widget--TEXT`) only renders after an operator is selected.
Add an explicit "Contains" operator selection step so the input is
available before we try to fill it.
Also: add `.first()` guard on the Fully rendered switch locator in the
knowledge-entity-type test (multiple elements can match) and add a note
that react-aria Switch stores state in the checked property, not
aria-checked.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three issues raised in PR #31581 review: 1. `openAddRuleDrawer`: `isVisible({ timeout: 500 })` silently ignores the timeout and returns synchronously — replace with `emptyBtn.or(headerBtn).first().waitFor()` so we genuinely wait, then branch on `isVisible()` after the element is confirmed present. 2. `waitForTimeout(300)` sleeps after clicking add-context-condition are flaky: replaced with `page.locator('.rule--field .ant-select').first() .waitFor({ state: 'visible' })` at all three call sites. 3. Comment on the Fully rendered switch assertion said "use .isChecked()" but the code uses Playwright's `toBeChecked()` matcher — reworded to accurately describe why the property-based check is needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🔴 Playwright Results — workflow failedValidated commit ✅ 0 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky Pipeline and setup failures (5)
Performance⚪ Performance metrics unavailable; see the CI and reporting failures above.
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
…erial from filter validation CRUD tests (empty→create→edit→delete) are an inherently sequential flow where edit and delete depend on state created by earlier steps. Collapsing them into a single test with test.step() makes the dependency explicit and atomic: a step failure aborts only that test rather than silently skipping later ones via describe.serial. Filter validation tests are each self-contained (each creates and discards its own rule within a single test body), so describe.serial was unnecessary. Removing it ensures a failure in the "incomplete condition" test never prevents the regression test (#31564) from running. Also replace waitForTimeout(300) with waitFor({ state: 'visible' }) on the condition-row field selector to eliminate timing-based flakes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review ✅ Approved 3 resolved / 3 findingsAdds comprehensive Playwright E2E test coverage for the Persona AI Context rule builder, including a regression test for #31564. Addressed review findings regarding flaky assertions and visibility checks. ✅ 3 resolved✅ Edge Case: isVisible({ timeout: 500 }) does not actually wait
✅ Quality: Hardcoded waitForTimeout(300) after adding condition row
✅ Quality: Misleading comment on Fully rendered switch assertion
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
1 participant
Summary
Settings/Persona/PersonaAIContext/), which previously had zero E2E coverage.Root cause (documented in test)
elasticSearchFormatinQueryBuilderElasticsearchFormatUtils.jspassed the rawconfiginstead ofextendedConfigtobuildEsRulein the single-value branch. This causedgetWidgetForFieldOpto returnundefined, so the rule appeared "empty" tohasUnfinishedRule, which returnedtrueand blocked save. The fix (PR #31565) always passesextendedConfig.Why the existing unit tests didn't catch it
All
hasUnfinishedRuleunit tests usedextension.*fields, which take thebuildExtensionQueryshortcut path before widget resolution is attempted — so the broken single-value path was never exercised.Tests added
Test plan