test(quantic): document suggestion jest tests #5146

mikegron · 2025-04-08T18:36:03Z

Created Jest tests for Document suggestion component. Had some troubles testing some things but should be good now lol

Some things maybe different as I was trying to follow jest docs:

Package mocks before imports so you have a clean mock using jest.mocked, instead of the prepareHeadlessState and mockSuccessfulHeadlessInitialization
Tested all parameters, actions dispatched, rendering etc.

I didn't really like the formatting but I hope you have suggestions! For instance, the global.CoveoHeadlessCaseAssist mock could probably be a utility.

github-actions · 2025-04-08T18:41:27Z

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File	Old (kb)	New (kb)
case-assist	248.6	248.6
commerce	361	361
search	419.6	419.6
insight	410.8	410.8
recommendation	259.5	259.5
ssr	413.4	413.4
ssr-commerce	377.5	377.5

SFINT-5793

mmitiche · 2025-04-08T19:42:37Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+        updateSuggestions();
+        await flushPromises();
+
+        expect(engineMock.dispatch).not.toHaveBeenCalled();


which action is the one we are checking that it is not dispatched here?

fetchSuggestion especially!

in that case wouldn't be better to have expect(fetchSuggestion).not.toHaveBeenCalled();?

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

mmitiche

nice tests!
looks good to me after addressing the issue of the polluting logs!

mmitiche · 2025-04-11T12:01:30Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+headlessLoaderMock.getHeadlessEnginePromise.mockImplementation(() =>
+  Promise.reject({message: 'Skip initialization'})
+);


This is causing the following which is polluting the logs: https://github.com/coveo/ui-kit/actions/runs/14365598472/job/40277925597?pr=5146

We need to address this, a suggestion could be to handle initialization like we do for other components.

