Skip to content

test: add web-backend vitest coverage#1821

Closed
Tehlikeli107 wants to merge 1 commit into
CapSoftware:mainfrom
Tehlikeli107:codex/web-backend-vitest
Closed

test: add web-backend vitest coverage#1821
Tehlikeli107 wants to merge 1 commit into
CapSoftware:mainfrom
Tehlikeli107:codex/web-backend-vitest

Conversation

@Tehlikeli107
Copy link
Copy Markdown

@Tehlikeli107 Tehlikeli107 commented May 14, 2026

Summary

  • add a Vitest test script and config for @cap/web-backend
  • add package-level example tests for effective video rules, Google Drive object-key parsing, and signed storage object tokens
  • keep the lockfile change limited to the packages/web-backend importer

Verification

  • corepack pnpm install --filter @cap/web-backend... --frozen-lockfile --ignore-scripts
  • corepack pnpm --filter @cap/web-backend test
  • corepack pnpm exec biome check packages/web-backend/package.json packages/web-backend/vitest.config.ts packages/web-backend/src/Videos/EffectiveVideoRules.test.ts packages/web-backend/src/Storage/GoogleDrive.test.ts packages/web-backend/src/Storage/SignedObject.test.ts
  • corepack pnpm exec tsc -p packages/web-backend/tsconfig.json --noEmit

This is a focused follow-up for the current architecture and avoids the desktop/utils/sdk scopes already covered by other open testing attempts.

/claim #54

Greptile Summary

This PR introduces Vitest coverage for @cap/web-backend, adding a config, a test script, and three test files targeting EffectiveVideoRules, parseVideoIdFromObjectKey, and the SignedObject token helpers.

  • EffectiveVideoRules.test.ts tests pure functions against no external state and is clean end-to-end.
  • GoogleDrive.test.ts correctly uses a static import because parseVideoIdFromObjectKey is env-independent; the assertion logic matches the Option.filter implementation precisely.
  • SignedObject.test.ts applies the vi.resetModules() + dynamic-import pattern to isolate @cap/env's cached state across tests, and the fake-timer expiry boundary check is correct (60_001 ms past a 60 s TTL).

Confidence Score: 4/5

Safe to merge — the changes are additive test files only, with no modifications to production code paths.

All three test files exercise the correct production logic and would catch real regressions. The only observations are minor style points in SignedObject.test.ts: a redundant setRequiredEnv() in beforeEach and unnecessary optional chaining on a value that is always defined.

packages/web-backend/src/Storage/SignedObject.test.ts has the minor style issues noted; the other test files and config are clean.

Important Files Changed

