Skip to content

test(shared,frontend): Add unit test coverage for schemas, API client, and storeAction#125

Merged
enko merged 5 commits intomainfrom
test/add-unit-test-coverage
Mar 8, 2026
Merged

test(shared,frontend): Add unit test coverage for schemas, API client, and storeAction#125
enko merged 5 commits intomainfrom
test/add-unit-test-coverage

Conversation

@enko
Copy link
Copy Markdown
Member

@enko enko commented Mar 8, 2026

Summary

  • Set up vitest in packages/shared/ with node environment for pure-logic schema tests
  • Add 43 tests for shared package: PasswordSchema, CircleInputSchema, EncounterInputSchema, PhoneInputSchema, EmailInputSchema, UrlInputSchema, DateInputSchema, SocialProfileInputSchema, ProfessionalHistoryInputSchema, and normalizePhoneNumber
  • Add 21 tests for frontend: apiRequest (auth headers, content types, error handling, credentials) and storeAction (loading state, success/error flows, re-throw behavior)

Test plan

  • cd packages/shared && pnpm vitest run — 43 tests pass
  • cd apps/frontend && pnpm vitest run — 21 tests pass
  • pnpm test — full monorepo suite passes (including backend + PHP)

Closes #117

🤖 Generated with Claude Code

…lient, and storeAction

Add 64 unit tests across shared package and frontend, covering validation
schemas (auth, circles, encounters, friends), phone number normalization,
the API client (auth headers, error handling, content types), and the
generic store action wrapper. Sets up vitest in the shared package.

Closes #117

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

await expect(apiRequest('/api/test')).rejects.toThrow(ApiError);

try {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Testing: Assertions in bare catch can pass silently

If the second apiRequest() call doesn't throw (e.g. due to a future mock refactor), the catch block is skipped and all three expect() calls inside it are silently omitted — the test still passes. The same pattern appears at the equivalent block below (line ~181).

Suggested fix — use .rejects.toMatchObject() to assert both the throw and its properties in one guaranteed step:

await expect(apiRequest('/api/test')).rejects.toMatchObject({
  statusCode: 422,
  message: 'Validation failed',
  code: 'VALIDATION_ERROR',
});

Apply the same refactor to the second error test (lines ~172-188).

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

Commit 01f4f30 | Reviewed 2026-03-08 | Status: Approved. Critical: 0, Important: 0, Suggestion: 1. Solid test additions closing 117. 64 new tests across shared and frontend. The previously flagged bare-catch issue was fixed. One suggestion: DateInputSchema in friends.test.ts only covers valid inputs - no negative test cases for invalid date formats or date_type values, unlike every other schema in the same file.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Found 1 issue to consider. See inline comments for details.

…ceOf(type.errors)

Produces more informative test failure messages by showing the actual
arktype validation error summary instead of a generic instanceof check.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

Update to summary: commit 18b2cb6 reviewed. No new issues found in d5a7164 refactor commit. Pre-existing catch-block issue remains.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

No new issues found in the refactor commit (d5a7164). The toHaveProperty(summary) change is a clean improvement. The pre-existing catch-block issue from the previous review remains open.

enko and others added 2 commits March 8, 2026 12:31
…sertions

Captures exact arktype error messages so any change in validation
wording surfaces as a clear diff rather than a silent pass. Also
excludes test files from tsc --build to avoid union type errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hObject

The bare catch blocks could pass silently if the call didn't throw.
Using rejects.toMatchObject guarantees both the throw and its
properties are asserted in one step.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
date_type: 'anniversary',
});
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Testing: DateInputSchema only covers valid inputs

Unlike the other schemas in this file, DateInputSchema has no negative test cases. Consider adding tests for invalid inputs to maintain consistent coverage:

it('rejects an invalid date format', () => {
  const result = DateInputSchema({
    date_value: '15-05-1990', // DD-MM-YYYY instead of YYYY-MM-DD
    date_type: 'birthday',
  });
  expect(result.summary).toContain('invalid');
});

it('rejects an invalid date_type', () => {
  const result = DateInputSchema({
    date_value: '1990-05-15',
    date_type: 'graduation', // not a valid type
  });
  expect(result.summary).toBeDefined();
});

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review complete at commit 01f4f30. No critical or important issues found. One suggestion posted inline regarding DateInputSchema missing negative test cases.

Address PR review suggestion by adding a test for invalid date_type
values, consistent with negative tests on other schemas in the file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review complete at commit af79c73. No new issues found. Previously flagged catch-block concern in client.test.ts is resolved in the current state (all error tests use rejects.toMatchObject/rejects.toThrow). Pre-existing DateInputSchema suggestion remains open (inline comment on friends.test.ts:128).

@enko enko merged commit ba82770 into main Mar 8, 2026
5 checks passed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 8, 2026

🎉 This PR is included in version 2.65.8 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(frontend): Add unit test coverage for stores, API client, and form validation

1 participant