feat(cc-widgets): UI Automation for Basic Task Controls#497
Conversation
…helperUtils and fix initUtils formatting
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
| if (!extensionPage) { | ||
| throw new Error('Extension page is not available for handling extension call'); | ||
| } |
There was a problem hiding this comment.
Should we rather check if the UI goes rightly to RONA if the extension page is not open?
There was a problem hiding this comment.
Should this be done as a separate test? If so, I think it would be more suitable to add it to incomingTask tests.
There was a problem hiding this comment.
@rsarika - Here' we are failing the test. However, shouldn't it be a test that simply ensures it goes to RONA if the extension isn't open?
| */ | ||
| export async function callTaskControlCheck(page: Page): Promise<void> { | ||
| // Verify call control container is visible | ||
| await expect(page.getByTestId('call-control-container').nth(0)).toBeVisible({ timeout: 30000 }); |
There was a problem hiding this comment.
If Akula's PR goes in before this, let us ensure to include the mute button as well
| * @param expectedIsHeld - Expected hold state (true for hold, false for resume) | ||
| * @throws Error if verification fails with detailed error message | ||
| */ | ||
| export function verifyHoldLogs(expectedIsHeld: boolean): void { |
There was a problem hiding this comment.
Do we not verify anywhere if the icons were changing based on the state?
There was a problem hiding this comment.
Added utils and checks for the same
| // Verify audio element properties using JavaScript evaluation | ||
| // Handle multiple audio elements with same ID by checking all of them | ||
| const audioProperties = await page.evaluate(() => { | ||
| const audioElements = document.querySelectorAll('#remote-audio') as NodeListOf<HTMLAudioElement>; |
There was a problem hiding this comment.
We add IDs(#remote-audio) as unique elements. Why do we need to do querySelectorAll? We could do querySelector instead that will result in not having a forEach in line#270
There was a problem hiding this comment.
Sure. Will look into it
| tracksCount = mediaStream.getTracks().length; | ||
| audioTracksCount = mediaStream.getAudioTracks().length; |
There was a problem hiding this comment.
Isn't the audioTracksCount enough here?
There was a problem hiding this comment.
Re-checking the way I am verifying audio stream
| * @returns Promise<void> | ||
| * @throws Error if remote audio element is not found | ||
| */ | ||
| export async function executeRemoteAudioQuery(page: Page): Promise<void> { |
| let context2: BrowserContext; | ||
| const maxRetries = 3; | ||
|
|
||
| test.describe('Basic Task Controls Tests', () => { |
There was a problem hiding this comment.
Why do we need this test. prefix everywhere?
Can't we simply do describe, beforeEach, beforeAll etc.,?
|
|
||
| try { | ||
| // Clear logs first to ensure clean state | ||
| clearCapturedLogs(); |
There was a problem hiding this comment.
Doesn't this happen before each test anyway?
There was a problem hiding this comment.
Yeah removed the unnecessary calls of that function. But in some cases it is needed because I wanted to clear the logs related to pause recording before looking at logs for resume recording within the same test
There was a problem hiding this comment.
Yes. Those clear log calls are alright. That is why I hadn't commented there.
| await page.waitForTimeout(2000); | ||
|
|
||
| // Verify resume callback logs | ||
| verifyHoldLogs(false); |
There was a problem hiding this comment.
It would have been better if we had the parameter as JSON. Gives more readability.
| verifyHoldLogs(false); | |
| verifyHoldLogs({expectedIsHeld: false}); |
There was a problem hiding this comment.
Should be similar everywhere we have parameters
There was a problem hiding this comment.
Yeah, this looks better. Will update the concerned functions with such parameters
|
|
||
| // Verify resume recording callback logs | ||
| verifyRecordingLogs(true); | ||
| clearCapturedLogs(); // Clear logs for next verification |
There was a problem hiding this comment.
Again, wouldn't this happen as part of beforeEach?
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| { | ||
| name: 'Test: Chrome', | ||
| // Run all tests except advanced tests and transfer/consult tests on Chrome | ||
| testIgnore: [/advanced-task-controls-test\.spec\.ts/], |
There was a problem hiding this comment.
why we are not running this in chromium?
| * Utility function to get all captured logs for debugging purposes. | ||
| * @returns Array of all captured log messages | ||
| */ | ||
| export function getAllCapturedLogs(): string[] { |
There was a problem hiding this comment.
we should remove this if not used
| * @param agentName - Name of the agent to consult with (e.g., 'User2 Agent2') | ||
| * @returns Promise<void> | ||
| */ | ||
| export async function consultViaAgent(page: Page, agentName: string = 'User2 Agent2'): Promise<void> { |
There was a problem hiding this comment.
combine 4 methods as one and differentiate using param, param should be a json
| if (!extensionPage) { | ||
| throw new Error('Extension page is not available for handling extension call'); | ||
| } |
| clearAdvancedCapturedLogs(); | ||
| }); | ||
|
|
||
| test('Normal Call Blind Transferred by Agent to Another Agent', async () => { |
There was a problem hiding this comment.
| test('Normal Call Blind Transferred by Agent to Another Agent', async () => { | |
| test('Desktop Call Blind Transferred by Agent to Another Agent', async () => { |
| await verifyCurrentState(agent1Page, USER_STATES.AVAILABLE); | ||
| }); | ||
|
|
||
| test('Normal Call Consulted via Agent and Declined (A1 → A2)', async () => { |
There was a problem hiding this comment.
| test('Normal Call Consulted via Agent and Declined (A1 → A2)', async () => { | |
| test('Desktop Call Consulted via Agent and Declined (A1 → A2)', async () => { |
| await agent1Page.waitForTimeout(2000); | ||
| }); | ||
|
|
||
| test('Normal Call Consulted via Agent and Not Picked Up by Agent 2', async () => { |
There was a problem hiding this comment.
| test('Normal Call Consulted via Agent and Not Picked Up by Agent 2', async () => { | |
| test('Desktop Call Consulted via Agent and Not Picked Up by Agent 2', async () => { |
| await agent1Page.waitForTimeout(2000); | ||
| }); | ||
|
|
||
| test('Consult Transfer - Normal Call to Agent 2', async () => { |
There was a problem hiding this comment.
| test('Consult Transfer - Normal Call to Agent 2', async () => { | |
| test('Consult Transfer - Desktop Call to Agent 2', async () => { |
| await endTask(agent1Page); | ||
| await agent1Page.waitForTimeout(3000); | ||
| await submitWrapup(agent1Page, WRAPUP_REASONS.RESOLVED); | ||
| await agent1Page.waitForTimeout(2000); |
There was a problem hiding this comment.
we should test two way audio for desktop calls, in all the consult and transfer flows
| } | ||
| })(), | ||
| (async () => { | ||
| await pageSetup(agent1Page, LOGIN_MODE.DESKTOP, 'AGENT1'); |
There was a problem hiding this comment.
lets make it Extension mode so that we can test two mode at one shot
update the tests accordingly
|
|
||
| // Verify the correct icon based on recording state | ||
| const expectedIcon = expectedIsRecording ? 'record-paused-bold' : 'record-bold'; | ||
| const actualIcon = await iconElement.getAttribute('name'); |
There was a problem hiding this comment.
We should also verify if the button in the top right (the recording indicator) is changing
|
Let the E2E tests pass and we shall merge. The reviews posted by Ravi & Me (In Ravi's review itself) should be addressed as a separate PR & ticket. |
# [1.28.0-ccwidgets.96](v1.28.0-ccwidgets.95...v1.28.0-ccwidgets.96) (2025-07-24) ### Features * **cc-widgets:** UI Automation for Basic Task Controls ([#497](#497)) ([6aa1c90](6aa1c90))
|
🎉 This PR is included in version 1.28.0-ccwidgets.96 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
COMPLETES #https://jira-eng-sjc12.cisco.com/jira/browse/CAI-6523
This pull request addresses
UI automation for testing the basic task controls
by making the following changes
https://app.vidcast.io/share/4ccc2926-2988-4150-b661-e651c529cc73
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging