test(calling): add call history e2e flow#4994
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bf0733e67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const callerRecord = await waitForCallHistoryRecord( | ||
| callerPage, | ||
| (record) => | ||
| recordMatchesCallCase(record, { | ||
| counterpartNumber: calleeNumber, | ||
| direction: 'OUTGOING', | ||
| startedAt, | ||
| dispositions: UNANSWERED_CALLER_DISPOSITIONS, | ||
| }), |
There was a problem hiding this comment.
Because this serial suite runs the missed-call test immediately before this rejected-call test, these predicates can return the previous missed call: isRecentRecord() accepts rows up to 120 seconds before startedAt, the counterpart/direction are identical, and the callee-side rejected dispositions include MISSED. If the rejected history row is not created yet, waitForCallHistoryRecord() can pass using the earlier missed-call records, so the test no longer verifies that rejecting the new call wrote history. Snapshot existing record keys before placing this call or use the waitForNewHistoryRecord() helper here as the journey test does.
Useful? React with 👍 / 👎.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9babdedee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const callerCanceledRecord = await waitForCallHistoryRecord( | ||
| callerPage, | ||
| (record) => | ||
| recordMatchesCallCase(record, { | ||
| counterpartNumber: calleeNumber, | ||
| direction: 'OUTGOING', | ||
| startedAt, | ||
| dispositions: UNANSWERED_CALLER_DISPOSITIONS, | ||
| }), |
There was a problem hiding this comment.
In this serial suite, the answered-call test runs immediately before this missed-call test, and the caller-side matcher here accepts INITIATED for unanswered calls while the answered caller record can also be INITIATED (ANSWERED_DISPOSITIONS includes it). Because isRecentRecord() allows records up to 120 seconds before this call's startedAt, waitForCallHistoryRecord() can pass by returning the previous answered outgoing row before the new missed-call row exists, so the test may not verify caller history for this missed call. Snapshot existing keys before placing the call, or use the new-record helper as the journey test does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 239129e5fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const callerRecord = await waitForCallHistoryRecord( | ||
| callerPage, | ||
| (record) => | ||
| recordMatchesCallCase(record, { | ||
| counterpartNumber: calleeNumber, | ||
| direction: 'OUTGOING', | ||
| startedAt, | ||
| dispositions: ANSWERED_DISPOSITIONS, | ||
| }), | ||
| 'caller outgoing answered call' | ||
| ); |
There was a problem hiding this comment.
In the SET_2USER suite, callHistoryTests() runs immediately after callLifecycleTests()/callLifecycleMediaTests() (see set-2user.spec.ts), which already create answered calls between the same two accounts. Since isRecentRecord() accepts records up to 120 seconds before this test's startedAt and these predicates only match counterpart/direction/answered disposition, this can return a lifecycle record before the new answered call is written, so the test may pass without verifying this call's history. Snapshot existing history keys before placing the call or use the new-record helper used by the journey test.
Useful? React with 👍 / 👎.
| backup: {region: string; uris: string[]}; | ||
| }; | ||
|
|
||
| const CALLING_UMD_BUNDLE_PATH = path.resolve(__dirname, '../../../webex/umd/calling.min.js'); |
There was a problem hiding this comment.
Is this required to run call history? Does the default sample app not support CallHistory?
There was a problem hiding this comment.
This is not required. The default sample app already supports CallHistory through callHistory: true and exposes window.callHistory. I removed the custom UMD bundle routing so the tests use the default sample app bundle.
| await page.route('**/samples/calling.min.js', (route) => | ||
| route.fulfill({ | ||
| path: CALLING_UMD_BUNDLE_PATH, | ||
| contentType: 'application/javascript', | ||
| }) | ||
| ); | ||
| await page.route('**/samples/calling.min.js.map', (route) => | ||
| route.fulfill({ | ||
| path: CALLING_UMD_BUNDLE_MAP_PATH, | ||
| contentType: 'application/json', | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Addressed.The tests now use the default sample app bundle directly, since the sample app already supports CallHistory.
| 'Run three User 1 to User 2 outgoing call attempts: missed, rejected, and answered.', | ||
| 'Run three User 2 to User 1 outgoing call attempts: missed, rejected, and answered.', | ||
| 'Open the sample app Call History list for both users.', | ||
| ], | ||
| expected: [ | ||
| 'User 1 history contains the three recent outgoing User 1 to User 2 records.', | ||
| 'User 2 history contains the three recent outgoing User 2 to User 1 records.', | ||
| 'The table header renders the Call History columns and all expected rows are visible.', | ||
| ], | ||
| }, | ||
| { | ||
| id: 'CH-CALL-001', | ||
| title: 'Answered call creates exact per-user history records', | ||
| preconditions: ['Caller and callee are registered and can place/answer calls.'], | ||
| steps: [ | ||
| 'Caller dials callee.', | ||
| 'Callee answers and the call reaches established state on both pages.', | ||
| 'Caller ends the call.', | ||
| 'Poll Call History for caller and callee.', | ||
| 'Render each returned record through the Call History UI table.', | ||
| ], | ||
| expected: [ | ||
| 'Caller history contains a recent OUTGOING record for the callee.', | ||
| 'Callee history contains a recent INCOMING record for the caller.', | ||
| 'Each UI row shows the exact Janus direction, disposition, start time, end time, and session type.', | ||
| 'Call timing is valid: start/end are ISO dates, end is not before start, and duration is plausible.', | ||
| ], | ||
| }, | ||
| { | ||
| id: 'CH-CALL-002', | ||
| title: 'Missed call creates exact callee MISSED history', | ||
| preconditions: ['Caller and callee are registered; callee does not answer the incoming call.'], | ||
| steps: [ | ||
| 'Caller dials callee.', | ||
| 'Callee rings and intentionally does not answer.', | ||
| 'Caller ends the ringing call.', | ||
| 'Poll Call History for caller and callee.', | ||
| 'Render the returned records through the Call History UI table.', | ||
| ], | ||
| expected: [ | ||
| 'Callee history contains a recent INCOMING MISSED record for the caller.', | ||
| 'Caller history contains a recent OUTGOING unanswered record for the callee.', | ||
| 'The UI reflects the exact backend status and call timing for each user.', | ||
| ], | ||
| }, | ||
| { | ||
| id: 'CH-CALL-003', | ||
| title: 'Rejected call creates exact per-user history records', | ||
| preconditions: ['Caller and callee are registered; callee can reject the incoming call.'], | ||
| steps: [ | ||
| 'Caller dials callee.', | ||
| 'Callee rejects the ringing call from the UI.', | ||
| 'Wait until both users have no active calls.', | ||
| 'Poll Call History for caller and callee.', | ||
| 'Render the returned records through the Call History UI table.', | ||
| ], | ||
| expected: [ | ||
| 'Caller and callee both receive recent history rows for the rejected call.', | ||
| 'The UI renders the exact Janus direction, disposition, start time, end time, and session type.', | ||
| ], | ||
| }, | ||
| { | ||
| id: 'CH-ALL-001', | ||
| title: 'User 1 login shows missed, rejected, and answered outgoing call history', | ||
| preconditions: ['Both users stay initialized, registered, and media-ready in one browser run.'], | ||
| steps: [ | ||
| 'User 1 calls User 2 and User 2 does not answer.', | ||
| 'User 1 calls User 2 again and User 2 rejects.', | ||
| 'User 1 calls User 2 again and User 2 answers.', | ||
| 'After both users complete their outgoing call journeys, open Call History for User 1.', | ||
| ], | ||
| expected: [ | ||
| 'User 1 shows three recent OUTGOING records in one login.', | ||
| 'The missed, rejected, and answered attempts show the backend dispositions returned by Call History.', | ||
| 'Every record has valid start and end timestamps.', | ||
| 'The answered call includes valid non-negative duration derived from the timestamps.', | ||
| 'The sample app Call History table renders all three outgoing records for User 1.', | ||
| ], | ||
| }, | ||
| { | ||
| id: 'CH-ALL-002', | ||
| title: 'User 2 login shows missed, rejected, and answered outgoing call history', | ||
| preconditions: ['Both users stay initialized, registered, and media-ready in one browser run.'], | ||
| steps: [ | ||
| 'User 2 calls User 1 and User 1 does not answer.', | ||
| 'User 2 calls User 1 again and User 1 rejects.', | ||
| 'User 2 calls User 1 again and User 1 answers.', | ||
| 'After both users complete their outgoing call journeys, open Call History for User 2.', | ||
| ], | ||
| expected: [ | ||
| 'User 2 shows three recent OUTGOING records in one login.', | ||
| 'The missed, rejected, and answered attempts show the backend dispositions returned by Call History.', | ||
| 'Every record has valid start and end timestamps.', | ||
| 'The answered call includes valid non-negative duration derived from the timestamps.', | ||
| 'The sample app Call History table renders all three outgoing records for User 2.', | ||
| ], | ||
| }, | ||
| ] as const; |
There was a problem hiding this comment.
The specs shouldn't be in the code.
There was a problem hiding this comment.
Fixed. I removed the specs metadata from the test group and replaced it with explicit Playwright test cases. Since GitHub marks this thread as outdated, this should be covered by the latest changes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9faf23f07e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const REJECTED_CALLEE_DISPOSITIONS = ['REJECTED', 'MISSED', 'CANCELED']; | ||
| const HISTORY_TIME_LOOKBACK_MS = 5000; | ||
| const RECENT_RECORD_TOLERANCE_MS = 120000; | ||
| const COUNTERPART_MATCH_MIN_DIGITS = 4; |
There was a problem hiding this comment.
Using only the last 4 digits to identify the counterpart can match unrelated call-history rows whenever two destinations share a suffix (common with enterprise extensions/DIDs), so waitForCallHistoryRecord() can resolve the wrong record and let assertions pass without validating the intended call. The helper default is already 7 digits, but this suite overrides it to 4, which materially weakens record identity in serial flows with many recent calls.
Useful? React with 👍 / 👎.
| record.other?.callbackAddress, | ||
| record.other?.primaryDisplayString, | ||
| record.other?.secondaryDisplayString, | ||
| record.other?.name, |
There was a problem hiding this comment.
Including record.other?.name in phoneMatchesRecord() can produce false matches because this field is a display label, not a canonical phone/address field, and it may contain digits (e.g., extensions/team labels). Since matching is suffix-based, a numeric name can satisfy the predicate even when the real callback/phone fields belong to a different call, causing history polling to select the wrong row.
Useful? React with 👍 / 👎.
| // Account roles resolved from testInfo.project.name → USER_SETS. | ||
| callLifecycleTests(); | ||
| callLifecycleMediaTests(); | ||
| callHistoryTests(); |
There was a problem hiding this comment.
We can use a different suite for this. The call history tests can be run in parallel
There was a problem hiding this comment.
I moved Call History into its own suite: set-2user-call-history.spec.ts, and added separate SET_2USER_CALL_HISTORY Playwright projects so it can run independently/parallel where account dependencies allow.
| .locator(CALLING_SELECTORS.DESTINATION_INPUT) | ||
| .fill(destination, {timeout: AWAIT_TIMEOUT}); | ||
| await page.locator(CALLING_SELECTORS.MAKE_CALL_BTN).click({timeout: AWAIT_TIMEOUT}); | ||
| await expect(page.locator(CALLING_SELECTORS.MAKE_CALL_BTN)).toBeDisabled({ |
There was a problem hiding this comment.
Why did we need these changes ? How are these related to call history tests ?
There was a problem hiding this comment.
These were not directly related to Call History. I removed the unrelated call.ts changes and kept the Call History-specific waits/assertions inside the Call History utilities/tests.
| * Navigate to the calling sample app | ||
| */ | ||
| export const navigateToCallingApp = async (page: Page): Promise<void> => { | ||
| await page.route('**/samples/calling.min.js', (route) => |
There was a problem hiding this comment.
Same comment here. What is this change about ?
There was a problem hiding this comment.
This override is not needed because the default sample app already supports CallHistory. I removed the page.route bundle override from navigateToCallingApp.
| } from '../utils/call-history'; | ||
| import {CALLING_SELECTORS} from '../constants'; | ||
|
|
||
| const CALL_HISTORY_AI_SPECS = [ |
There was a problem hiding this comment.
Why do we have these specs added in the testgroup ? Shouldn't this have testcases ?
There was a problem hiding this comment.
Those spec objects were redundant metadata and did not belong in the test group. I removed them and changed the file to use explicit Playwright test cases with the test IDs in the test names.
| }, | ||
| ] as const; | ||
|
|
||
| const ANSWERED_DISPOSITIONS = ['ANSWERED', 'INITIATED']; |
There was a problem hiding this comment.
These constants should be in constants file
There was a problem hiding this comment.
Done. I moved the Call History disposition/timing constants into playwright/constants/call-history.ts and export them through playwright/constants/index.ts.
|
|
||
| type UserLabel = HistoryDebugRecord['user']; | ||
|
|
||
| type CallJourneyLeg = { |
There was a problem hiding this comment.
These types also shouldn't be pasrt of test group. If we these types for testcases, then move to a separate file and import from there
There was a problem hiding this comment.
Done. I moved the Call History journey/matcher/debug types into playwright/utils/call-history-types.ts and import them where needed.
| return matchingRecord as CallHistoryRecord; | ||
| }; | ||
|
|
||
| const clearCallHistoryTable = async (page: Page): Promise<void> => { |
There was a problem hiding this comment.
Why do we have so many util methods created ? I see lot of them here as well as in test-group itself
There was a problem hiding this comment.
I cleaned this up. The test group now mostly contains the actual test cases. Shared low-level API/UI helpers remain in utils/call-history.ts, and journey-specific orchestration is isolated in utils/call-history-journey.ts so the test cases stay readable and we avoid duplicating polling, matching, and UI verification logic.
| @@ -0,0 +1,4 @@ | |||
| import {callHistoryTests} from '../test-groups/call-history'; | |||
There was a problem hiding this comment.
name doesnt have to set say set-2. If we have single file for call history tests then stating call-history-spec.ts in the name should be enough
There was a problem hiding this comment.
Done, renamed the suite file to call-history.spec.ts and updated the USER_SETS mapping accordingly.
| @@ -0,0 +1,45 @@ | |||
| import type {Page} from '@playwright/test'; | |||
There was a problem hiding this comment.
nitpick: are the types in these files used in multiple other files? im seeing type definitions in packages/calling/playwright/utils/call-history.ts as well? either those can be moved here or these definitions can be moved to the file they are used it.
| const HISTORY_URL_PATTERN = '**/history/userSessions**'; | ||
| const HISTORY_EVENTUAL_CONSISTENCY_TIMEOUT = 150000; | ||
| const HISTORY_POLL_INTERVALS = [5000, 5000, 10000, 10000, 15000]; | ||
| const TIMING_TOLERANCE_MS = 120000; | ||
| const DURATION_TOLERANCE_SECONDS = 120; |
There was a problem hiding this comment.
move these to constants as well
|
|
||
| // Call History uses USER_1+USER_2 after their single-user suites complete, | ||
| // so it can run alongside the SET_2USER call lifecycle suite in PROD. | ||
| SET_2USER_CALL_HISTORY: { |
There was a problem hiding this comment.
you can call it SET_CALL_HISTORY since we are dropping the 2USER/3USER naming convention
| // Call History has its own suite and can run in parallel with SET_2USER - PROD | ||
| // because it uses USER_1+USER_2 after those single-user suites complete. | ||
| { | ||
| name: 'SET_2USER_CALL_HISTORY - PROD', |
There was a problem hiding this comment.
same here.
you can call it
SET_CALL_HISTORYsince we are dropping the 2USER/3USER naming convention
| }, | ||
| // INT aliases overlap between USER_1/2 and USER_4/5, so keep INT ordered. | ||
| { | ||
| name: 'SET_2USER_CALL_HISTORY - INT', |
4 participants
COMPLETES #https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7876
This pull request addresses
Adds automated Playwright e2e coverage for the Webex Calling Call History flow in the two-user calling suite. The tests validate that call history records are created and rendered correctly for answered, missed, and rejected call scenarios.
by making the following changes
SET_2USERsuite.Change Type
The following scenarios were tested
SET_2USER.The GAI Coding Policy And Copyright Annotation Best Practices
I certified that