Skip to content

test: add package-level Vitest examples#1819

Closed
zhengjynicolas wants to merge 2 commits into
CapSoftware:mainfrom
zhengjynicolas:codex/package-test-bootstrap
Closed

test: add package-level Vitest examples#1819
zhengjynicolas wants to merge 2 commits into
CapSoftware:mainfrom
zhengjynicolas:codex/package-test-bootstrap

Conversation

@zhengjynicolas
Copy link
Copy Markdown

@zhengjynicolas zhengjynicolas commented May 14, 2026

Summary

  • Add test scripts for @cap/web-domain, @cap/sdk-embed, and @cap/sdk-recorder so these packages participate in the existing Turbo test pipeline.
  • Add small Vitest examples for domain image/video helpers, embed URL construction, and recorder MIME type selection.
  • Update the lockfile only for the new package-level Vitest importer entries.

Verification

  • pnpm install --frozen-lockfile --ignore-scripts
  • pnpm turbo run test --filter=@cap/web-domain --filter=@cap/sdk-embed --filter=@cap/sdk-recorder
  • pnpm exec biome check packages/web-domain/package.json packages/web-domain/src/ImageUpload.test.ts packages/web-domain/src/Video.test.ts packages/sdk-embed/package.json packages/sdk-embed/src/vanilla/cap-embed.test.ts packages/sdk-recorder/package.json packages/sdk-recorder/src/core/mime-types.test.ts pnpm-lock.yaml
  • pnpm --filter @cap/web-domain build
  • pnpm --filter @cap/sdk-embed typecheck

/claim #54

Greptile Summary

Wires three packages (@cap/web-domain, @cap/sdk-embed, @cap/sdk-recorder) into the existing Turbo test pipeline by adding "test": "vitest run" scripts and vitest devDependencies, then supplies a small set of Vitest unit tests for each.

  • web-domainImageUpload.test.ts covers all four branches of extractFileKey (plain key, virtual-hosted URL, path-style S3 URL, Google URL); Video.test.ts covers file-key derivation for Mp4Source, M3U8Source, SegmentsSource, and normalizeSegmentEntry — all assertions match the implementation.
  • sdk-embedcap-embed.test.ts tests default and custom-base createEmbedUrl output; the second test omits assertions for the unconditionally-set sdk and pk params.
  • sdk-recordermime-types.test.ts tests fallback to empty string and fallback from vp9,opus to vp8,opus, but never exercises the case where both are supported, so the priority ordering is not directly verified.

Confidence Score: 4/5

Safe to merge — only adds test infrastructure and unit tests with no changes to production code paths.

The production code is untouched; the only risk is that a couple of test cases have gaps (missing pk/sdk assertions in cap-embed.test.ts and missing vp9-priority case in mime-types.test.ts) that could let quiet regressions slip through future changes to those modules.

mime-types.test.ts and cap-embed.test.ts have the coverage gaps noted above; the web-domain tests are solid.

Important Files Changed

Filename Overview
packages/sdk-embed/src/vanilla/cap-embed.test.ts Tests createEmbedUrl; the second test omits assertions for the sdk and pk params that are also set unconditionally, leaving a silent regression path.
packages/sdk-recorder/src/core/mime-types.test.ts Tests MIME-type selection; missing a case where both vp9,opus and vp8,opus are supported, so the stated preference ordering is never directly exercised.
packages/web-domain/src/ImageUpload.test.ts Covers key/URL/path-style/Google-URL branches of extractFileKey; test inputs and expected outputs match the implementation.
packages/web-domain/src/Video.test.ts Tests file-key derivation for all three source types and normalizeSegmentEntry; all expected values match the implementation's padding/path logic.
pnpm-lock.yaml Adds three importer entries for vitest@3.2.4, reusing the already-resolved peer-dependency set; no new package versions introduced.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/sdk-recorder/src/core/mime-types.test.ts:22-26
The test title claims it verifies "first supported MIME type by preference order", but the mock only marks `vp8,opus` and `video/webm` as supported — so `vp9,opus` is simply absent. The test never exercises the case where a higher-priority type wins over a lower-priority one that is also available. If someone accidentally swapped the first two entries in `PREFERRED_MIME_TYPES`, this test would still pass. Adding a case where both `vp9,opus` and `vp8,opus` are supported confirms the priority ordering is actually enforced.

```suggestion
	it("returns the first supported MIME type by preference order", () => {
		setSupportedMimeTypes(["video/webm;codecs=vp8,opus", "video/webm"]);

		expect(getSupportedMimeType()).toBe("video/webm;codecs=vp8,opus");
	});

	it("prefers vp9,opus over vp8,opus when both are supported", () => {
		setSupportedMimeTypes([
			"video/webm;codecs=vp9,opus",
			"video/webm;codecs=vp8,opus",
		]);

		expect(getSupportedMimeType()).toBe("video/webm;codecs=vp9,opus");
	});
```

### Issue 2 of 2
packages/sdk-embed/src/vanilla/cap-embed.test.ts:34-36
The second test exercises `publicKey` and `sdk` params but never asserts their values. `pk` and `sdk` could silently regress (e.g., a rename or removal) while this test still passes. Adding the two assertions closes that blind spot without any extra setup.

