Skip to content

Conversation

@gocanto
Copy link
Contributor

@gocanto gocanto commented Aug 11, 2025

Summary

  • ensure ApiClient tests stub the VITE_API_URL environment variable
  • clean up after each test to avoid leaked env changes

Testing

  • npx vitest run tests/stores/api/client.test.ts

https://chatgpt.com/codex/tasks/task_e_6899cc90f2fc8333891024def255a9ea

Summary by CodeRabbit

  • Tests

    • Improved API client test reliability by stubbing environment variables, ensuring proper cleanup, adjusting initialization order, and passing an explicit request nonce/token for isolation.
  • Chores

    • Pinned Node toolchain to 22.18.0 across configs and added strict engine enforcement (engine-strict + preinstall check) to require Node 22.12+.

@coderabbitai
Copy link

coderabbitai bot commented Aug 11, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Caution

Review failed

The pull request is closed.

Walkthrough

Updates tests to stub VITE_API_URL with vi.stubEnv, instantiate ApiClient after stubbing, pass a per-request nonce to ApiClient.get/post in tests, and unstub envs in afterEach. Separately, Node version pins and enforcement were added (.nvmrc, .tool-versions, mise.toml, package.json), and engine-strict was enabled in .npmrc.

Changes

Cohort / File(s) Summary of Changes
Tests — ApiClient env & nonce
tests/stores/api/client.test.ts
Use vi.stubEnv('VITE_API_URL', ...), move ApiClient instantiation after stubbing, add let nonce = 'nonce' and pass nonce to ApiClient.get/post, and call vi.unstubAllEnvs() in afterEach.
Api surface change (call sites / signature)
src/.../api/client.ts (public API)
Public method signatures changed: post(path, payload)post(path, nonce, payload) and get(path)get(path, nonce) (tests updated accordingly).
Node toolchain version pins
.nvmrc, .tool-versions, mise.toml
Added Node version 22.18.0 entries to pin tooling/runtime.
Package configuration
package.json
Tightened engines.node to 22.18.0 and added a preinstall script that enforces Node 22.12+ at install time.
npm config
.npmrc
Added engine-strict=true.

Sequence Diagram(s)

sequenceDiagram
    participant T as Test
    participant E as Environment (VITE_API_URL)
    participant C as ApiClient
    participant F as fetch (network)

    T->>E: vi.stubEnv('VITE_API_URL', url)
    T->>C: new ApiClient()  -- reads VITE_API_URL
    T->>C: C.get(path, nonce) / C.post(path, nonce, payload)
    C->>F: perform fetch(...)  -- includes nonce per-request handling
    T->>T: afterEach -> vi.unstubAllEnvs()
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

testing

Poem

I nibbled at envs beneath moonlight,
Stubbed a URL and shaped it right—
Gave each request a tiny nonce,
Cleared the burrow so no stubs ensconce,
Hopped off happy to greener bytes. 🥕🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed16134 and 136c17b.

📒 Files selected for processing (6)
  • .npmrc (1 hunks)
  • .nvmrc (1 hunks)
  • .tool-versions (1 hunks)
  • mise.toml (1 hunks)
  • package.json (1 hunks)
  • tests/stores/api/client.test.ts (5 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/fix-client.test.ts-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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

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!

I've addressed an issue in the client tests to ensure they are more robust and isolated. The primary goal was to correctly handle the VITE_API_URL environment variable within the test suite. This change ensures that each test runs with a predictable environment, preventing potential side effects from previous tests and making the test suite more reliable.

Highlights

  • Test Environment Setup: The VITE_API_URL environment variable is now properly stubbed using vi.stubEnv within the beforeEach hook, ensuring each test runs with a clean and controlled environment. This replaces the previous global assignment.
  • Test Environment Teardown: Added vi.unstubAllEnvs() to the afterEach hook to clean up any stubbed environment variables after each test. This prevents environment variable leakage between tests, ensuring test isolation and reliability.
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 or fill out our survey to provide feedback.

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

  1. 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.

@gocanto gocanto marked this pull request as draft August 11, 2025 11:04
Copy link

@gemini-code-assist gemini-code-assist 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

This pull request correctly refactors the client tests to use vi.stubEnv for setting environment variables, which is a great improvement for ensuring test isolation and preventing state leakage. The cleanup in afterEach with vi.unstubAllEnvs is also a good practice. I've added a couple of minor suggestions to align the code formatting with the existing style in the file.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 392e7e3 and ed16134.

📒 Files selected for processing (1)
  • tests/stores/api/client.test.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/stores/api/client.test.ts (2)
src/stores/api/client.ts (1)
  • ApiClient (29-134)
tests/setup.ts (1)
  • LocalStorageMock (5-19)
🪛 GitHub Actions: Test Format Workflow
tests/stores/api/client.test.ts

[error] 13-27: git diff --exit-code failed with exit code 1. Uncommitted changes detected in tests/stores/api/client.test.ts (indentation changes in beforeEach/afterEach blocks).

🔇 Additional comments (1)
tests/stores/api/client.test.ts (1)

17-19: Good fix: stub env before constructing ApiClient

Stubbing VITE_API_URL via vi.stubEnv and instantiating the client afterwards ensures the base URL is read correctly. localStorage.clear in beforeEach is appropriate isolation.

Comment on lines 20 to 23
vi.stubGlobal('fetch', vi.fn());
});
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Unstub stubbed globals in teardown to avoid leaks

