Refactor ontology playwright test - #31661
Conversation
anuj-kumary
added
the
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🔴 Playwright Results — workflow failedValidated commit ✅ 693 passed · ❌ 1 failed · 🟡 0 flaky · ⏭️ 0 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) 48m 49s ⏱️ Max setup 3m 24s · max shard execution 20m 16s · max shard-job elapsed before upload 23m 44s · reporting 4s 🌐 195.85 requests/attempt · 2.48 app boots/UI scenario · 8.88% common-shard skew Optimization targets still in progress:
Genuine Failures (failed on all attempts)❌
|
…er context disposal createApiContext now calls createAdminApiContext() to get an independently owned context per suite. The previous performAdminLogin path returned the shared worker-scoped APIRequestContext; disposeApiContext then called apiContext.dispose() on it, closing the worker context and causing every subsequent suite's afterAll cleanup to fail with "Target page, context or browser has been closed". Also fixes the double-dispose: afterAction from createAdminApiContext already tears down both the login context and the API context, so the unconditional apiContext.dispose() before the branch is removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The refresh button intermittently clears the glossary filter state due to a timing race between the WebSocket auto-update event (from the API relation add/remove) and the manual reload cycle. When the race goes wrong, the graph reloads without the filter and stats show "0 Terms0 Relations0 Isolated" instead of the expected counts. Replace both refresh→waitForGraphLoaded sequences in the Relation Sync test with navigateAndFilterByGlossary, which always fetches fresh data with the filter correctly applied. The test still verifies that API changes are reflected in the graph — just via a deterministic path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ght specs Move entity setup from beforeAll/afterAll (shared across tests) to beforeEach/afterEach (fresh per test). Each test now creates its own glossary + terms, runs, and deletes them — eliminating all cross-test shared-state failure modes: - No cascade failure when one test's afterAll disposes a context used by another test's beforeAll - No cross-worker WebSocket delete-toast contamination (createApiContext is pure HTTP — no browser page, no WS session opened) - No stats-assertion drift from other parallel workers' entities (each test filters by its own UUID-scoped glossary ID) Also fixes OntologyExplorerFilters.spec.ts line ~197: the glossary dropdown was opened without searching first, risking a pagination miss on a busy AUT. Now searches by name before asserting visibility — consistent with the pattern already used in the Dropdown Search tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…ilures If beforeEach throws before all entity assignments complete (e.g. the createApiContext call itself fails on the first test run), the module-level `let` variables remain undefined. Playwright still runs afterEach, and calling deleteEntities(apiContext, undefined, ...) or undefined.delete(apiContext) would throw — masking the original failure. Fix all 8 ontology spec files: - Wrap deleteEntities() calls in `if (primaryEntity)` guards - Use `?.delete()` optional chaining on GlossaryTermRelationsGraph direct .delete() calls - Split multi-glossary afterEach blocks by glossary group so partial creation still cleans up the entities that did get created Addresses gitar-bot review finding on PR #31661. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The "should reflect relation add and remove in the graph" test performs three navigateAndFilterByGlossary calls, each of which internally calls waitForGraphLoaded twice (up to 30 s each). Three navigations × two waits = up to 180 s of graph-loading overhead — well beyond the default 60 s test budget, causing CI to timeout at the second navigation. Fix: call test.slow() at the start of the test body to triple the timeout to 180 s. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review ✅ Approved 1 resolved / 1 findingsRefactors ontology Playwright tests to use per-test entity isolation and fresh API contexts, while guarding afterEach cleanup against partial beforeEach failures. No issues found. ✅ 1 resolved✅ Edge Case: afterEach may deref undefined entities if beforeEach fails midway
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
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.Summary by Gitar
createApiContextanddisposeApiContextin ontology playwright utils to useperformAdminLogininstead of browser pages.This will update automatically on new commits.
Greptile Summary
The PR refactors ontology Playwright tests to use fresh, independently owned admin API contexts and per-test entity lifecycles.
createAdminApiContext.afterActioncallback.Confidence Score: 5/5
The PR appears safe to merge.
The previously reported shared API-context disposal failure is fixed because the helper now creates a fresh context for each call and invokes the matching ownership-aware cleanup callback; no blocking failure remains.
Important Files Changed
Sequence Diagram
Reviews (8): Last reviewed commit: "fix(e2e): add test.slow() to Relation Sy..." | Re-trigger Greptile