test: update api client tests #35
Conversation
WalkthroughRemoves the nonce parameter from ApiClient.get/post signatures, updates tests to call get/post without nonce, adds a test spy on ApiClient.appendSignature, and tightens store tests to assert exact endpoint strings and parameter forwarding (posts now forwards filters via post). Cache/HTTP behavior checks remain. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test
participant Store as useApiStore
participant Client as ApiClient
participant Sign as appendSignature()
participant HTTP as HTTP Adapter
participant Cache as ETag/Cache
Test->>Store: call store method (e.g., getPosts(filters))
Store->>Client: post("posts?limit=5", filters)
note over Client: nonce removed — post(path, body)
Client->>Sign: appendSignature(request)
Sign-->>Client: (spy returns undefined in tests)
Client->>Cache: check ETag / cached response
alt Cache hit (304)
Cache-->>Client: return cached data
else Miss / bypass
Client->>HTTP: send request
HTTP-->>Client: response / error / 304
Client->>Cache: update cache/ETag (on success/304)
end
Client-->>Store: response or error
Store-->>Test: final result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request updates the API client tests to align with recent changes in the get and post method signatures, removing the need for a nonce parameter. Additionally, it introduces mocking for the appendSignature method to prevent unintended network requests during test execution, improving test reliability and performance.
Highlights
- API Client Test Updates: Tests for the API client have been updated to reflect changes in the client.get and client.post method signatures, which no longer require a nonce argument.
- Network Call Prevention: The appendSignature method of the API client is now stubbed in tests to prevent actual network calls, ensuring tests are isolated and faster.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully updates the API client tests to align with the new method signatures for get and post, and correctly stubs the appendSignature method to prevent network calls during testing. The changes are logical and well-implemented. I've only left one comment regarding code formatting consistency, which seems to be an editor configuration issue (spaces vs. tabs). Once that's addressed, this should be good to go.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/stores/api/client.test.ts (5)
48-59: Strengthen assertions: verify signature step invoked and HTTP method.You already spy on appendSignature; assert it was called with expected args and that the request used POST.
it('handles post success and error responses', async () => { const data = { ok: true }; (fetch as vi.Mock).mockResolvedValue(new Response(JSON.stringify(data), { status: 200 })); const result = await client.post('test', { id: 1 }); expect(result).toEqual(data); + const appendSpy = (client as any).appendSignature as unknown as import('vitest').SpyInstance; + expect(appendSpy).toHaveBeenCalledOnce(); + const [nonce, headers, origin] = appendSpy.mock.calls[0]; + expect(nonce).toMatch(/^[a-f0-9]{32}$/); + expect(origin).toBe(new URL('test', url).href); + expect((fetch as vi.Mock).mock.calls[0][1]?.method).toBe('POST'); (fetch as vi.Mock).mockResolvedValue(new Response('fail', { status: 500, statusText: 'err' })); await expect(client.post('test', { id: 2 })).rejects.toBeInstanceOf(HttpError); });
67-69: Also assert the conditional request header is sent.Confirms cache validation wiring (If-None-Match) before the 304.
const result = await client.get('test'); expect(result).toEqual({ cached: true }); +const reqInit = (fetch as vi.Mock).mock.calls[0][1] as RequestInit; +const sentHeaders = reqInit.headers as Headers; +expect(sentHeaders.get('If-None-Match')).toBe('x');
80-83: Verify exact cache payload (etag + data).Stronger assertion that we store both ETag and body.
const result = await client.get('test'); expect(result).toEqual(data); -expect(localStorage.getItem('api-cache-test')).not.toBeNull(); +const cached = JSON.parse(localStorage.getItem('api-cache-test')!); +expect(cached).toEqual({ etag: 'abc', data });
88-89: Check HttpError details (status) for better signal.Keeps the test resilient and informative.
-await expect(client.get('oops')).rejects.toBeInstanceOf(HttpError); +await expect(client.get('oops')).rejects.toMatchObject({ name: 'HttpError', status: 404 });
17-21: Fix “vitest not found” by adding dev deps and test script.Project-level, but blocking. Add Vitest and jsdom (for localStorage) and wire scripts.
--- a/package.json +++ b/package.json @@ { "scripts": { - "test": "npm run not-configured" + "test": "vitest --environment=jsdom", + "test:ci": "vitest run --environment=jsdom" }, "devDependencies": { + "vitest": "^2.1.1", + "jsdom": "^24.1.0", + "typescript": "^5.5.0" } }If you prefer happy-dom instead of jsdom, swap jsdom with happy-dom and use --environment=happy-dom.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/stores/api/client.test.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/stores/api/client.test.ts (2)
src/stores/api/client.ts (1)
ApiClient(29-200)src/stores/api/http-error.ts (1)
HttpError(1-15)
🪛 GitHub Actions: Test Format Workflow
tests/stores/api/client.test.ts
[error] 14-24: Command 'git diff --exit-code' failed with exit code 1. Unstaged changes detected in tests/stores/api/client.test.ts (beforeEach block formatting changes).
[error] 45-61: Command 'git diff --exit-code' failed with exit code 1. Unstaged changes detected in tests/stores/api/client.test.ts (test block formatting changes).
[error] 64-72: Command 'git diff --exit-code' failed with exit code 1. Unstaged changes detected in tests/stores/api/client.test.ts (test case formatting changes).
[error] 77-90: Command 'git diff --exit-code' failed with exit code 1. Unstaged changes detected in tests/stores/api/client.test.ts (additional expectations formatting changes).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/stores/api/store.test.ts (3)
37-43: Reduce brittleness in console assertion; consider non-dev branch test.Assert on a substring to avoid coupling to exact message; add a test for isDev() = false where no log occurs.
- expect(spy).toHaveBeenCalledWith('API client booted ...'); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('API client booted'));Optional extra test:
it('does not log in prod mode', () => { const spy = vi.spyOn(console, 'log').mockImplementation(() => {}); client.isDev.mockReturnValue(false); store.boot(); expect(spy).not.toHaveBeenCalled(); spy.mockRestore(); });
69-75: Strengthen filters coverage in posts test.Use non-empty filters and add a test for undefined/null to ensure defaulting/forwarding works.
Example additional test:
it('forwards non-empty filters when fetching posts', async () => { const filters = { tags: ['vue'], page: 2 }; client.post.mockResolvedValue({ list: [] }); await store.getPosts(filters); expect(client.post).toHaveBeenCalledWith('posts?limit=5', filters); }); it('handles undefined filters by sending empty object', async () => { client.post.mockResolvedValue({ list: [] }); await store.getPosts(undefined as any); expect(client.post).toHaveBeenCalledWith('posts?limit=5', {}); });
82-87: Add a slug-encoding case.Ensure slugs with spaces/special chars are URL-encoded before calling client.get.
Example test:
it('encodes slug when getting a single post', async () => { client.get.mockResolvedValue({ slug: 'a b' }); await store.getPost('a b'); expect(client.get).toHaveBeenCalledWith('posts/a%20b'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/stores/api/client.test.ts(4 hunks)tests/stores/api/store.test.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/stores/api/client.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
tests/stores/api/store.test.ts (1)
tests/partials/ArticlesListPartial.test.ts (1)
useApiStore(75-81)
🔇 Additional comments (9)
tests/stores/api/store.test.ts (9)
45-50: LGTM: profile endpoint matches new get(path) signature.
57-63: LGTM: categories endpoint with explicit limit.
94-99: LGTM: experience endpoint path is correct.
106-111: LGTM: recommendations endpoint path is correct.
118-123: LGTM: projects endpoint path is correct.
130-135: LGTM: talks endpoint path is correct.
142-147: LGTM: social endpoint path is correct.
154-159: LGTM: education endpoint path is correct.
1-4: Remove outdated Vitest suggestion Vitest is already listed in devDependencies and the “test” script invokes it.Likely an incorrect or invalid review comment.
Summary
Testing
npm test(fails: vitest not found)https://chatgpt.com/codex/tasks/task_e_68be392abf7c8333ac60ae9be3e03a7a
Summary by CodeRabbit
New Features
Refactor
Tests