chore(compass-assistant): fix assistant tests #7228
Conversation
gagik
force-pushed
the
gagik/fix-assistant-tests
branch
from
b04a9d2 to
76c6025
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes failing assistant tests by updating test selectors and mock methods to match the leafygreen chat components. The changes primarily involve replacing test IDs with accessibility-based selectors and addressing component API changes.
- Updates test selectors from data-testid attributes to accessibility-based selectors (placeholder text and aria-labels)
- Adds mock for scrollTo method and updates styling to fix layout issues
- Adds new leafygreen component features like MessageActions and sourceType properties
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| compass-assistant-provider.spec.tsx | Updates selectors and re-enables previously skipped tests |
| assistant-chat.tsx | Adds styling fixes, MessageActions component, and input trimming logic |
| assistant-chat.spec.tsx | Updates selectors, adds scrollTo mock, and adjusts test expectations for leafygreen components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| // TODO(COMPASS-9751): These are temporary patches to make the Assistant chat take the entire | ||
| // width and height of the drawer since Leafygreen doesn't support this yet. | ||
| const assistantChatFixesStyles = css({ |
There was a problem hiding this comment.
These aren't required for the tests but are CSS patchups that I feel like I might as well include here.
gagik
added
the
no-title-validation
| // Mock scrollTo method for DOM elements to prevent test failures | ||
| before(function () { | ||
| if (!Element.prototype.scrollTo) { | ||
| Element.prototype.scrollTo = function () { | ||
| // No-op implementation for testing | ||
| }; | ||
| } | ||
| }); |
There was a problem hiding this comment.
We are collecting all those under https://github.com/mongodb-js/compass/blob/main/configs/mocha-config-compass/register/jsdom-extra-mocks-register.js, do you mind moving this also?
| // width and height of the drawer since Leafygreen doesn't support this yet. | ||
| const assistantChatFixesStyles = css({ | ||
| // Negative margin to patch the padding of the drawer. | ||
| margin: -spacing[400], |
There was a problem hiding this comment.
We're doing more or less the same for data modeling, I wonder if we should move this to our drawer portal already
There was a problem hiding this comment.
yeah we will raise this with Leafygreen to confirm they'll also do this and there'll be a discussion about this during the Atlas Chatbots Sync if you'd be interested (invited you as optional)
|
|
||
| userEvent.type( | ||
| screen.getByTestId('assistant-chat-input'), | ||
| screen.getByPlaceholderText('Ask MongoDB Assistant a question'), |
There was a problem hiding this comment.
A good example of how not relying on test-ids would save us some refactoring time 🙂
There was a problem hiding this comment.
funnily enough I opted for testids back then because I thought this would be less design dependent but the inability to pass it to specific subcomments did backfire..
agree generally though, with the only risk being stuff like text being duplicated in multiple components which is something I ran into a lot with context menu tests
gribnoysup
left a comment
gribnoysup
left a comment
There was a problem hiding this comment.
Let's move scrollTo stub to the shared stubs place, otherwise lgtm
Updates selectors and mocks methods to match leafygreen components.