vi.restoreAllMocks won’t revert vi.stubGlobal. Add vi.unstubAllGlobals() in afterEach so fetch doesn’t remain mocked across other tests.


🏁 Script executed:

#!/bin/bash
# Locate the client test file
TEST_FILE=$(fd client.test.ts | head -n 1)
echo "Client test file: $TEST_FILE"

# Show stubGlobal usage
echo "=== stubGlobal context ==="
rg -n -C3 "vi.stubGlobal" "$TEST_FILE" || true

# Show afterEach or teardown
echo "=== afterEach context ==="
rg -n -C3 "afterEach" "$TEST_FILE" || true

Length of output: 913


Add vi.unstubAllGlobals() in teardown to clean up stubbed globals

The existing afterEach calls restore mocks and environments but doesn’t revert globals stubbed with vi.stubGlobal. Without unstubbing, fetch remains mocked across tests.

• File: tests/stores/api/client.test.ts
• Location: inside the existing afterEach block

Suggested diff:

 afterEach(() => {
   vi.restoreAllMocks();
   vi.unstubAllEnvs();
+  vi.unstubAllGlobals();
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
vi.stubGlobal('fetch', vi.fn());
});
afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllEnvs();
vi.unstubAllGlobals();
});
🤖 Prompt for AI Agents
In tests/stores/api/client.test.ts around lines 20 to 21, the afterEach block
restores mocks/environments but does not revert globals stubbed with
vi.stubGlobal (e.g., fetch), so add a call to vi.unstubAllGlobals() inside the
existing afterEach to ensure any globally stubbed values are removed between
tests; keep it alongside the existing restore calls so cleanup runs after each
test.

Comment on lines 24 to 28
vi.restoreAllMocks();
vi.unstubAllEnvs();
});
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Add vi.unstubAllGlobals and fix formatting to unblock CI

  • Add vi.unstubAllGlobals() to properly revert the fetch stub after each test.
  • The “Test Format Workflow” failed due to indentation changes. Run your repo’s formatter and commit the result.

Proposed change:

 afterEach(() => {
-        vi.restoreAllMocks();
-        vi.unstubAllEnvs();
+        vi.restoreAllMocks();
+        vi.unstubAllGlobals();
+        vi.unstubAllEnvs();
 });

To fix and verify formatting locally:


🏁 Script executed:

#!/bin/bash
# Format the changed test file and verify no further diffs remain.
npx prettier --write tests/stores/api/client.test.ts

# If your repo uses a specific script, prefer it, e.g.:
# pnpm format
# or
# npm run format

# Show if Prettier made changes that need committing
git diff -- tests/stores/api/client.test.ts
# Optional: ensure CI’s format check would pass for this file
npx prettier --check tests/stores/api/client.test.ts

Length of output: 1089


Add missing vi.unstubAllGlobals() in afterEach and commit formatting

Tests/stores/api/client.test.ts needs to clean up the fetch stub and keep CI happy:

• In the afterEach block, insert vi.unstubAllGlobals() between vi.restoreAllMocks() and vi.unstubAllEnvs().
• Run your formatter (e.g. pnpm format, npm run format or npx prettier --write tests/stores/api/client.test.ts) and commit the result so indentation changes are captured and the CI format check passes.

Proposed diff:

 tests/stores/api/client.test.ts
 afterEach(() => {
-   vi.restoreAllMocks();
-   vi.unstubAllEnvs();
+   vi.restoreAllMocks();
+   vi.unstubAllGlobals();
+   vi.unstubAllEnvs();
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
vi.restoreAllMocks();
vi.unstubAllEnvs();
});
tests/stores/api/client.test.ts
afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllGlobals();
vi.unstubAllEnvs();
});
🤖 Prompt for AI Agents
In tests/stores/api/client.test.ts around lines 24 to 26, the afterEach cleanup
is missing a call to vi.unstubAllGlobals(), so insert a call to
vi.unstubAllGlobals() between vi.restoreAllMocks() and vi.unstubAllEnvs(), then
run the project formatter (e.g. pnpm format / npm run format / npx prettier
--write tests/stores/api/client.test.ts) and commit the formatted file so CI
format checks pass.

@gocanto gocanto force-pushed the codex/fix-client.test.ts-tests branch from ff429c8 to 706d39b Compare August 12, 2025 03:45
@gocanto gocanto changed the title fix client tests chore: Tests + Version Aug 12, 2025
@gocanto gocanto marked this pull request as ready for review August 12, 2025 04:15
@gocanto gocanto merged commit c191de8 into main Aug 12, 2025
2 of 5 checks passed
@gocanto gocanto deleted the codex/fix-client.test.ts-tests branch August 12, 2025 04:16
This was referenced Sep 5, 2025
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.

2 participants