Skip to content

chore(compass-assistant): fix assistant tests #7228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2025
Merged

chore(compass-assistant): fix assistant tests #7228

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76c6025
chore(compass-assistant): fix assistant tests
gagik Aug 26, 2025
00ccee3
chore: minimize side effects from scroll to override
gagik Aug 26, 2025
c6c82f4
chore: move scrollTo to jsdom-extra-mocks-register
gagik Aug 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 66 additions & 39 deletions packages/compass-assistant/src/assistant-chat.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
import React from 'react';
import { render, screen, userEvent } from '@mongodb-js/testing-library-compass';
import {
render,
screen,
userEvent,
waitFor,
} from '@mongodb-js/testing-library-compass';
import { AssistantChat } from './assistant-chat';
import { expect } from 'chai';
import { createMockChat } from '../test/utils';
import { createMockChat, withMockedScrollTo } from '../test/utils';
import type { AssistantMessage } from './compass-assistant-provider';

// TODO: some internal logic in lg-chat breaks all these tests, re-enable the tests
describe.skip('AssistantChat', function () {
describe('AssistantChat', function () {
withMockedScrollTo();

let originalScrollTo: typeof Element.prototype.scrollTo;
// Mock scrollTo method for DOM elements to prevent test failures
before(function () {
originalScrollTo = Element.prototype.scrollTo.bind(Element.prototype);
Element.prototype.scrollTo = () => {};
});
after(function () {
Element.prototype.scrollTo = originalScrollTo;
});

const mockMessages: AssistantMessage[] = [
{
id: 'user',
Expand Down Expand Up @@ -36,8 +52,10 @@ describe.skip('AssistantChat', function () {
it('renders input field and send button', function () {
renderWithChat([]);

const inputField = screen.getByTestId('assistant-chat-input');
const sendButton = screen.getByTestId('assistant-chat-send-button');
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);
const sendButton = screen.getByLabelText('Send message');

expect(inputField).to.exist;
expect(sendButton).to.exist;
Expand All @@ -47,9 +65,9 @@ describe.skip('AssistantChat', function () {
renderWithChat([]);

// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const inputField = screen.getByTestId(
'assistant-chat-input'
) as HTMLInputElement;
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
) as HTMLTextAreaElement;

userEvent.type(inputField, 'What is MongoDB?');

Expand All @@ -60,58 +78,67 @@ describe.skip('AssistantChat', function () {
renderWithChat([]);

// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const sendButton = screen.getByTestId(
'assistant-chat-send-button'
const sendButton = screen.getByLabelText(
'Send message'
) as HTMLButtonElement;

expect(sendButton.disabled).to.be.true;
expect(sendButton.getAttribute('aria-disabled')).to.equal('true');
});

it('send button is enabled when input has text', function () {
renderWithChat([]);

const inputField = screen.getByTestId('assistant-chat-input');
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const sendButton = screen.getByTestId(
'assistant-chat-send-button'
const sendButton = screen.getByLabelText(
'Send message'
) as HTMLButtonElement;

userEvent.type(inputField, 'What is MongoDB?');

expect(sendButton.disabled).to.be.false;
});

it('send button is disabled for whitespace-only input', function () {
// Not currently supported by the LeafyGreen Input Bar
it.skip('send button is disabled for whitespace-only input', async function () {
renderWithChat([]);

const inputField = screen.getByTestId('assistant-chat-input');
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const sendButton = screen.getByTestId(
'assistant-chat-send-button'
const sendButton = screen.getByLabelText(
'Send message'
) as HTMLButtonElement;

userEvent.type(inputField, ' ');

expect(sendButton.disabled).to.be.true;
await waitFor(() => {
expect(sendButton.getAttribute('aria-disabled')).to.equal('true');
});
});

it('displays messages in the chat feed', function () {
renderWithChat(mockMessages);

expect(screen.getByTestId('assistant-message-user')).to.exist;
expect(screen.getByTestId('assistant-message-assistant')).to.exist;
expect(screen.getByTestId('assistant-message-user')).to.have.text(
expect(screen.getByTestId('assistant-message-user')).to.contain.text(
'Hello, MongoDB Assistant!'
);
expect(screen.getByTestId('assistant-message-assistant')).to.have.text(
expect(screen.getByTestId('assistant-message-assistant')).to.contain.text(
'Hello! How can I help you with MongoDB today?'
);
});

it('calls sendMessage when form is submitted', function () {
const { chat } = renderWithChat([]);
const inputField = screen.getByTestId('assistant-chat-input');
const sendButton = screen.getByTestId('assistant-chat-send-button');
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);
const sendButton = screen.getByLabelText('Send message');

userEvent.type(inputField, 'What is aggregation?');
userEvent.click(sendButton);
Expand All @@ -124,24 +151,26 @@ describe.skip('AssistantChat', function () {
renderWithChat([]);

// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const inputField = screen.getByTestId(
'assistant-chat-input'
) as HTMLInputElement;
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
) as HTMLTextAreaElement;

userEvent.type(inputField, 'Test message');
expect(inputField.value).to.equal('Test message');

userEvent.click(screen.getByTestId('assistant-chat-send-button'));
userEvent.click(screen.getByLabelText('Send message'));
expect(inputField.value).to.equal('');
});

it('trims whitespace from input before sending', function () {
const { chat } = renderWithChat([]);

const inputField = screen.getByTestId('assistant-chat-input');
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);

userEvent.type(inputField, ' What is sharding? ');
userEvent.click(screen.getByTestId('assistant-chat-send-button'));
userEvent.click(screen.getByLabelText('Send message'));

expect(chat.sendMessage.calledWith({ text: 'What is sharding?' })).to.be
.true;
Expand All @@ -150,8 +179,10 @@ describe.skip('AssistantChat', function () {
it('does not call sendMessage when input is empty or whitespace-only', function () {
const { chat } = renderWithChat([]);

const inputField = screen.getByTestId('assistant-chat-input');
const chatForm = screen.getByTestId('assistant-chat-form');
const inputField = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);
const chatForm = screen.getByTestId('assistant-chat-input');

// Test empty input
userEvent.click(chatForm);
Expand All @@ -169,16 +200,12 @@ describe.skip('AssistantChat', function () {
const userMessage = screen.getByTestId('assistant-message-user');
const assistantMessage = screen.getByTestId('assistant-message-assistant');

// User messages should have different background color than assistant messages
// User messages should have different class names than assistant messages
expect(userMessage).to.exist;
expect(assistantMessage).to.exist;

const userStyle = window.getComputedStyle(userMessage);
const assistantStyle = window.getComputedStyle(assistantMessage);

expect(userStyle.backgroundColor).to.not.equal(
assistantStyle.backgroundColor
);
// Check that they have different class names (indicating different styling)
expect(userMessage.className).to.not.equal(assistantMessage.className);
});

it('handles messages with multiple text parts', function () {
Expand Down
35 changes: 31 additions & 4 deletions packages/compass-assistant/src/assistant-chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
LgChatMessage,
LgChatMessageFeed,
LgChatInputBar,
spacing,
css,
} from '@mongodb-js/compass-components';

const { ChatWindow } = LgChatChatWindow;
Expand All @@ -20,6 +22,20 @@ interface AssistantChatProps {
chat: Chat<AssistantMessage>;
}

// 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({
Copy link
Contributor Author

@gagik gagik Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't required for the tests but are CSS patchups that I feel like I might as well include here.

Copy link
Contributor Author

@gagik gagik Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raised both with LG

// Negative margin to patch the padding of the drawer.
margin: -spacing[400],
Copy link
Collaborator

@gribnoysup gribnoysup Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're doing more or less the same for data modeling, I wonder if we should move this to our drawer portal already

Copy link
Contributor Author

@gagik gagik Aug 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

'> div, > div > div, > div > div > div, > div > div > div > div': {
height: '100%',
},
});
const messageFeedFixesStyles = css({ height: '100%' });
const chatWindowFixesStyles = css({
height: '100%',
});

export const AssistantChat: React.FunctionComponent<AssistantChatProps> = ({
chat,
}) => {
Expand All @@ -42,19 +58,30 @@ export const AssistantChat: React.FunctionComponent<AssistantChatProps> = ({

const handleMessageSend = useCallback(
(messageBody: string) => {
void sendMessage({ text: messageBody });
const trimmedMessageBody = messageBody.trim();
if (trimmedMessageBody) {
void sendMessage({ text: trimmedMessageBody });
}
},
[sendMessage]
);

return (
<div data-testid="assistant-chat" style={{ height: '100%', width: '100%' }}>
<div
data-testid="assistant-chat"
className={assistantChatFixesStyles}
style={{ height: '100%', width: '100%' }}
>
<LeafyGreenChatProvider variant={Variant.Compact}>
<ChatWindow title="MongoDB Assistant">
<MessageFeed data-testid="assistant-chat-messages">
<ChatWindow title="MongoDB Assistant" className={chatWindowFixesStyles}>
<MessageFeed
data-testid="assistant-chat-messages"
className={messageFeedFixesStyles}
>
{lgMessages.map((messageFields) => (
<Message
key={messageFields.id}
sourceType="markdown"
{...messageFields}
data-testid={`assistant-message-${messageFields.id}`}
/>
Expand Down
18 changes: 10 additions & 8 deletions packages/compass-assistant/src/compass-assistant-provider.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from '@mongodb-js/compass-components';
import type { AtlasService } from '@mongodb-js/atlas-service/provider';
import { CompassAssistantDrawer } from './compass-assistant-drawer';
import { createMockChat } from '../test/utils';
import { createMockChat, withMockedScrollTo } from '../test/utils';

// Test component that renders AssistantProvider with children
const TestComponent: React.FunctionComponent<{
Expand Down Expand Up @@ -84,8 +84,8 @@ describe('AssistantProvider', function () {
);
});

// TODO: some internal logic in lg-chat breaks all these tests, re-enable the tests
describe.skip('with existing chat instance', function () {
describe('with existing chat instance', function () {
withMockedScrollTo();
before(function () {
// TODO(COMPASS-9618): skip in electron runtime for now, drawer has issues rendering
if ((process as any).type === 'renderer') {
Expand Down Expand Up @@ -118,7 +118,7 @@ describe('AssistantProvider', function () {
);

expect(screen.getByTestId('assistant-message-2')).to.exist;
expect(screen.getByTestId('assistant-message-2')).to.have.text(
expect(screen.getByTestId('assistant-message-2')).to.contain.text(
'Test assistant message'
);
});
Expand All @@ -138,8 +138,10 @@ describe('AssistantProvider', function () {

await renderOpenAssistantDrawer(mockChat);

const input = screen.getByTestId('assistant-chat-input');
const sendButton = screen.getByTestId('assistant-chat-send-button');
const input = screen.getByPlaceholderText(
'Ask MongoDB Assistant a question'
);
const sendButton = screen.getByLabelText('Send message');

userEvent.type(input, 'Hello assistant');
userEvent.click(sendButton);
Expand Down Expand Up @@ -176,10 +178,10 @@ describe('AssistantProvider', function () {
await renderOpenAssistantDrawer(mockChat);

userEvent.type(
screen.getByTestId('assistant-chat-input'),
screen.getByPlaceholderText('Ask MongoDB Assistant a question'),
Copy link
Collaborator

@gribnoysup gribnoysup Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good example of how not relying on test-ids would save us some refactoring time 🙂

Copy link
Contributor Author

@gagik gagik Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

'Hello assistant!'
);
userEvent.click(screen.getByTestId('assistant-chat-send-button'));
userEvent.click(screen.getByLabelText('Send message'));

expect(sendMessageSpy.calledOnce).to.be.true;
expect(sendMessageSpy.firstCall.args[0]).to.deep.include({
Expand Down
14 changes: 14 additions & 0 deletions packages/compass-assistant/test/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,17 @@ export const createMockChat = ({
sendMessage: sinon.SinonStub;
};
};

export function withMockedScrollTo() {
let originalScrollTo: typeof Element.prototype.scrollTo;
// Mock scrollTo method for DOM elements to prevent test failures
before(function () {
originalScrollTo = Element.prototype.scrollTo;
if (!Element.prototype.scrollTo) {
Element.prototype.scrollTo = () => {};
}
});
after(function () {
Element.prototype.scrollTo = originalScrollTo;
});
}