test(chat): align ChatInterface selector hooks#2085
Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
WalkthroughThis PR updates the AI Chat Interface component to add a data-testid attribute to the ModelDisplay component and synchronizes all test selectors to use a new naming convention, replacing message-* prefixes with chat-* and assistant-message-specific identifiers. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Analysis 📊Changed Files Summary:
CI Status: Running automated checks... |
Dependency Audit Results |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b572eb5479
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div className="flex items-center gap-3"> | ||
| <h2 className="text-lg font-semibold">AI Chat</h2> | ||
| <ModelDisplay | ||
| data-testid="model-badge" |
There was a problem hiding this comment.
Passing data-testid to <ModelDisplay> here does not actually create a model-badge element in the DOM, because ModelDisplay currently only destructures model, className, showDetails, and compact and never forwards extra props to its root node (src/components/ai/ModelDisplay.tsx). In this scenario, selectors like getByTestId('model-badge') in the updated tests (and the existing e2e locator) still fail, so the selector-alignment fix remains broken.
Useful? React with 👍 / 👎.
Quick Checks Results
✅ All quick checks passed! |
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce selector drift failures by aligning ChatInterface unit tests with the component’s current data-testid hooks, and by adding a test hook for the header’s model display.
Changes:
- Update
ChatInterfaceunit tests to use current selectors (e.g.,chat-input,user-message,assistant-message). - Add
data-testid="model-badge"to the header model display inChatInterface.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/components/ai/ChatInterface.test.tsx | Updates RTL selectors to match current ChatInterface DOM hooks. |
| src/components/ai/ChatInterface.tsx | Adds a data-testid hook intended to make the header model display selectable in tests. |
| <ModelDisplay | ||
| data-testid="model-badge" | ||
| model={availableModels.find((m) => m.id === selectedModel)} | ||
| compact | ||
| /> |
There was a problem hiding this comment.
data-testid="model-badge" is being passed to ModelDisplay, but ModelDisplay doesn’t currently forward arbitrary props to a DOM element (it only uses model/className/showDetails/compact). As a result, no element will actually have data-testid="model-badge", and the updated tests using getByTestId('model-badge') will fail. Fix by either (a) wrapping ModelDisplay in a simple DOM element that carries the data-testid, or (b) updating ModelDisplay to accept and spread standard HTML props (including data-testid) onto its top-level element in all render branches.
| <ModelDisplay | |
| data-testid="model-badge" | |
| model={availableModels.find((m) => m.id === selectedModel)} | |
| compact | |
| /> | |
| <div data-testid="model-badge"> | |
| <ModelDisplay | |
| model={availableModels.find((m) => m.id === selectedModel)} | |
| compact | |
| /> | |
| </div> |
Build Status ✅ Build successful✅ Build completed successfully! |
🔒 Security Audit Results✅ Secret Scanning: No secrets detected 📊 View full results: Security Audit Summary |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ai/ChatInterface.tsx`:
- Line 363: ModelDisplay currently defines ModelDisplayProps without accepting
generic HTML props, so attributes like data-testid passed from ChatInterface are
dropped; update the ModelDisplayProps to extend
React.ComponentPropsWithoutRef<'div'> (or include
HTMLAttributes<HTMLDivElement>) and in the ModelDisplay functional component
destructure the rest (`...rest`) from props and spread them onto the root
element (the div/rendered root in ModelDisplay) so test attributes and other
HTML props (e.g., data-testid="model-badge") are forwarded; ensure to keep
existing props (model, className, showDetails, compact) and merge className
appropriately when spreading rest.
| <div className="flex items-center gap-3"> | ||
| <h2 className="text-lg font-semibold">AI Chat</h2> | ||
| <ModelDisplay | ||
| data-testid="model-badge" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MODEL_FILE="$(fd 'ModelDisplay.tsx$' src | head -n 1)"
echo "Inspecting: ${MODEL_FILE}"
cat -n "${MODEL_FILE}"
echo
echo "Prop-forwarding signal check:"
rg -n "data-testid|\\.\\.\\.|HTMLAttributes|ComponentProps|props" "${MODEL_FILE}"Repository: ryanmaclean/vibecode-webgui
Length of output: 10704
Fix ModelDisplay to forward test attributes and HTML props to the root element.
The component's ModelDisplayProps interface accepts only model, className, showDetails, and compact, but does not destructure or forward additional props like data-testid to any DOM element. The data-testid="model-badge" attribute passed in ChatInterface.tsx (line 363) is discarded and tests cannot select this component via that hook. Update the component to accept and spread HTMLAttributes<HTMLDivElement> (or use React.ComponentPropsWithoutRef<'div'>) to enable proper test attribute forwarding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ai/ChatInterface.tsx` at line 363, ModelDisplay currently
defines ModelDisplayProps without accepting generic HTML props, so attributes
like data-testid passed from ChatInterface are dropped; update the
ModelDisplayProps to extend React.ComponentPropsWithoutRef<'div'> (or include
HTMLAttributes<HTMLDivElement>) and in the ModelDisplay functional component
destructure the rest (`...rest`) from props and spread them onto the root
element (the div/rendered root in ModelDisplay) so test attributes and other
HTML props (e.g., data-testid="model-badge") are forwarded; ensure to keep
existing props (model, className, showDetails, compact) and merge className
appropriately when spreading rest.
Test Results ✅ PassedTest Suites: 57 failed, 5 skipped, 488 passed, 545 of 550 total ✅ All tests passed! Ready for review. View test outputCheck the Actions tab for detailed test output. |
PR Status Summary
✅ All checks passed! This PR is ready to merge. 🎉 |
Problem
Dependabot PRs are repeatedly blocked by ChatInterface selector drift in unit tests (selectors in tests no longer match component DOM hooks).
Scope
src/components/ai/ChatInterface.tsxtests/components/ai/ChatInterface.test.tsxChanges
model-badgetest hook to header model display inChatInterface.ChatInterfaceunit tests to use current selectors (chat-input,user-message,assistant-message).Risk/Rollback
Low risk: selector/test-hook alignment only; no runtime logic changes.
Rollback: revert this PR.
Validation
npx jest --runInBand tests/components/ai/ChatInterface.test.tsxjest-environment-jsdommissing in this workspace, so CI is source of truth for this change.Summary by CodeRabbit