```suggestion
		expect(url.origin).toBe("https://app.example.com");
		expect(url.pathname).toBe("/embed/video_456");
		expect(url.searchParams.get("sdk")).toBe("1");
		expect(url.searchParams.get("pk")).toBe("pk_live_456");
		expect(url.searchParams.get("autoplay")).toBe("1");
```

Reviews (1): Last reviewed commit: "test: add package-level vitest examples" | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

@superagent-security superagent-security Bot added contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis. labels May 14, 2026
Comment on lines +22 to +26
it("returns the first supported MIME type by preference order", () => {
setSupportedMimeTypes(["video/webm;codecs=vp8,opus", "video/webm"]);

expect(getSupportedMimeType()).toBe("video/webm;codecs=vp8,opus");
});
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 The test title claims it verifies "first supported MIME type by preference order", but the mock only marks vp8,opus and video/webm as supported — so vp9,opus is simply absent. The test never exercises the case where a higher-priority type wins over a lower-priority one that is also available. If someone accidentally swapped the first two entries in PREFERRED_MIME_TYPES, this test would still pass. Adding a case where both vp9,opus and vp8,opus are supported confirms the priority ordering is actually enforced.

Suggested change
it("returns the first supported MIME type by preference order", () => {
setSupportedMimeTypes(["video/webm;codecs=vp8,opus", "video/webm"]);
expect(getSupportedMimeType()).toBe("video/webm;codecs=vp8,opus");
});
it("returns the first supported MIME type by preference order", () => {
setSupportedMimeTypes(["video/webm;codecs=vp8,opus", "video/webm"]);
expect(getSupportedMimeType()).toBe("video/webm;codecs=vp8,opus");
});
it("prefers vp9,opus over vp8,opus when both are supported", () => {
setSupportedMimeTypes([
"video/webm;codecs=vp9,opus",
"video/webm;codecs=vp8,opus",
]);
expect(getSupportedMimeType()).toBe("video/webm;codecs=vp9,opus");
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/sdk-recorder/src/core/mime-types.test.ts
Line: 22-26

Comment:
The test title claims it verifies "first supported MIME type by preference order", but the mock only marks `vp8,opus` and `video/webm` as supported — so `vp9,opus` is simply absent. The test never exercises the case where a higher-priority type wins over a lower-priority one that is also available. If someone accidentally swapped the first two entries in `PREFERRED_MIME_TYPES`, this test would still pass. Adding a case where both `vp9,opus` and `vp8,opus` are supported confirms the priority ordering is actually enforced.

```suggestion
	it("returns the first supported MIME type by preference order", () => {
		setSupportedMimeTypes(["video/webm;codecs=vp8,opus", "video/webm"]);

		expect(getSupportedMimeType()).toBe("video/webm;codecs=vp8,opus");
	});

	it("prefers vp9,opus over vp8,opus when both are supported", () => {
		setSupportedMimeTypes([
			"video/webm;codecs=vp9,opus",
			"video/webm;codecs=vp8,opus",
		]);

		expect(getSupportedMimeType()).toBe("video/webm;codecs=vp9,opus");
	});
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +34 to +36
expect(url.origin).toBe("https://app.example.com");
expect(url.pathname).toBe("/embed/video_456");
expect(url.searchParams.get("autoplay")).toBe("1");
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 The second test exercises publicKey and sdk params but never asserts their values. pk and sdk could silently regress (e.g., a rename or removal) while this test still passes. Adding the two assertions closes that blind spot without any extra setup.

Suggested change
expect(url.origin).toBe("https://app.example.com");
expect(url.pathname).toBe("/embed/video_456");
expect(url.searchParams.get("autoplay")).toBe("1");
expect(url.origin).toBe("https://app.example.com");
expect(url.pathname).toBe("/embed/video_456");
expect(url.searchParams.get("sdk")).toBe("1");
expect(url.searchParams.get("pk")).toBe("pk_live_456");
expect(url.searchParams.get("autoplay")).toBe("1");
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/sdk-embed/src/vanilla/cap-embed.test.ts
Line: 34-36

Comment:
The second test exercises `publicKey` and `sdk` params but never asserts their values. `pk` and `sdk` could silently regress (e.g., a rename or removal) while this test still passes. Adding the two assertions closes that blind spot without any extra setup.

```suggestion
		expect(url.origin).toBe("https://app.example.com");
		expect(url.pathname).toBe("/embed/video_456");
		expect(url.searchParams.get("sdk")).toBe("1");
		expect(url.searchParams.get("pk")).toBe("pk_live_456");
		expect(url.searchParams.get("autoplay")).toBe("1");
```

How can I resolve this? If you propose a fix, please make it concise.

@zhengjynicolas
Copy link
Copy Markdown
Author

Addressed the two Greptile coverage gaps in follow-up commit e2bfbe8:\n\n- added a MIME priority test where both vp9/opus and vp8/opus are supported, asserting vp9 wins\n- added sdk/pk assertions to the custom embed URL test\n\nVerification after the update:\n- pnpm turbo run test --filter=@cap/sdk-embed --filter=@cap/sdk-recorder\n- pnpm exec biome check packages/sdk-embed/src/vanilla/cap-embed.test.ts packages/sdk-recorder/src/core/mime-types.test.ts\n- git diff --check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🙋 Bounty claim contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants