test(intelligent-assistant): add saved prompts coverage for PR #4600 - #4822
HusneShabbir wants to merge 5 commits into
Conversation
…-developer#4600 Add SavedPromptCard component tests, settingsTab provider state coverage, and Playwright e2e for sidebar apply and settings create flows. Signed-off-by: HusneShabbir <[email protected]> Co-authored-by: Cursor <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4822 +/- ##
=======================================
Coverage 62.01% 62.01%
=======================================
Files 2680 2680
Lines 106534 106534
Branches 29834 29819 -15
=======================================
+ Hits 66064 66068 +4
+ Misses 39910 39906 -4
Partials 560 560
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Remove the Playwright spec that failed bootstrap locally and in CI; keep L1/L3 unit coverage for SavedPromptCard and settingsTab state. Signed-off-by: HusneShabbir <[email protected]> Co-authored-by: Cursor <[email protected]>
Fix overlay bootstrap, Chatbot-scoped locators, mock seeding, and settings Save button targeting so saved-prompt sidebar and settings flows pass locally. Co-authored-by: Cursor <[email protected]>
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 5:24 PM UTC · Completed 5:46 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 |
ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| ); | ||
| sharedPage = boot.page; | ||
| translations = boot.translations; | ||
| }); |
There was a problem hiding this comment.
[low] variable-declaration-order
The test.beforeAll block explicitly types the boot variable (const boot: LightspeedE2eBootstrap), whereas all seven existing e2e test files omit the type annotation, relying on inference.
Suggested fix: Remove the explicit type annotation: const boot = await bootstrapLightspeedE2ePage(browser, { openFullscreenChat: false });
|
|
||
| export const closeSortDropdown = async (page: Page) => { | ||
| await page.keyboard.press('Escape'); | ||
| }; |
There was a problem hiding this comment.
[low] locator-function-scope
The new savedPromptSidebarItem locator scopes to page.getByLabel(Chatbot, { exact: true }) instead of sidePanelLocator(page), which is the established base container for sidebar locators in this file. This may be intentional for overlay mode compatibility.
Suggested fix: Verify whether sidePanelLocator works in overlay mode before switching. If it does, use it for consistency.
fullsend-ai-review
Bot
added
the
ready-for-merge
Introduce SavedPromptsPage with translation-driven locators, move fixture data to responses.ts, and reuse loginAsGuest in overlay setup like other specs. Co-authored-by: Cursor <[email protected]>
…coverage Fix sidebar seeding via mock reload to bust React Query cache, add settings kebab/gear/empty-state e2e flows, and cover Apply/Send/Delete kebab actions in unit tests. Co-authored-by: Cursor <[email protected]>
|
| const closeSettings = this.page.getByRole('button', { | ||
| name: this.t['mcp.settings.closeAriaLabel'], | ||
| }); | ||
| if (await closeSettings.isVisible().catch(() => false)) { |
There was a problem hiding this comment.
I don't think you need the catch construct here, isVisible should just return false if the element isn't on screen
also applies to all the other occurrences of this :)
| await expect | ||
| .poll( | ||
| async () => { | ||
| const emptyVisible = await this.savedPromptsMenu() | ||
| .getByRole('menuitem', { | ||
| name: this.t['savedPrompts.sidebar.empty'], | ||
| }) | ||
| .isVisible() | ||
| .catch(() => false); | ||
| const promptVisible = await this.savedPromptSidebarItem(promptName) | ||
| .isVisible() | ||
| .catch(() => false); | ||
| return !emptyVisible && promptVisible; | ||
| }, | ||
| { timeout: 15_000 }, | ||
| ) | ||
| .toBe(true); |
There was a problem hiding this comment.
wouldn't it be a lot simpler to check if savedPromptSidebarItem is visible using an assertion that auto retries?
and for completeness, you can then check if the empty state is not visible the same way
| created_at: string; | ||
| updated_at: string; | ||
| }; | ||
| import type { SavedPromptMock } from '../fixtures/responses'; |
There was a problem hiding this comment.
looks like an import from '../fixtures/responses' already exists
3 participants
Description
Follow-up test coverage for the Saved Prompts feature merged in #4600.
Resolves:
https://redhat.atlassian.net/browse/RHIDP-15753
https://redhat.atlassian.net/browse/RHIDP-15754
https://redhat.atlassian.net/browse/RHIDP-15755
No product code changes — tests and e2e infrastructure only.
L3 component —
SavedPromptCard.test.tsxonApplyToInput(content)onSendDirectly(content)onDelete(prompt)intelligentAssistantMessagesL1 unit —
useLightspeedProviderState.test.tsxsettingsTab/setSettingsTabexposed through provider context?settingsTab=saved-promptshandlingL4a Playwright —
lightspeed.saved-prompts.test.tsUses a dedicated page object (
SavedPromptsPage) with translation-backed locators, matching other intelligent-assistant e2e specs.E2e mock/setup notes
mockSavedPrompts()route handler + in-memory seed (devMode.ts,fixtures/responses.ts)page.reload()after seeding to clear React Query’s 5-minute saved-prompts cache (fixes stale “No saved prompts yet” in the sidebar)dialog(not generic drawer panel)openFullscreenChat: falsefor overlay modeTest plan
yarn test SavedPromptCard useLightspeedProviderState --watchAll=falseinworkspaces/intelligent-assistant/plugins/intelligent-assistantAPP_MODE=legacy npx playwright test e2e-tests/lightspeed.saved-prompts.test.ts --project=eninworkspaces/intelligent-assistant(7 tests, ~45s locally)Checklist