Integration tests #108
Integration tests #108
Conversation
- Added `dist-test/` to `.gitignore` to exclude test build outputs. - Updated `biome.jsonc` to ignore `dist-test/` in the configuration. - Added `jsdom` as a new dependency in `package.json` and updated `pnpm-lock.yaml` accordingly. - Updated `vitest.config.ts` to specify project paths for testing. - Added `vitest` as a dev dependency in multiple example projects. - Updated `vite` version in the `vite-plugin` package to ensure compatibility.
|
yamcodes
added
the
@arkenv/vite-plugin
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@yamcodes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
WalkthroughAdds Vitest-based tests and config for the Vite plugin, new TESTING.md, example and example-build config changes, workspace and package metadata updates, updated ignore rules, and enables VCS in tooling metadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester as Vitest
participant Vite as Vite build
participant Plugin as @arkenv/vite-plugin
participant Ark as arkenv.defineEnv (mock)
Tester->>Vite: start build(root=example, plugins=[react(), Plugin], mode)
Vite->>Plugin: invoke plugin hooks (config/build)
Plugin->>Ark: call defineEnv(schema, resolvedEnv)
Ark-->>Plugin: mocked response / record call
Plugin-->>Vite: plugin completes
Vite-->>Tester: return build result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
… and `pnpm-lock.yaml`. Clean up example project configurations by removing unused `vitest` dependency.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
package.json (1)
26-26: Remove unused root jsdom dep and enforce Node >=18
- No references to jsdom in root configs or imports; remove it from the root package.json and add it only in packages/examples that need a browser-like test env.
- jsdom v25 requires Node.js v18 or newer; add
"engines": { "node": ">=18" }to package.json or ensure CI runs on Node ≥ 18.examples/with-bun/package.json (1)
16-18: Add atestscript to make Vitest usable from this example.Right now
vitestis added but not runnable viapnpm testin this package.Suggested scripts block addition:
{ "scripts": { "start": "bun run index.ts", "dev": "bun --watch run index.ts", "test": "vitest" } }examples/basic/package.json (1)
16-17: Consider dropping Vitest here (or add a local test script).If
examples/basichas no tests, keeping Vitest inflates installs. Either remove it or add a simpletestscript to justify the dep.Apply to remove the unused dep:
"devDependencies": { "tsx": "^4.19.2", - "typescript": "^5.0.0", - "vitest": "^3.2.4" + "typescript": "^5.0.0" },If you prefer to keep it, add a script (outside the selected range):
"scripts": { "test": "vitest" }examples/with-vite-react-ts/package.json (1)
23-26: Either wire up jsdom via a local Vitest config or remove jsdom/Vitest here.This example is used as a build fixture; unless it has tests, these devDeps are likely unnecessary. If you plan browser-like tests, add
vitest.config.tswithenvironment: "jsdom". Otherwise, drop both to speed up installs.Remove if unused:
"devDependencies": { "@types/react": "^19.0.8", "@types/react-dom": "^19.0.3", "@vitejs/plugin-react": "^4.3.4", "globals": "^15.14.0", - "jsdom": "^25.0.1", "typescript": "~5.7.2", - "vite": "^7.0.0", - "vitest": "^3.2.4" + "vite": "^7.0.0" }If you keep them, add (outside this file)
examples/with-vite-react-ts/vitest.config.ts:import { defineConfig } from "vitest/config"; export default defineConfig({ test: { environment: "jsdom" } });vitest.config.ts (1)
5-6: Good move to test.projects; also exclude dist-test from discovery.Given builds write to dist-test, exclude it so Vitest doesn’t scan artifacts.
export default defineConfig({ test: { - projects: ["packages/arkenv", "packages/vite-plugin"], + projects: ["packages/arkenv", "packages/vite-plugin"], + exclude: ["**/dist-test/**"], }, });This aligns with Vitest 3.2+ “projects” semantics. See Vitest docs on projects/workspace. (vitest.dev)
TESTING.md (1)
8-9: Tighten wording and casing; keep CLI consistent with project naming.Minor grammar/case tweaks (“Vite”) and agreement fixes. With the config change to name the project “vite-plugin”, the CLI examples stay valid.
-1. **Documentation** - Show real-world usage patterns -2. **Test Fixtures** - Provide real projects to test against +1. **Documentation** - Shows real-world usage patterns +2. **Test Fixtures** - Provides real projects to test against-### Integration Tests (`packages/vite-plugin/src/*.test.ts`) -- Test the vite plugin using the `with-vite-react-ts` example as a fixture +### Integration Tests (`packages/vite-plugin/src/*.test.ts`) +- Test the Vite plugin using the `with-vite-react-ts` example as a fixture-The CI pipeline runs: -- Unit tests for core functionality -- Integration tests for the vite plugin using real examples -- Ensures no regressions in real-world usage scenarios +The CI pipeline runs: +- Unit tests for core functionality +- Integration tests for the Vite plugin using real examples +- Ensures no regressions in real-world usage scenariosAlso applies to: 21-23, 69-71
packages/vite-plugin/src/index.test.ts (1)
12-14: Reset module cache between tests to avoid stale importsEnsures each dynamic import re-evaluates with a clean state and mock bindings.
beforeEach(() => { mockDefineEnv.mockClear(); + vi.resetModules(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.gitignore(1 hunks)TESTING.md(1 hunks)biome.jsonc(1 hunks)examples/basic/package.json(1 hunks)examples/with-bun/package.json(1 hunks)examples/with-vite-react-ts/package.json(1 hunks)package.json(1 hunks)packages/vite-plugin/package.json(1 hunks)packages/vite-plugin/src/index.test.ts(1 hunks)packages/vite-plugin/vitest.config.ts(1 hunks)vitest.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
vitest.config.ts (2)
packages/arkenv/src/define-env.test.ts (1)
it(27-86)packages/vite-plugin/src/index.ts (2)
T(15-20)config(17-19)
packages/vite-plugin/vitest.config.ts (1)
packages/vite-plugin/src/index.ts (2)
config(17-19)T(15-20)
packages/vite-plugin/src/index.test.ts (2)
packages/arkenv/src/define-env.test.ts (4)
it(27-86)env(72-85)process(28-36)expect(38-52)packages/vite-plugin/src/index.ts (2)
T(15-20)config(17-19)
examples/with-bun/package.json (1)
packages/arkenv/src/define-env.test.ts (1)
it(27-86)
TESTING.md (3)
packages/arkenv/src/errors.test.ts (6)
errors(35-50)errors(63-74)it(34-75)it(77-107)errors(78-92)errors(94-106)packages/arkenv/src/define-env.test.ts (3)
it(27-86)env(72-85)process(28-36)packages/arkenv/src/utils.test.ts (1)
it(4-40)
packages/vite-plugin/package.json (2)
packages/vite-plugin/src/index.ts (1)
T(15-20)examples/with-vite-react-ts/src/App.tsx (1)
App(6-34)
🪛 GitHub Actions: tests
package.json
[error] 1-1: Lockfile specifiers don't match: 1 dependencies were added (vitest@^3.2.4); 1 dependencies are mismatched (vite: lockfile ^6.0.0 vs manifest ^7.0.0).
packages/vite-plugin/package.json
[error] 1-1: Lockfile specifiers don't match: 1 dependencies were added (vitest@^3.2.4); 1 dependencies are mismatched (vite: lockfile ^6.0.0 vs manifest ^7.0.0).
🪛 LanguageTool
TESTING.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...s"** - Our examples serve dual purposes: 1. Documentation - Show real-world usage ...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...ation** - Show real-world usage patterns 2. Test Fixtures - Provide real projects ...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... Tests (packages/arkenv/src/*.test.ts) - Test individual functions and edge cases...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...s (packages/vite-plugin/src/*.test.ts) - Test the vite plugin using the `with-vit...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...st Coverage ### Core Package (arkenv) - ✅ Environment variable parsing and valid...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...ironment variable parsing and validation - ✅ Type checking and error handling - ✅ D...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...ion - ✅ Type checking and error handling - ✅ Default value handling - ✅ Custom type...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...rror handling - ✅ Default value handling - ✅ Custom type validation (host, port, et...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... ### Vite Plugin (@arkenv/vite-plugin) - ✅ Plugin integration with Vite - ✅ Envir...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...lugin`) - ✅ Plugin integration with Vite - ✅ Environment variable loading and injec...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...vironment variable loading and injection - ✅ Real project build testing using the e...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...d testing using the example as a fixture - ✅ Error handling for missing environment...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...## CI Integration The CI pipeline runs: - Unit tests for core functionality - Inte...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...uns: - Unit tests for core functionality - Integration tests for the vite plugin us...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ... for the vite plugin using real examples - Ensures no regressions in real-world usa...
(QB_NEW_EN)
🔇 Additional comments (2)
biome.jsonc (1)
15-15: LGTM: Excluding dist-test mirrors the new test output dir.Matches the repo’s test build strategy and prevents formatter/linter churn on artifacts.
.gitignore (1)
3-3: Ignore dist-test directories at any depth.Tests write dist-test inside packages; the current rule only ignores root-level dist-test.
Apply:
-dist-test/ +**/dist-test/Likely an incorrect or invalid review comment.
- Changed `.gitignore` to include `.env.local` and exclude `.env.production`. - Added `dotenv` as a dependency in `package.json` and updated its version in `pnpm-lock.yaml`. - Updated test files to load environment variables from `.env.test` for improved testing consistency.
…ining only `vite` version 7.0.0 for the Vite React TypeScript example.
…and clean up test file by eliminating environment variable loading.
- Set the test name to "@arkenv/vite-plugin" in vitest.config.ts. - Updated the build configuration in index.test.ts to include write: false for the test output.
…ild output handling - Modified assertions to check for a RollupOutput array when write: false is set. - Ensured that the test verifies the output structure correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.gitignore (1)
17-18: Deduplicate and simplify .env ignore patterns; confirm intent for .env.production
- .env.local is redundant if .env*.local is present, and .env*.local appears again at Line 30. Keep a single rule in the env section and drop duplicates.
Apply:
# environment variables .env -.env.local -.env*.local +.env*.local @@ # Next.js .next/ .source/ .vercel -.env*.local
- You removed ignoring .env.production. If that’s intentional for committed, non-secret configs, fine. If not, re-add an ignore to avoid accidental secrets leakage.
Optional hardening if you want to keep production files out:
# environment variables .env .env*.local +.env.production +# or broadly: +# .env.* +# !.env.exampleAlso applies to: 30-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.gitignore(2 hunks)packages/vite-plugin/.env.test(1 hunks)packages/vite-plugin/src/index.test.ts(1 hunks)packages/vite-plugin/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vite-plugin/.env.test
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/vite-plugin/vitest.config.ts
- packages/vite-plugin/src/index.test.ts
🔇 Additional comments (1)
.gitignore (1)
3-3: Good call adding dist-test/Aligns with Vitest/Vite test builds and prevents committing artifacts.
…ct structure - Changed assertions to check for a RollupOutput object with an output property when write: false is set. - Enhanced error handling for unexpected output types.
…ment variable - Added a mock implementation to throw an error when VITE_TEST is not a string. - Restored the original mock implementation after the test to maintain test integrity.
…orts - Introduced a path alias for the 'arkenv' module to simplify imports in test files. - Updated the configuration to enhance module resolution in the testing environment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/vite-plugin/vitest.config.ts (1)
6-6: Align project name with TESTING.md CLI (--project vite-plugin).Set
test.nameto"vite-plugin"to match the documented CLI flag and avoid confusion.- name: "@arkenv/vite-plugin", + name: "vite-plugin",
🧹 Nitpick comments (6)
packages/vite-plugin/vitest.config.ts (2)
1-2: Make __dirname usage ESM-safe in config files.
__dirnameisn’t defined in native ESM. Vitest usually handles TS configs, but making this robust avoids edge failures. Compute it viaimport.meta.url.+import { fileURLToPath } from "node:url"; import path from "node:path"; import { defineConfig } from "vitest/config"; + +// ESM-safe __dirname for config files +const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ - arkenv: path.resolve(__dirname, "../arkenv/src/index.ts"), + arkenv: path.resolve(__dirname, "../arkenv/src/index.ts"),Also applies to: 15-18
11-11: Exclude dist-test outputs from coverage scanning.You’re already ignoring
dist/; adddist-test/(and optionally example build dirs) to prevent noise.- exclude: ["node_modules/", "dist/", "**/*.test.ts", "**/*.config.ts"], + exclude: [ + "node_modules/", + "dist/", + "dist-test/", + "examples/**/dist*/", + "**/*.test.ts", + "**/*.config.ts", + ],packages/vite-plugin/src/index.test.ts (4)
31-36: Default build mode to "test" for deterministic env loading.Your assertions rely on
.env.test. Make that explicit so the tests don’t accidentally pass due to process env state.- return build({ - plugins: [react(), plugin(envVars)], - root: EXAMPLE_ROOT, - configFile: false, - ...options, - }); + return build({ + // default to test mode so loadEnv() reads .env.test; caller can override + mode: "test", + plugins: [react(), plugin(envVars)], + root: EXAMPLE_ROOT, + configFile: false, + ...options, + });
156-164: Avoid optional terser dependency in tests (or add it explicitly).
minify: "terser"requires theterserpackage. If it’s not in dev deps, CI will fail. Prefer"esbuild"here or addterserto devDependencies.- build: { + build: { outDir: "dist-complex-test", write: false, - minify: "terser", + minify: "esbuild", sourcemap: true, },If you intend to keep
"terser", confirm it’s installed at the workspace root.
49-56: Strengthen assertion by checking single invocation.Optional: ensure the plugin calls
defineEnvexactly once per build.expect(mockDefineEnv).toHaveBeenCalledWith( { VITE_TEST: "string" }, expect.objectContaining({ VITE_TEST: "test-value", }), ); + expect(mockDefineEnv).toHaveBeenCalledTimes(1);
139-144: Simplify RollupOutput check.You can assert the shape directly for readability.
- if (result && typeof result === "object" && "output" in result) { - expect(Array.isArray(result.output)).toBe(true); - expect(result.output.length).toBeGreaterThan(0); - } else { - throw new Error("Expected RollupOutput object with output property"); - } + expect(result).toHaveProperty("output"); + expect(Array.isArray((result as any).output)).toBe(true); + expect((result as any).output.length).toBeGreaterThan(0);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/vite-plugin/package.json(1 hunks)packages/vite-plugin/src/index.test.ts(1 hunks)packages/vite-plugin/vitest.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vite-plugin/package.json
🧰 Additional context used
🧬 Code graph analysis (2)
packages/vite-plugin/vitest.config.ts (2)
packages/arkenv/src/define-env.test.ts (4)
it(27-86)env(72-85)process(28-36)expect(38-52)packages/vite-plugin/src/index.ts (2)
config(17-19)T(15-20)
packages/vite-plugin/src/index.test.ts (2)
packages/arkenv/src/define-env.test.ts (4)
it(27-86)env(72-85)process(28-36)expect(38-52)packages/vite-plugin/src/index.ts (2)
T(15-20)config(17-19)
…d examples - Removed workspaces for packages and apps from package.json. - Updated arkenv dependency version to ^0.2.0 in multiple example projects. - Added new example projects to pnpm-workspace.yaml and included their dependencies in pnpm-lock.yaml. - Enhanced dependency management by including tsx and other related packages in lock file.
github-actions
bot
added
the
example
…lock.yaml - Bumped the version of @arkenv/vite-plugin from 0.0.2 to 0.0.4 in both package.json and pnpm-lock.yaml to ensure compatibility with the latest features and fixes. - Removed outdated ark.env dependency from pnpm-lock.yaml to streamline the dependency tree.
- Updated tsconfig.json in the basic example to include "outDir": "./dist" for better build output management. - Modified package.json in the with-bun example to add a "build" script for compiling TypeScript files to the dist directory.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
examples/basic/tsconfig.json (1)
9-11: Emit path addition is fine; consider excluding outputs from TS programoutDir "./dist" is fine. Optional: add an exclude at the root to keep TS from scanning build artifacts (and future "dist-test") in editor tooling.
Example addition (outside compilerOptions):
{ "exclude": ["dist", "dist-test"] }examples/with-bun/package.json (1)
7-9: Bun scripts are solid; ensure ESM/CJS settings are alignedThis package declares "type": "module" but the tsconfig (see examples/with-bun/tsconfig.json) sets "module": "commonjs". That mismatch can surprise imports and tooling.
Consider switching TS to ESM (ESNext + Bundler resolution) to match "type": "module" and Bun defaults. See suggested tsconfig diff in the tsconfig review.
Also applies to: 11-11
examples/with-bun/tsconfig.json (1)
28-28: Align module system with package.json "type": "module" and BunCommonJS here conflicts with ESM package type and may cause interop friction. Recommend ESM settings and modern resolution.
Apply:
- "module": "commonjs" /* Specify what module code is generated. */, + "module": "ESNext", + "moduleResolution": "Bundler",Optional quality-of-life:
- "target": "es2016" + "target": "ESNext"And consider an output dir consistent with scripts:
+ "outDir": "./dist"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
examples/basic/package.json(1 hunks)examples/basic/tsconfig.json(1 hunks)examples/with-bun/package.json(1 hunks)examples/with-bun/tsconfig.json(1 hunks)examples/with-vite-react-ts/package.json(1 hunks)package.json(0 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- package.json
✅ Files skipped from review due to trivial changes (1)
- examples/basic/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
examples/with-vite-react-ts/package.json (2)
examples/with-vite-react-ts/src/App.tsx (2)
App(6-34)count(21-21)packages/vite-plugin/src/index.ts (2)
T(15-20)config(17-19)
🔇 Additional comments (1)
pnpm-workspace.yaml (1)
4-4: Workspace scope expansion LGTMIncluding examples/* in the workspace is correct and avoids ad-hoc linking. No issues.
…#110) Add Bun installation to CI and a build script to the `with-bun` example to fix CI build failures. The CI build was failing for the `with-bun` example because the `bun` runtime was not available in the GitHub Actions environment, and the `examples/with-bun/package.json` was missing a `build` script that Turborepo expected to execute. This PR resolves both issues. --- <a href="https://cursor.com/background-agent?bcId=bc-0576b958-3bf0-48b9-9af4-23f9d614b0af"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"> <source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"> <img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"> </picture> </a> <a href="https://cursor.com/agents?id=bc-0576b958-3bf0-48b9-9af4-23f9d614b0af"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"> <source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"> <img alt="Open in Web" src="https://cursor.com/open-in-web.svg"> </picture> </a> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
…-lock.yaml - Bumped the version of @vitejs/plugin-react from 4.7.0 to 5.0.2 in both package.json files for improved compatibility and features. - Updated related dependencies in pnpm-lock.yaml to reflect the changes in the plugin version.
- Added a new "typecheck" configuration in turbo.json to specify TypeScript input files. - Updated tsconfig.json in the vite-plugin package to change module resolution from "node" to "bundler" for improved compatibility with modern build tools.
dist-test/to.gitignoreto exclude test build outputs.biome.jsoncto ignoredist-test/in the configuration.jsdomas a new dependency inpackage.jsonand updatedpnpm-lock.yamlaccordingly.vitest.config.tsto specify project paths for testing.vitestas a dev dependency in multiple example projects.viteversion in thevite-pluginpackage to ensure compatibility.Summary by CodeRabbit