mmitiche · 2025-04-11T12:07:03Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+  loadCaseAssistAnalyticsActions: jest.fn().mockReturnValue({}),
+  loadDocumentSuggestionActions: jest.fn().mockReturnValue({
+    fetchDocumentSuggestions: functionsMocks.fetchDocumentSuggestions,
+    logDocumentSuggestionClick: functionsMocks.logDocumentSuggestionClick,
+    logDocumentSuggestionRating: functionsMocks.logDocumentSuggestionRating,


To stay lawful to the Headless implementation, logDocumentSuggestionClick and logDocumentSuggestionRating should come from loadCaseAssistAnalyticsActions and not loadDocumentSuggestionActions :

ui-kit/packages/headless/src/features/case-assist/case-assist-analytics-actions-loader.ts

Lines 116 to 130 in 77feade

export function loadCaseAssistAnalyticsActions(

engine: CaseAssistEngine

): CaseAssistAnalyticsActionCreators {

engine.addReducers({});

return {

logCaseStart,

logCaseNextStage,

logCreateCase,

logSolveCase,

logAbandonCase,

logUpdateCaseField,

logClassificationClick,

logDocumentSuggestionClick,

logDocumentSuggestionRating,

logQuickviewDocumentSuggestionClick,

mmitiche · 2025-04-11T12:10:46Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+      const sections = element.shadowRoot.querySelectorAll(
+        selectors.accordionSection
+      );
+      expect(sections.length).toBe(2);


an improvements could be to store the suggestions in a variable exampleSuggestions.
And then here instead of comparing to 2, we compare to exampleSuggestions.length

This case our test become a bit more scalalble, if in the future we need to add a new test case where we need three document suggestions for example, we would only care about changing exampleSuggestions, the rest will follow smoothly.

mmitiche · 2025-04-11T12:13:17Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+        expect(section.label).toBe(
+          documentSuggestionListMock.state.documents[index].title
+        );
+        const uniqueId =
+          documentSuggestionListMock.state.documents[index].uniqueId;
+        expect(section.getAttribute('data-id')).toBe(uniqueId);


Suggested change

expect(section.label).toBe(

documentSuggestionListMock.state.documents[index].title

);

const uniqueId =

documentSuggestionListMock.state.documents[index].uniqueId;

expect(section.getAttribute('data-id')).toBe(uniqueId);

sections.forEach((section, index) => {

const {title, uniqueId} = documentSuggestionListMock.state.documents[index];

expect(section.label).toBe(title);

expect(section.getAttribute('data-id')).toBe(uniqueId);

mmitiche · 2025-04-11T12:14:03Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+      expect(element.shadowRoot.querySelector(selectors.quickview)).toBeFalsy();
+    });
+
+    it('should log a suggestion click if not opened', async () => {


Suggested change

it('should log a suggestion click if not opened', async () => {

it('should log a suggestion click if the suggestion is not opened', async () => {

mmitiche · 2025-04-11T12:21:13Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+    });
+  });
+
+  describe('when two document suggestions have a similar title', () => {


the cypress test do this with a component that has numberOfAutoOpenedDocuments set to 0:

ui-kit/packages/quantic/cypress/e2e/default-2/document-suggestion/document-suggestion.cypress.ts

Lines 210 to 215 in 5946a9e

describe('when two document suggestions have a similar title', () => {

it('should open the right document suggestion when clicked', () => {

const exampleResponseId = crypto.randomUUID();

visitDocumentSuggestion({

numberOfAutoOpenedDocuments: 0,

});

I think we should do the same here, as this cypress test was added after a bug that we fixed, so we need to replicate exactly the stup that was causing the issue previously.

SimonMilord

Nice!

SimonMilord · 2025-04-11T15:01:47Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+  );
+}
+
+function checkElement(element, selector, shouldExist) {


maybe this could be in a utils?

I personally don't prefer the usage of such functions, cause they lead to the absence of assertions in the tests which for more is not very ideal.

I prefer that when I read the test, I instantly understand what's going on and what's being tested without having to check for function definitions that are defined in the same file or even less ideal when defined in separate files.

I know the benefit of such functions is to reduce code duplication and follow the DRY mindset (Don't repeat yourself ).
But for me DRY is less applicable and less beneficial when it comes to test writing, for tests and for me, it's more about clarity and simplicity, even if it includes some code duplication like here a duplication of the expect assertion that checks that an element is present.

Thoughts? @mikegron @SimonMilord @erocheleau

SimonMilord · 2025-04-11T15:04:05Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+      });
+    });
+
+    it('should not render quick view button when withoutQuickview is set to true', async () => {


Suggested change

it('should not render quick view button when withoutQuickview is set to true', async () => {

it('should not render quickview button when withoutQuickview is set to true', async () => {

SimonMilord · 2025-04-11T15:10:37Z

...e-app/main/default/lwc/quanticDocumentSuggestion/__tests__/quanticDocumentSuggestion.test.js

+      );
+    });
+
+    it('should handle document rating event', async () => {


Suggested change

it('should handle document rating event', async () => {

it('should log analytics when rating a document', async () => {

mikegron self-assigned this Apr 8, 2025

mikegron requested a review from a team as a code owner April 8, 2025 18:36

mikegron requested review from erocheleau, SimonMilord and mmitiche April 8, 2025 18:36

mikegron temporarily deployed to PR Artifacts April 8, 2025 18:51 — with GitHub Actions Inactive

test(quantic): document suggestion jest tests

0d57657

SFINT-5793

mikegron force-pushed the feat/sfint-5793 branch from d04c4f9 to 0d57657 Compare April 8, 2025 20:03

mikegron temporarily deployed to PR Artifacts April 8, 2025 20:18 — with GitHub Actions Inactive

mmitiche reviewed Apr 8, 2025

View reviewed changes

Pr changes and refactor

af8e677

mikegron temporarily deployed to PR Artifacts April 9, 2025 19:53 — with GitHub Actions Inactive

mmitiche approved these changes Apr 11, 2025

View reviewed changes

mmitiche reviewed Apr 11, 2025

View reviewed changes

SimonMilord approved these changes Apr 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(quantic): document suggestion jest tests #5146

test(quantic): document suggestion jest tests #5146

mikegron commented Apr 8, 2025 •

edited

Loading

github-actions bot commented Apr 8, 2025

mmitiche Apr 8, 2025

mikegron Apr 9, 2025

mmitiche Apr 11, 2025

mmitiche left a comment

mmitiche Apr 11, 2025

mmitiche Apr 11, 2025

mmitiche Apr 11, 2025

mmitiche Apr 11, 2025

mmitiche Apr 11, 2025

mmitiche Apr 11, 2025

SimonMilord left a comment

SimonMilord Apr 11, 2025

mmitiche Apr 11, 2025 •

edited

Loading

SimonMilord Apr 11, 2025

SimonMilord Apr 11, 2025

	export function loadCaseAssistAnalyticsActions(
	engine: CaseAssistEngine
	): CaseAssistAnalyticsActionCreators {
	engine.addReducers({});
	return {
	logCaseStart,
	logCaseNextStage,
	logCreateCase,
	logSolveCase,
	logAbandonCase,
	logUpdateCaseField,
	logClassificationClick,
	logDocumentSuggestionClick,
	logDocumentSuggestionRating,
	logQuickviewDocumentSuggestionClick,

	it('should log a suggestion click if not opened', async () => {
	it('should log a suggestion click if the suggestion is not opened', async () => {

	describe('when two document suggestions have a similar title', () => {
	it('should open the right document suggestion when clicked', () => {
	const exampleResponseId = crypto.randomUUID();
	visitDocumentSuggestion({
	numberOfAutoOpenedDocuments: 0,
	});

	it('should not render quick view button when withoutQuickview is set to true', async () => {
	it('should not render quickview button when withoutQuickview is set to true', async () => {

	it('should handle document rating event', async () => {
	it('should log analytics when rating a document', async () => {

test(quantic): document suggestion jest tests #5146

Are you sure you want to change the base?

test(quantic): document suggestion jest tests #5146

Conversation

mikegron commented Apr 8, 2025 • edited Loading

github-actions bot commented Apr 8, 2025

Pull Request Report

PR Title

Live demo links

Bundle Size

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitiche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonMilord left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitiche Apr 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikegron commented Apr 8, 2025 •

edited

Loading

mmitiche Apr 11, 2025 •

edited

Loading