Filename Overview
packages/web-backend/vitest.config.ts New vitest config using node environment, standard include/exclude patterns — looks correct
packages/web-backend/package.json Adds test script and vitest ~2.1.9 devDependency; straightforward and correctly scoped
packages/web-backend/src/Videos/EffectiveVideoRules.test.ts Tests pure functions with no side-effects or env dependencies; coverage of defaults, inherited settings, and password hashing looks thorough
packages/web-backend/src/Storage/GoogleDrive.test.ts Correctly tests the pure parseVideoIdFromObjectKey with a static import; assertions are accurate against the implementation
packages/web-backend/src/Storage/SignedObject.test.ts Uses vi.resetModules() + dynamic import pattern to isolate env-sensitive module; setRequiredEnv() in importSignedObject() is redundant with beforeEach but harmless; expiry boundary and tamper tests are correct
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/web-backend/src/Storage/SignedObject.test.ts:32-36
The `setRequiredEnv()` call in `beforeEach` is redundant: `importSignedObject()` always calls `vi.resetModules()` first (which clears `@cap/env`'s cached state) and then calls `setRequiredEnv()` itself before the dynamic import. The `beforeEach` call fires before the module reset happens, so it's the import-time call inside `importSignedObject()` that's meaningful. Having the call in both places obscures which one is load-order critical.

```suggestion
	beforeEach(() => {
		vi.useFakeTimers();
		vi.setSystemTime(new Date("2026-05-15T00:00:00.000Z"));
	});
```

### Issue 2 of 2
packages/web-backend/src/Storage/SignedObject.test.ts:67-70
`signature` is always defined here because `createStorageObjectToken` always returns a string in the form `encodedPayload.signature`, and neither segment contains a `.`. The optional chaining (`?.`) is misleading — if `signature` were ever `undefined`, `signature?.slice(0, -1)` would evaluate to `undefined`, and the template literal would produce `"undefinedA"` or `"undefinedB"`, which coincidentally would still differ from the real signature. Using a non-optional access makes the intent explicit.

```suggestion
		const [payload, signature] = token.split(".");
		if (!payload || !signature) throw new Error("unexpected token format");
		const changedSignature = `${signature.slice(0, -1)}${
			signature.endsWith("A") ? "B" : "A"
		}`;
```

Reviews (1): Last reviewed commit: "test: add web-backend vitest coverage" | 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 +32 to +36
beforeEach(() => {
setRequiredEnv();
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-05-15T00:00:00.000Z"));
});
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 setRequiredEnv() call in beforeEach is redundant: importSignedObject() always calls vi.resetModules() first (which clears @cap/env's cached state) and then calls setRequiredEnv() itself before the dynamic import. The beforeEach call fires before the module reset happens, so it's the import-time call inside importSignedObject() that's meaningful. Having the call in both places obscures which one is load-order critical.

Suggested change
beforeEach(() => {
setRequiredEnv();
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-05-15T00:00:00.000Z"));
});
beforeEach(() => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-05-15T00:00:00.000Z"));
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/web-backend/src/Storage/SignedObject.test.ts
Line: 32-36

Comment:
The `setRequiredEnv()` call in `beforeEach` is redundant: `importSignedObject()` always calls `vi.resetModules()` first (which clears `@cap/env`'s cached state) and then calls `setRequiredEnv()` itself before the dynamic import. The `beforeEach` call fires before the module reset happens, so it's the import-time call inside `importSignedObject()` that's meaningful. Having the call in both places obscures which one is load-order critical.

```suggestion
	beforeEach(() => {
		vi.useFakeTimers();
		vi.setSystemTime(new Date("2026-05-15T00:00:00.000Z"));
	});
```

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

Comment on lines +67 to +70
const [payload, signature] = token.split(".");
const changedSignature = `${signature?.slice(0, -1)}${
signature?.endsWith("A") ? "B" : "A"
}`;
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 signature is always defined here because createStorageObjectToken always returns a string in the form encodedPayload.signature, and neither segment contains a .. The optional chaining (?.) is misleading — if signature were ever undefined, signature?.slice(0, -1) would evaluate to undefined, and the template literal would produce "undefinedA" or "undefinedB", which coincidentally would still differ from the real signature. Using a non-optional access makes the intent explicit.

Suggested change
const [payload, signature] = token.split(".");
const changedSignature = `${signature?.slice(0, -1)}${
signature?.endsWith("A") ? "B" : "A"
}`;
const [payload, signature] = token.split(".");
if (!payload || !signature) throw new Error("unexpected token format");
const changedSignature = `${signature.slice(0, -1)}${
signature.endsWith("A") ? "B" : "A"
}`;
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/web-backend/src/Storage/SignedObject.test.ts
Line: 67-70

Comment:
`signature` is always defined here because `createStorageObjectToken` always returns a string in the form `encodedPayload.signature`, and neither segment contains a `.`. The optional chaining (`?.`) is misleading — if `signature` were ever `undefined`, `signature?.slice(0, -1)` would evaluate to `undefined`, and the template literal would produce `"undefinedA"` or `"undefinedB"`, which coincidentally would still differ from the real signature. Using a non-optional access makes the intent explicit.

```suggestion
		const [payload, signature] = token.split(".");
		if (!payload || !signature) throw new Error("unexpected token format");
		const changedSignature = `${signature.slice(0, -1)}${
			signature.endsWith("A") ? "B" : "A"
		}`;
```

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

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