Skip to content

test: add local docker compose coverage#1846

Open
abdul712 wants to merge 1 commit into
CapSoftware:mainfrom
abdul712:bounty/cap-54-local-docker-tests
Open

test: add local docker compose coverage#1846
abdul712 wants to merge 1 commit into
CapSoftware:mainfrom
abdul712:bounty/cap-54-local-docker-tests

Conversation

@abdul712
Copy link
Copy Markdown

@abdul712 abdul712 commented May 20, 2026

Summary

  • add a package-level Vitest setup for @cap/local-docker so it participates in the root Turbo test pipeline
  • add an example compose-file test that parses docker-compose.yml and verifies the expected local services plus named-volume consistency
  • remove stale minio-data and minio-certs volume declarations that were not used by any service, and fix the local S3 comment typo

This is a narrow, non-overlapping slice for #54; I did not touch the desktop/utils/sdk/env/database testing areas covered by active PRs.

/claim #54

AI-assisted with Codex; I reviewed the diff and kept payment details out of public comments.

Verification

  • corepack pnpm install --filter @cap/local-docker --frozen-lockfile --ignore-scripts
  • corepack pnpm --filter @cap/local-docker test
  • corepack pnpm turbo run test --filter=@cap/local-docker
  • corepack pnpm exec biome check packages/local-docker/package.json packages/local-docker/vitest.config.ts packages/local-docker/docker-compose.test.ts
  • git diff --check

Greptile Summary

This PR wires @cap/local-docker into the Turbo test pipeline by adding a Vitest setup and a compose-file smoke test, and cleans up two unused named-volume declarations (minio-data, minio-certs) from docker-compose.yml.

  • New test (docker-compose.test.ts): parses docker-compose.yml with the yaml package and asserts expected service names, key image/build properties, and that every declared named volume is actually referenced by a service — the stale-volume removal is what makes the second assertion pass.
  • Compose cleanup: removes minio-data and minio-certs volume declarations (never used; minio already uses a bind-mount at ~/minio/data) and fixes the "Strorage" typo in the comment.
  • Package wiring: adds vitest ~2.1.9 and yaml ^2.8.1 as dev dependencies, consistent with versions used by apps/discord-bot and apps/desktop, and locks them in pnpm-lock.yaml.

Confidence Score: 4/5

Safe to merge; the compose cleanup and test addition are narrow and well-scoped with no risk to running services.

The changes are straightforward: two stale volume declarations are removed, a typo is fixed, and a new test correctly validates the resulting compose file. The test logic for named-volume consistency is sound. The only observations are style-level: readCompose() is called once per test rather than shared via beforeAll, and the include glob in vitest.config.ts is broader than needed for a flat package.

No files require special attention; docker-compose.test.ts and vitest.config.ts have minor style-level suggestions worth a quick look.

Important Files Changed

Filename Overview
packages/local-docker/docker-compose.test.ts New test file parsing docker-compose.yml; correct logic overall, but readCompose() is invoked twice (once per test case) and the include glob in vitest.config.ts is broader than needed
packages/local-docker/docker-compose.yml Removes stale minio-data and minio-certs volume declarations that were never referenced by any service, and fixes the "Strorage" typo in the comment
packages/local-docker/package.json Adds test script and devDependencies for vitest (~2.1.9) and yaml (^2.8.1), consistent with other packages in the monorepo
packages/local-docker/vitest.config.ts Minimal Vitest config; include pattern ["**/*.test.ts"] is broader than necessary for a flat package with a single test file
pnpm-lock.yaml Lock file updated to reflect vitest 2.1.9 and yaml 2.8.1 additions for the local-docker package
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/local-docker/vitest.config.ts:6
The `include` pattern `["**/*.test.ts"]` will recurse through all subdirectories. Since this package's tests live at the root level, scoping it to `["*.test.ts"]` is both cleaner and avoids any accidental discovery of test files that end up in sub-directories (e.g., a future `scripts/` folder).

```suggestion
		include: ["*.test.ts"],
```

### Issue 2 of 2
packages/local-docker/docker-compose.test.ts:34-36
`readCompose()` is called independently inside each `it` block, which reads and parses the YAML file twice per test run. Hoisting the parse into a `beforeAll` and sharing the result removes the duplicate I/O and makes the shared setup intent explicit.

```suggestion
describe("local Docker compose file", () => {
	let compose: ComposeFile;

	beforeAll(async () => {
		compose = await readCompose();
	});

	it("keeps the expected local development services wired", () => {
```

Reviews (1): Last reviewed commit: "test: add local docker compose coverage" | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

export default defineConfig({
test: {
environment: "node",
include: ["**/*.test.ts"],
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 include pattern ["**/*.test.ts"] will recurse through all subdirectories. Since this package's tests live at the root level, scoping it to ["*.test.ts"] is both cleaner and avoids any accidental discovery of test files that end up in sub-directories (e.g., a future scripts/ folder).

Suggested change
include: ["**/*.test.ts"],
include: ["*.test.ts"],
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/local-docker/vitest.config.ts
Line: 6

Comment:
The `include` pattern `["**/*.test.ts"]` will recurse through all subdirectories. Since this package's tests live at the root level, scoping it to `["*.test.ts"]` is both cleaner and avoids any accidental discovery of test files that end up in sub-directories (e.g., a future `scripts/` folder).

```suggestion
		include: ["*.test.ts"],
```

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

Comment on lines +34 to +36
describe("local Docker compose file", () => {
it("keeps the expected local development services wired", async () => {
const compose = await readCompose();
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 readCompose() is called independently inside each it block, which reads and parses the YAML file twice per test run. Hoisting the parse into a beforeAll and sharing the result removes the duplicate I/O and makes the shared setup intent explicit.

Suggested change
describe("local Docker compose file", () => {
it("keeps the expected local development services wired", async () => {
const compose = await readCompose();
describe("local Docker compose file", () => {
let compose: ComposeFile;
beforeAll(async () => {
compose = await readCompose();
});
it("keeps the expected local development services wired", () => {
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/local-docker/docker-compose.test.ts
Line: 34-36

Comment:
`readCompose()` is called independently inside each `it` block, which reads and parses the YAML file twice per test run. Hoisting the parse into a `beforeAll` and sharing the result removes the duplicate I/O and makes the shared setup intent explicit.

```suggestion
describe("local Docker compose file", () => {
	let compose: ComposeFile;

	beforeAll(async () => {
		compose = await readCompose();
	});

	it("keeps the expected local development services wired", () => {
```

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant