-
Notifications
You must be signed in to change notification settings - Fork 36
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
Conversation
Pull Request ReportPR Title✅ Title follows the conventional commit spec. Live demo linksBundle Size
|
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/fixture.ts
Outdated
Show resolved
Hide resolved
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/pageObject.ts
Outdated
Show resolved
Hide resolved
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/pageObject.ts
Outdated
Show resolved
Hide resolved
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/pageObject.ts
Outdated
Show resolved
Hide resolved
packages/quantic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/pageObject.ts
Outdated
Show resolved
Hide resolved
...ic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/quanticDocumentSuggestion.e2e.ts
Outdated
Show resolved
Hide resolved
...ic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/quanticDocumentSuggestion.e2e.ts
Outdated
Show resolved
Hide resolved
...ic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/quanticDocumentSuggestion.e2e.ts
Show resolved
Hide resolved
@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. |
There was a problem hiding this 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. |
There was a problem hiding this 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.
I think we can keep the EP tests since they are done for reference in the future and we could replace them eventually 🤷 |
There was a problem hiding this 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'; |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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?
...ic/force-app/main/default/lwc/quanticDocumentSuggestion/e2e/quanticDocumentSuggestion.e2e.ts
Show resolved
Hide resolved
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 | ||
); | ||
} |
There was a problem hiding this comment.
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.
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