Skip to content

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

Open
gcoinstash-cmd wants to merge 10 commits into
CapSoftware:mainfrom
gcoinstash-cmd:test/webrtc-session-desc-1788090834
Open

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

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 WebRTC session-description conversion and several shared recorder utility behaviors.

  • Adds offer, answer, and missing-description assertions for the Chrome extension WebRTC helper.
  • Adds recorder-core coverage for recording-mode detection, cancellation classification, and display-media retry classification.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking gap in the advertised undefined-input coverage.

The changes affect tests only, and the sole accepted concern is that one test names two nullish inputs while asserting only the null case.

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 useful WebRTC conversion coverage, but the null-or-undefined case exercises only null.
packages/recorder-core/tests/recorder-utils.test.ts Adds valid coverage for recording-mode and capture-error utility behavior without introducing a functional issue.
Prompt To Fix All With AI
### Issue 1
apps/chrome-extension/src/shared/webrtc.test.ts:83-85
**Undefined case remains untested**

This test claims to cover both null and undefined descriptions but invokes `toSessionDescriptionInit` only with null, so an undefined-input regression would leave the suite green despite the advertised coverage.

```suggestion
		expect(() => toSessionDescriptionInit(null)).toThrow(
			"Missing session description",
		);
		expect(() => toSessionDescriptionInit(undefined)).toThrow(
			"Missing session 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 +83 to +85
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 to cover both null and undefined descriptions but invokes toSessionDescriptionInit only with null, so an undefined-input regression would leave the suite green despite the advertised coverage.

Suggested change
expect(() => toSessionDescriptionInit(null)).toThrow(
"Missing session description",
);
expect(() => toSessionDescriptionInit(null)).toThrow(
"Missing session description",
);
expect(() => toSessionDescriptionInit(undefined)).toThrow(
"Missing session 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: 83-85

Comment:
**Undefined case remains untested**

This test claims to cover both null and undefined descriptions but invokes `toSessionDescriptionInit` only with null, so an undefined-input regression would leave the suite green despite the advertised coverage.

```suggestion
		expect(() => toSessionDescriptionInit(null)).toThrow(
			"Missing session description",
		);
		expect(() => toSessionDescriptionInit(undefined)).toThrow(
			"Missing session 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.

1 participant