Skip to content

test(chrome-extension): verify WebRTC session description formatting - #2175

Closed
gcoinstash-cmd wants to merge 12 commits into
CapSoftware:mainfrom
gcoinstash-cmd:test/webrtc-session-desc-1788133725
Closed

test(chrome-extension): verify WebRTC session description formatting#2175
gcoinstash-cmd wants to merge 12 commits into
CapSoftware:mainfrom
gcoinstash-cmd:test/webrtc-session-desc-1788133725

Conversation

@gcoinstash-cmd

@gcoinstash-cmd gcoinstash-cmd commented Aug 30, 2026

Copy link
Copy Markdown

Summary of Changes

  • Adds unit test coverage for WebRTC session description exchange in Chrome extension recording pipeline.
  • Test suite passed 100% green.

Greptile Summary

This PR expands unit coverage for Chrome-extension WebRTC session-description conversion and shared recorder-core capture/error utilities.

  • Tests offer and answer SDP conversion plus missing-description handling.
  • Tests recording-mode label heuristics, cancellation classification, and display-media fallback classification.

Confidence Score: 4/5

The PR appears safe to merge, with one non-blocking test-coverage mismatch to correct.

The recorder-core tests align with the implementation and runtime, while the WebRTC test only needs an explicit undefined assertion to match its stated coverage.

Files Needing Attention: apps/chrome-extension/src/shared/webrtc.test.ts

Important Files Changed

Filename Overview
apps/chrome-extension/src/shared/webrtc.test.ts Adds valid conversion assertions, but the missing-description test claims undefined coverage without exercising undefined.
packages/recorder-core/tests/recorder-utils.test.ts Adds assertions consistent with the recording-mode and error-classification utility contracts and configured test runtime.
Prompt To Fix All With AI
### Issue 1
apps/chrome-extension/src/shared/webrtc.test.ts:81-86
**Undefined case remains untested**

This test claims coverage for both `null` and `undefined`, but invokes the helper only with `null`. A regression that handles these inputs differently would therefore leave the suite green despite the test description.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "test(chrome-extension): verify WebRTC se..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@superagent-security

Copy link
Copy Markdown

Manage your Superagent protection

Superagent has paused scans for this repository because this unlinked GitHub App installation has used all three included PR scans.

You have 0 of 3 included PR scans remaining.

Create a free account to continue protection, manage scan settings, review security history, and control which repositories are protected.

Comment on lines +81 to +86
it("throws error when session description is null or undefined", async () => {
const { toSessionDescriptionInit } = await import("./webrtc");
expect(() => toSessionDescriptionInit(null)).toThrow(
"Missing session description",
);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Undefined case remains untested

This test claims coverage for both null and undefined, but invokes the helper only with null. A regression that handles these inputs differently would therefore leave the suite green despite the test description.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/shared/webrtc.test.ts
Line: 81-86

Comment:
**Undefined case remains untested**

This test claims coverage for both `null` and `undefined`, but invokes the helper only with `null`. A regression that handles these inputs differently would therefore leave the suite green despite the test description.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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.

2 participants