Skip to content

test: wire desktop and utils coverage into turbo #1808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
svperior-jon wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −2
Open

test: wire desktop and utils coverage into turbo #1808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/desktop/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"localdev": "dotenv -e ../../.env -- vinxi dev --port 3002",
"build": "vinxi build",
"tauri": "tauri",
"test": "vitest run",
"test:memory": "node scripts/desktop-memory-soak.js",
"test:memory:unit": "vitest run scripts/desktop-memory-soak.test.js"
},
Expand Down
6 changes: 4 additions & 2 deletions packages/utils/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
},
"scripts": {
"typecheck": "tsc -b",
"build": "tsdown"
"build": "tsdown",
"test": "vitest run"
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 12, 2026

Choose a reason for hiding this comment

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

P2 @cap/utils has no vitest.config.ts. Vitest will default to Node environment and the standard **/*.test.* glob, which works for the current pure-function tests. As the package grows (or if path aliases from tsconfig are needed), the absence of an explicit config may cause friction. Consider adding a minimal config now to make the test environment intentional.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/utils/package.json
Line: 10

Comment:
`@cap/utils` has no `vitest.config.ts`. Vitest will default to Node environment and the standard `**/*.test.*` glob, which works for the current pure-function tests. As the package grows (or if path aliases from `tsconfig` are needed), the absence of an explicit config may cause friction. Consider adding a minimal config now to make the test environment intentional.

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

},
"publishConfig": {
"main": "./dist/index.js"
Expand All @@ -16,7 +17,8 @@
"react-dom": "^19.1.1",
"react-router-dom": "^6.18.0",
"tsconfig": "workspace:*",
"typescript": "^5.8.3"
"typescript": "^5.8.3",
"vitest": "~2.1.9"
},
"dependencies": {
"@aws-sdk/client-s3": "^3.485.0",
Expand Down
50 changes: 50 additions & 0 deletions packages/utils/src/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { describe, expect, it } from "vitest";

import {
calculateStrokeDashoffset,
classNames,
getDisplayProgress,
getProgressCircleConfig,
isEmailAllowedByRestriction,
uuidFormat,
uuidParse,
} from "./helpers";

describe("helpers", () => {
it("merges class names and resolves conflicting Tailwind utilities", () => {
expect(classNames("px-2 text-sm", false, "px-4")).toBe("text-sm px-4");
});

it("round-trips uuid formatting helpers", () => {
const formatted = "123e4567-e89b-12d3-a456-426614174000";
const compact = "123e4567e89b12d3a456426614174000";

expect(uuidParse(formatted)).toBe(compact);
expect(uuidFormat(compact)).toBe(formatted);
});

it("calculates progress circle geometry", () => {
const { circumference } = getProgressCircleConfig();

expect(calculateStrokeDashoffset(25, circumference)).toBeCloseTo(
circumference * 0.75,
);
expect(calculateStrokeDashoffset(100, circumference)).toBe(0);
});

it("prefers upload progress over processing progress", () => {
expect(getDisplayProgress(42, 10)).toBe(42);
expect(getDisplayProgress(undefined, 64)).toBe(64);
});
Comment on lines +35 to +38
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 12, 2026

Choose a reason for hiding this comment

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

P1 The getDisplayProgress test skips the uploadProgress = 0 case. Because the implementation checks uploadProgress !== undefined, passing 0 returns 0 and never falls back to processingProgress. This is a non-obvious semantic boundary worth pinning so a future change to === undefined vs. a falsy check doesn't silently regress.

Suggested change
it("prefers upload progress over processing progress", () => {
expect(getDisplayProgress(42, 10)).toBe(42);
expect(getDisplayProgress(undefined, 64)).toBe(64);
});
it("prefers upload progress over processing progress", () => {
expect(getDisplayProgress(42, 10)).toBe(42);
expect(getDisplayProgress(undefined, 64)).toBe(64);
// uploadProgress=0 is defined, so processing progress is NOT used
expect(getDisplayProgress(0, 64)).toBe(0);
});
Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/utils/src/helpers.test.ts
Line: 35-38

Comment:
The `getDisplayProgress` test skips the `uploadProgress = 0` case. Because the implementation checks `uploadProgress !== undefined`, passing `0` returns `0` and never falls back to `processingProgress`. This is a non-obvious semantic boundary worth pinning so a future change to `=== undefined` vs. a falsy check doesn't silently regress.

```suggestion
	it("prefers upload progress over processing progress", () => {
		expect(getDisplayProgress(42, 10)).toBe(42);
		expect(getDisplayProgress(undefined, 64)).toBe(64);
		// uploadProgress=0 is defined, so processing progress is NOT used
		expect(getDisplayProgress(0, 64)).toBe(0);
	});
```

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


it("allows emails by exact address or domain restrictions", () => {
expect(isEmailAllowedByRestriction("owner@cap.so", "")).toBe(true);
expect(isEmailAllowedByRestriction("owner@cap.so", "cap.so")).toBe(true);
expect(
isEmailAllowedByRestriction("owner@cap.so", "admin@example.com"),
).toBe(false);
expect(
isEmailAllowedByRestriction("admin@example.com", " admin@example.com "),
).toBe(true);
});
});
3 changes: 3 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.