Skip to content

test(quantic): document suggestion e2e #5176

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.

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 6 commits into from
Apr 24, 2025
Merged

test(quantic): document suggestion e2e #5176

merged 6 commits into from
Apr 24, 2025

Conversation

mikegron
Copy link
Contributor

@mikegron mikegron commented Apr 15, 2025

SFINT-5793

Playwright tests for the remaining tests of document suggestions. Almost all cypress were tested in jest.

BONUS: I capture the sendBeacon data to test the EP analytics! Check in the fixture for how it is done. We could probably place that in a test util. Perfect for SFINT-5584

image

@mikegron mikegron self-assigned this Apr 15, 2025
@mikegron mikegron requested a review from a team as a code owner April 15, 2025 14:47
@developer-experience-bot
Copy link
Contributor

developer-experience-bot bot commented Apr 15, 2025

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 248.8 248.8 0
commerce 361.1 361.1 0
search 419.8 419.8 0
insight 410.9 410.9 0
recommendation 259.6 259.6 0
ssr 413.5 413.5 0
ssr-commerce 377.6 377.6 0

@mmitiche
Copy link
Contributor

@mikegron I see you started testing EP in this PR for document suggestion, our initial plan was to separate the analytics EP testing in separate PR in order to focus more on figuring out the best way and the best organization to do that using the available Playwright features, we have this ticket for the following investigation: https://coveord.atlassian.net/browse/SFINT-6076

And then we would do the following ticket to implement the thing: https://coveord.atlassian.net/browse/SFINT-5913

This separation was made to ensure we end up with most optimal structure in order to test the analytics and keep the test specs simple by minimizing code duplication.

Let me know what you think about that, we can also keep all the tests you aded for EP here as an example implementation for when we focus fully on the analytics EP testing.

@mikegron mikegron requested review from SimonMilord and Copilot April 17, 2025 18:02
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds end-to-end tests for document suggestions in Quantic using Playwright, while also capturing analytics data via sendBeacon for enhanced test validation.

  • Introduces a new analytics events regex and helper function in the Playwright requests utility.
  • Adds new page object methods and test cases to handle both legacy and next analytics modes.
  • Updates fixture and page object files to support the revised document suggestion behavior.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/quantic/playwright/utils/requests.ts Added analyticsEventsUrlRegex and isUaEventsEvent to capture events requests.
packages/quantic/playwright/page-object/caseAssistObject.ts Introduced fetchSuggestionsButton property and corresponding fetchSuggestions method.
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/quanticDocumentSuggestion.e2e.ts Added e2e tests covering both legacy and next analytics modes for document suggestions.
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/pageObject.ts Created a page object for document suggestion with proper locators and event wait methods.
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/fixture.ts Updated test fixture to integrate the new document suggestion page object and options.

Copy link
Contributor

@SimonMilord SimonMilord left a comment

Choose a reason for hiding this comment

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

I agree with @mmitiche point, I think the EP analytics should be added together later on a to avoid confusion and make sure all components analytics are tested the same. Other than that, it looks fine to me.

@mikegron
Copy link
Contributor Author

I think we can keep the EP tests since they are done for reference in the future and we could replace them eventually 🤷

@mikegron mikegron enabled auto-merge April 22, 2025 15:22
@mikegron mikegron added this pull request to the merge queue Apr 24, 2025
Copy link
Contributor

@mmitiche mmitiche left a comment

Choose a reason for hiding this comment

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

LGTM!


['legacy', 'next'].forEach((mode) => {
test.describe(`quantic document suggestion with analytics mode: ${mode}`, () => {
let analyticsMode = 'legacy';
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to initialize with the value legacy ?

]);
}

await expect(documentSuggestion.noSuggestionsMessage).toBeVisible();
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this expectation in the beforeEach block?

Comment on lines +53 to +78
if (analyticsMode === 'legacy') {
expect(payload).toBeDefined();
const event = JSON.parse(payload.clickEvent);
expect(event.actionCause).toBe('documentSuggestionClick');
expect(event.documentPosition).toBe(clickedPosition + 1);
expect(event.documentTitle).toBe(
suggestions.documents[clickedPosition].title
);
expect(event.documentUrl).toBe(
suggestions.documents[clickedPosition].clickUri
);
} else {
expect(payload).toBeDefined();
expect(payload.length).toBe(1);
expect(payload[0].meta.config.trackingId).toBe(fakeTrackingId);
expect(payload[0].meta.type).toBe('CaseAssist.DocumentSuggestionClick');
expect(payload[0].responseId).toBeDefined();
expect(payload[0].position).toBe(clickedPosition + 1);
expect(payload[0].itemMetadata.title).toBe(
suggestions.documents[clickedPosition].title
);
expect(payload[0].itemMetadata.uniqueFieldName).toBe('uniqueId');
expect(payload[0].itemMetadata.uniqueFieldValue).toBe(
suggestions.documents[clickedPosition].uniqueId
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think of making this logic in a function inside the page object file of this component, a functions that takes the expected payload and do the necessary checks, I think it would make the tests cleaner and we could reuse this function when needed.

Merged via the queue into master with commit f855ccd Apr 24, 2025
105 checks passed
@mikegron mikegron deleted the feat/sfint-5793-2 branch April 24, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants