feat: replace jest with vitest [sc-23945] #1061
Conversation
Jest lacks native TypeScript support and is annoying to deal with. This PR replaces Jest with Vitest which is largely compatible with the way tests are defined in Jest. Some code changes had to be made to make the switch possible.
WalkthroughThis change migrates the testing framework across two CLI-related packages from Jest to Vitest. All test files are updated to use explicit imports for Vitest testing utilities, replacing any implicit or Jest-based imports and mocking functions. Node.js built-in module imports are modernized to use the Changes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
packages/create-cli/vitest.config.mts (1)
3-10: Approve Vitest configuration and suggest including additional patterns.
Thetest.environmentandincludesettings look good for unit tests, but if you ever use.test.tssuffixes alongside.spec.ts, consider adding that pattern.test: { environment: 'node', include: [ './src/**/*.spec.ts', + './src/**/*.test.ts', ], },packages/create-cli/src/utils/prompts.ts (1)
1-1: Optional: Modernize built-in module import.
For consistency across packages, you might consider updatingimport * as path from 'path'toimport path from 'node:path', aligning with the other Node.js specifiers.packages/create-cli/vitest.config.e2e.mts (1)
1-15: Consider adding a timeout configuration for e2e testsThe Vitest configuration for e2e tests looks good overall, with appropriate environment, include, and exclude patterns. However, since end-to-end tests typically take longer than unit tests, consider adding a timeout configuration.
export default defineConfig({ test: { environment: 'node', include: [ './e2e/__tests__/**/*.spec.ts', ], exclude: [ '**/fixtures/**', '**/node_modules/**', '**/dist/**', ], + testTimeout: 15000, // 15 seconds for e2e tests }, })packages/create-cli/package.json (1)
15-16: Update test scripts to use Vitest
The"test"and"test:e2e"scripts have been updated to invoke Vitest instead of Jest—this aligns with the PR objective. Please ensure that any CI/pipeline definitions (e.g., GitHub Actions workflows) are updated accordingly. Additionally, consider running Vitest in non-watch mode for CI by changing"test": "vitest"to"test": "vitest run"or adding--watch=false.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/cli/src/reporters/__tests__/__snapshots__/github-md-builder.spec.ts.snapis excluded by!**/*.snappackages/cli/src/reporters/__tests__/__snapshots__/json-builder.spec.ts.snapis excluded by!**/*.snappackages/cli/src/reporters/__tests__/__snapshots__/util.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (55)
package.json(1 hunks)packages/cli/e2e/__tests__/check-parse-error.spec.ts(1 hunks)packages/cli/e2e/__tests__/deploy.spec.ts(1 hunks)packages/cli/e2e/__tests__/destroy.spec.ts(1 hunks)packages/cli/e2e/__tests__/env/env.add.spec.ts(1 hunks)packages/cli/e2e/__tests__/env/env.ls.spec.ts(1 hunks)packages/cli/e2e/__tests__/env/env.pull.spec.ts(1 hunks)packages/cli/e2e/__tests__/env/env.rm.spec.ts(1 hunks)packages/cli/e2e/__tests__/env/env.upate.spec.ts(1 hunks)packages/cli/e2e/__tests__/help.spec.ts(1 hunks)packages/cli/e2e/__tests__/login.spec.ts(1 hunks)packages/cli/e2e/__tests__/logout.spec.ts(1 hunks)packages/cli/e2e/__tests__/switch.spec.ts(1 hunks)packages/cli/e2e/__tests__/sync-playwright.spec.ts(1 hunks)packages/cli/e2e/__tests__/test.spec.ts(1 hunks)packages/cli/e2e/__tests__/trigger.spec.ts(1 hunks)packages/cli/e2e/__tests__/whoami.spec.ts(1 hunks)packages/cli/package.json(3 hunks)packages/cli/src/constructs/__tests__/api-check.spec.ts(1 hunks)packages/cli/src/constructs/__tests__/browser-check.spec.ts(1 hunks)packages/cli/src/constructs/__tests__/multi-step-check.spec.ts(1 hunks)packages/cli/src/constructs/__tests__/tcp-check.spec.ts(1 hunks)packages/cli/src/reporters/__tests__/github-md-builder.spec.ts(1 hunks)packages/cli/src/reporters/__tests__/json-builder.spec.ts(1 hunks)packages/cli/src/reporters/__tests__/util.spec.ts(1 hunks)packages/cli/src/services/__tests__/checkly-config-loader.spec.ts(1 hunks)packages/cli/src/services/__tests__/config.spec.ts(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/empty-script-project/src/empty-envvar.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/multiple-glob-patterns-project/__checks1__/__nested-checks__/test.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/multiple-glob-patterns-project/__checks1__/test.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/multiple-glob-patterns-project/__checks2__/test.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/simple-project-with-pl/src/login.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/simple-project/src/login.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser-fixtures/simple-project/src/services/search-service.check.js(1 hunks)packages/cli/src/services/__tests__/project-parser.spec.ts(2 hunks)packages/cli/src/services/__tests__/test-filters.spec.ts(1 hunks)packages/cli/src/services/__tests__/util.spec.ts(1 hunks)packages/cli/src/services/check-parser/__tests__/check-parser.spec.ts(1 hunks)packages/cli/src/services/check-parser/__tests__/parse-files.spec.ts(1 hunks)packages/cli/src/services/util.ts(1 hunks)packages/cli/vitest.config.e2e.mts(1 hunks)packages/cli/vitest.config.mts(1 hunks)packages/create-cli/e2e/__tests__/bootstrap.spec.ts(1 hunks)packages/create-cli/package.json(3 hunks)packages/create-cli/src/actions/dependencies.ts(1 hunks)packages/create-cli/src/actions/git.ts(1 hunks)packages/create-cli/src/utils/__tests__/directory.spec.ts(1 hunks)packages/create-cli/src/utils/__tests__/prompts.spec.ts(2 hunks)packages/create-cli/src/utils/fileloader.ts(1 hunks)packages/create-cli/src/utils/fullname.ts(1 hunks)packages/create-cli/src/utils/prompts.ts(1 hunks)packages/create-cli/src/utils/terminal.ts(1 hunks)packages/create-cli/tsconfig.json(1 hunks)packages/create-cli/vitest.config.e2e.mts(1 hunks)packages/create-cli/vitest.config.mts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/create-cli/src/utils/__tests__/prompts.spec.ts (1)
packages/create-cli/src/utils/directory.ts (1)
generateProjectName(92-99)
packages/cli/src/services/__tests__/project-parser.spec.ts (1)
packages/cli/src/rest/api.ts (1)
privateLocations(98-98)
🪛 GitHub Actions: test
packages/cli/src/services/__tests__/project-parser.spec.ts
[error] 125-125: Test failure in parseProject() > should parse a project with TypeScript check files due to construct creation error.
🔇 Additional comments (71)
package.json (1)
33-33: Skip trivial formatting change.
A single newline was added at the end of the file; no functional impact.packages/create-cli/vitest.config.mts (1)
1-2: Approve import ofdefineConfig.
UsingdefineConfigfromvitest/configis correct and follows Vitest conventions.packages/cli/src/reporters/__tests__/json-builder.spec.ts (1)
1-2: Explicit Vitest imports added as needed.
Bringing indescribe,expect, andtestfrom Vitest removes reliance on globals and aligns with the framework migration.packages/cli/src/reporters/__tests__/github-md-builder.spec.ts (1)
1-2: Explicit Vitest imports added for clarity.
Importingdescribe,expect, andtestfrom Vitest is consistent with other tests and avoids implicit globals.packages/cli/src/services/__tests__/project-parser-fixtures/multiple-glob-patterns-project/__checks2__/test.check.js (1)
2-2: Verify ESM support for.jsfixtures.
You've replaced the CommonJSrequirewith an ES moduleimport. Ensure that this directory or its nearestpackage.jsonhas"type": "module"so Node/Vitest treats.jsas ESM. Otherwise, consider renaming this file to.mjsor adjusting the project’s module type.packages/cli/src/services/__tests__/test-filters.spec.ts (1)
1-1: Explicit Vitest imports are correct.
Importingdescribe,expect, andtestfrom'vitest'aligns perfectly with the Jest→Vitest migration and makes the test utilities explicit without altering test logic.packages/cli/e2e/__tests__/check-parse-error.spec.ts (2)
1-1: Modernize Node.js import.
Switching toimport path from 'node:path'follows the current ESM best practice and makes the specifier explicit.
4-4: Explicit Vitest imports added.
Replacing implicit Jest globals withimport { describe, it, expect } from 'vitest'correctly wires up Vitest and preserves the original test behavior.packages/cli/src/constructs/__tests__/multi-step-check.spec.ts (1)
1-1: Explicit Vitest imports are correct.
Addingimport { describe, it, expect } from 'vitest'ensures clarity about the test framework being used and completes the Jest→Vitest migration.packages/create-cli/src/utils/prompts.ts (1)
2-3: Default imports with ESModuleInterop.
Switching toimport chalk from 'chalk'andimport prompts from 'prompts'leveragesesModuleInteropand modern ESM patterns. Ensure that yourtsconfig.jsonhas"esModuleInterop": trueenabled to support these imports.packages/cli/src/services/__tests__/project-parser-fixtures/simple-project-with-pl/src/login.check.js (1)
2-2: Verify ESM configuration for fixture.jsfiles.
Converting from CommonJSrequireto animportstatement in a.jsfixture assumes ES module resolution (e.g.,"type": "module"inpackage.jsonor appropriate Vitest loader settings). Please confirm that the test runner and project configuration handle this file as an ES module at runtime.packages/cli/src/constructs/__tests__/tcp-check.spec.ts (1)
1-2: The Vitest import looks good.This change correctly adds explicit imports for the test utilities from Vitest, which is necessary when migrating from Jest (where these are globally available) to Vitest (where explicit imports are required).
packages/cli/src/services/__tests__/project-parser-fixtures/multiple-glob-patterns-project/__checks1__/__nested-checks__/test.check.js (1)
2-2: Good conversion from CommonJS to ESM import.This change correctly converts a CommonJS
requirestatement to a modern ES moduleimportstatement, which aligns with the modernization efforts in the codebase. This also removes the need for the ESLint disable comment for@typescript-eslint/no-var-requiresthat was likely present in the original code.packages/cli/e2e/__tests__/logout.spec.ts (1)
1-2: The Vitest import looks good.This change correctly adds explicit imports for the test utilities from Vitest, which is necessary when migrating from Jest (where these are globally available) to Vitest (where explicit imports are required).
packages/create-cli/src/utils/fullname.ts (1)
1-2: Good conversion to default imports.This change correctly converts namespace imports (
import * as ...) to default imports (import ... from ...), which is a more modern approach and consistent with the codebase modernization. This would be made possible by theesModuleInteropflag in the TypeScript configuration.packages/cli/e2e/__tests__/help.spec.ts (1)
1-2: The Vitest import looks good.This change correctly adds explicit imports for the test utilities from Vitest, which is necessary when migrating from Jest (where these are globally available) to Vitest (where explicit imports are required).
packages/cli/e2e/__tests__/login.spec.ts (1)
2-3: Core implementation of Jest to Vitest migration.Explicitly importing test functions from Vitest instead of relying on Jest globals is a key aspect of the migration. This approach:
- Provides better clarity about where testing utilities come from
- Follows ESM best practices by avoiding global namespace pollution
- Directly addresses the PR's main objective
The test code remains unchanged otherwise, demonstrating Vitest's compatibility with Jest's API.
packages/cli/src/services/__tests__/project-parser-fixtures/simple-project/src/login.check.js (1)
2-2: ESM import conversion is correct for BrowserCheck.Replacing the previous CommonJS
requirewith an ES moduleimportaligns with the overall migration to ES modules in test fixtures.packages/cli/e2e/__tests__/switch.spec.ts (1)
2-2: Add explicit Vitest imports.Importing
describe,it, andexpectfrom Vitest is necessary to support the new testing framework and makes test dependencies explicit.packages/cli/src/services/__tests__/project-parser-fixtures/multiple-glob-patterns-project/__checks1__/test.check.js (1)
2-2: ESM import conversion for BrowserCheck.Switching from
requiretoimportforBrowserCheckmaintains consistency with the rest of the ES module–based test fixtures.packages/cli/src/services/__tests__/checkly-config-loader.spec.ts (2)
1-1: Modernize Node.js built-in import.Using
import path from 'node:path'follows the new standard for built-in modules and ensures clarity.
3-3: Add explicit Vitest imports.Bringing in
describe,it, andexpectfrom Vitest is required for the framework migration and improves readability.packages/cli/e2e/__tests__/trigger.spec.ts (2)
1-1: Modernize Node.js built-in import.Standardizing the
pathimport toimport path from 'node:path'aligns with the project’s module import conventions.
5-5: Add explicit Vitest imports.Including
describe,expect,beforeAll,afterAll, andtestfrom Vitest is essential to fully transition the tests to the new framework.packages/create-cli/src/utils/__tests__/directory.spec.ts (1)
1-3: Good modernization of imports.The changes properly modernize the path import using the
node:prefix and explicitly import test utilities from Vitest. This aligns well with the migration from Jest to Vitest.packages/cli/e2e/__tests__/sync-playwright.spec.ts (1)
1-4: Well-structured imports for test framework migration.The changes correctly update Node.js built-in module imports to use the
node:prefix and default import style. Explicitly importing Vitest test utilities follows best practices for the new testing framework.packages/cli/src/services/__tests__/util.spec.ts (1)
1-3: Clean migration to Vitest imports.The import updates properly follow the pattern used throughout the codebase, with the
node:prefix for built-in modules and explicit test utility imports from Vitest.packages/cli/src/reporters/__tests__/util.spec.ts (2)
1-2: Good organization of imports.Moving the
luxonimport to the top and explicitly importing Vitest test utilities improves code organization and clarity.
15-17: Properly configured test environment.Adding the
beforeAllhook to set the default timezone ensures consistent test behavior across environments. This is important for tests that might involve date/time formatting.packages/cli/e2e/__tests__/env/env.ls.spec.ts (2)
1-1: Good update to modern Node.js module import styleUsing the
node:prefix for built-in modules is a best practice in modern Node.js, as it clearly distinguishes built-in modules from third-party packages and provides better error messages.
5-5: Appropriate explicit imports for Vitest testing functionsExplicitly importing testing functions from Vitest instead of relying on globals (which was common with Jest) is a good practice that makes dependencies clear and improves IDE support.
packages/create-cli/src/actions/dependencies.ts (1)
1-3: Good modernization of import statementsThe changes to use the
node:prefix for built-in modules (fs,path) and switching to default imports align with modern Node.js practices and provide better clarity on module sources.packages/cli/e2e/__tests__/whoami.spec.ts (1)
2-4: Appropriate Vitest imports and improved import orderingThe explicit imports from Vitest and the reordering of imports (third-party modules first, then local modules) improve code clarity and follow common import conventions.
packages/cli/src/services/__tests__/config.spec.ts (2)
2-2: Proper Vitest testing function importsExplicitly importing testing functions and the mocking utility (
vi) from Vitest follows best practices by making dependencies explicit rather than relying on globals.
5-5: Correct update of mocking syntaxChanging from Jest's mocking utility to Vitest's equivalent (
vi.mock) maintains the same functionality while properly adapting to the new testing framework.packages/cli/e2e/__tests__/env/env.add.spec.ts (1)
1-1: Import updates look good for Vitest migrationThe changes correctly implement two key aspects of the Jest to Vitest migration:
- Using the modern Node.js import style with the
node:prefix- Explicitly importing test utilities from Vitest instead of relying on Jest globals
This is consistent with the migration pattern across the codebase.
Also applies to: 5-5
packages/cli/e2e/__tests__/env/env.rm.spec.ts (1)
1-1: Import updates look good for Vitest migrationThe changes correctly implement two key aspects of the Jest to Vitest migration:
- Using the modern Node.js import style with the
node:prefix- Explicitly importing test utilities from Vitest instead of relying on Jest globals
This is consistent with the migration pattern seen in other test files.
Also applies to: 5-5
packages/cli/e2e/__tests__/destroy.spec.ts (1)
1-1: Import updates look good for Vitest migrationThe changes correctly implement two key aspects of the Jest to Vitest migration:
- Using the modern Node.js import style with the
node:prefix- Explicitly importing test utilities from Vitest instead of relying on Jest globals
This is consistent with the migration pattern seen in other test files.
Also applies to: 5-5
packages/cli/src/constructs/__tests__/browser-check.spec.ts (2)
1-2: Updated import statements follow modern Node.js conventions.The change from namespace imports to default imports with the
node:prefix aligns with modern JavaScript best practices, improving code clarity and potentially performance.
4-4: Explicit vitest imports replace implicit Jest globals.Explicitly importing test functions from Vitest increases code clarity by making dependencies explicit and aligns with the project goal of replacing Jest with Vitest.
packages/cli/e2e/__tests__/env/env.upate.spec.ts (2)
1-1: Node.js import updated to use modern convention.Updated from namespace import to default import with the
node:prefix, which is the recommended approach for built-in Node.js modules.
5-5: Added explicit Vitest test function imports.Test functions are now explicitly imported from Vitest rather than relying on implicit globals, which improves code clarity and enables better static analysis.
packages/cli/e2e/__tests__/env/env.pull.spec.ts (2)
1-2: Updated Node.js imports to use modern conventions.Changed from namespace imports to default imports with the
node:prefix, following current best practices for Node.js modules.
6-6: Added explicit Vitest test function imports.Test functions are now explicitly imported from Vitest instead of relying on Jest globals, improving code clarity and supporting the migration to Vitest.
packages/cli/src/constructs/__tests__/api-check.spec.ts (2)
1-2: Node.js imports follow modern conventions.Updated from namespace imports to default imports with the
node:prefix, aligning with current Node.js best practices.
4-4: Explicit Vitest test function imports added.Test functions are now explicitly imported from Vitest instead of being implicitly available as Jest globals, improving code clarity and supporting type checking.
packages/cli/e2e/__tests__/deploy.spec.ts (1)
1-1: LGTM! Modernized module imports and explicit Vitest imports.The imports have been properly updated to use the Node.js prefix pattern and explicit Vitest imports are now correctly defined.
Also applies to: 7-10
packages/cli/src/services/check-parser/__tests__/check-parser.spec.ts (1)
1-3: LGTM! Clean import modernization.The import statements have been appropriately modernized to use the Node.js prefix pattern and explicit Vitest imports.
packages/cli/vitest.config.mts (1)
1-16:✅ Verification successful
LGTM! Well-structured Vitest configuration.
The Vitest configuration is well-organized with appropriate environment settings and include/exclude patterns.
🏁 Script executed:
#!/bin/bash # Verify that the test configuration works properly by checking for any test files that might be missed # Check if there are any spec files that wouldn't be included by the pattern find ./packages/cli/src -name "*.spec.ts" | grep -v "fixtures\|node_modules\|dist" | grep -v "./src" # Verify that the vitest dependency is correctly added to package.json grep -A 5 "vitest" packages/cli/package.jsonLength of output: 562
✅ Vitest configuration verified
- No
.spec.tsfiles are excluded by theinclude/excludepatterns in packages/cli/vitest.config.mts.vitest@3.1.2 is correctly listed in packages/cli/package.json.Looks good to merge!
packages/cli/package.json (2)
18-19: Scripts Updated to Vitest
The"test"and"test:e2e"commands are now invoking Vitest instead of Jest. Ensure that the end-to-end suite correctly referencesvitest.config.e2e.mtsand that any required environment variables (NODE_CONFIG_DIR) are properly set during CI and local runs.
119-120: DevDependencies: Vitest Introduced & TypeScript Bumped
Vitest has been added todevDependenciesand the TypeScript version was updated. Confirm there are no lingering Jest-related packages elsewhere in the monorepo to prevent conflicting test runners.packages/cli/vitest.config.e2e.mts (1)
1-16: End-to-End Vitest Configuration
The E2E config correctly sets the Node environment, includes only./e2e/__tests__/**/*.spec.ts, excludes fixtures,node_modules, anddist, and applies a 15 s timeout. This aligns with the updatedtest:e2escript.packages/create-cli/src/utils/__tests__/prompts.spec.ts (2)
2-6: Modernize Imports for Vitest & Node Built-ins
Replaced Jest globals and implicit Node imports with explicit ESM imports (node:path) and Vitest functions. This ensures type safety and clarity in test files.
18-25: Switch tovifor Spies & Mocks
Updatedjest.spyOn/jest.fn()/jest.clearAllMocks()tovi.spyOn/vi.fn()/vi.clearAllMocks(). Good alignment with Vitest’s mocking API.packages/cli/e2e/__tests__/test.spec.ts (2)
1-7: Explicit Vitest & Node Built-in Imports
Importingdescribe,it, andexpectfrom Vitest and usingnode:path/node:fsis consistent with the migration. This removes reliance on globals and ensures compatibility with ESM.
10-10: Custom Suite Timeout
Adding{ timeout: 45000 }todescriberaises the default timeout for all contained tests. This aligns with potentially slow E2E runs—good catch.packages/cli/src/services/check-parser/__tests__/parse-files.spec.ts (1)
1-4: Adopt ESM & Vitest Imports
Changedimport * as path from 'path'toimport path from 'node:path'and added explicitvitestimports (describe,it,expect). This standardizes core module usage and test utility access.packages/create-cli/e2e/__tests__/bootstrap.spec.ts (2)
1-4: Modernized Node.js module imports withnode:prefixThe change to use the
node:prefix for built-in Node.js modules is a good practice. This makes the import source explicit and helps avoid potential conflicts with third-party packages of the same name.
8-9: Explicit Vitest imports replace implicit Jest globalsThe explicit imports from Vitest are a good practice as they make dependencies clear and improve IDE support for autocomplete and type checking. This change is consistent with the PR objective of replacing Jest with Vitest.
packages/cli/src/services/__tests__/project-parser.spec.ts (4)
1-2: Modernized Node.js module import withnode:prefixGood update to use the standardized
node:prefix for the path module.
5-6: Explicit Vitest imports replace implicit Jest globalsExplicitly importing test functions from Vitest follows modern JavaScript best practices by making dependencies clear.
28-29: Updated mock methods from Jest to Vitest equivalentsThe change from
jest.resetAllMocks()tovi.resetAllMocks()and fromjest.spyOn()tovi.spyOn()correctly adapts the code to use Vitest's mocking API instead of Jest's.
123-141:❌ Incorrect review comment
Fix failing TypeScript project test
There's a pipeline failure in this test. The error suggests there's an issue with the construct creation during the TypeScript project parsing.
🏁 Script executed:
#!/bin/bash # Description: Verify the structure of the TypeScript project fixture # Check the fixture structure ls -la "$(dirname "$0")/packages/cli/src/services/__tests__/project-parser-fixtures/typescript-project" cat "$(dirname "$0")/packages/cli/src/services/__tests__/project-parser-fixtures/typescript-project/checkly.config.ts" # Check runtimes handling in parseProject function grep -r "availableRuntimes" "$(dirname "$0")/packages/cli/src/services"Length of output: 789
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "1. Locate the TypeScript fixture directory:" TS_FIX_DIR=$(find . -type d -path "./packages/cli/src/services/__tests__/*/project-parser-fixtures/typescript-project" || true) if [[ -z "$TS_FIX_DIR" ]]; then echo "⚠️ Could not find 'typescript-project' under project-parser-fixtures." exit 1 fi echo "Found fixture at: $TS_FIX_DIR" echo echo "2. List all files in the fixture:" ls -R "$TS_FIX_DIR" echo echo "3. Show contents of checkly.config.ts:" cat "$TS_FIX_DIR/checkly.config.ts" || echo "⚠️ File not found: $TS_FIX_DIR/checkly.config.ts" echo echo "4. Inspect parseProject implementation for runtime handling:" rg --context 3 "function parseProject" -n packages/cli/src/services rg --context 3 "availableRuntimes" -n packages/cli/src/servicesLength of output: 417
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "1. Find the project-parser-fixtures directory:" FIX_BASE=$(find packages/cli/src/services/__tests__ -type d -name project-parser-fixtures || true) if [[ -z "$FIX_BASE" ]]; then echo "⚠️ Could not locate project-parser-fixtures under __tests__" exit 1 fi echo "Fixture base found at: $FIX_BASE" echo echo "2. List subdirectories under project-parser-fixtures:" find "$FIX_BASE" -maxdepth 1 -type d | sed '1d' echo echo "3. Locate and inspect the TypeScript fixture:" TS_DIR=$(find "$FIX_BASE" -type d -name typescript-project || true) if [[ -z "$TS_DIR" ]]; then echo "⚠️ No 'typescript-project' directory found under $FIX_BASE" exit 1 fi echo "TypeScript project fixture: $TS_DIR" echo "Contents:" ls -R "$TS_DIR" echo echo "4. Show checkly.config.ts if present:" if [[ -f "$TS_DIR/checkly.config.ts" ]]; then cat "$TS_DIR/checkly.config.ts" else echo "⚠️ $TS_DIR/checkly.config.ts does not exist" fi echo echo "5. Inspect parseProject implementation for handling availableRuntimes:" rg -n "export async function parseProject" -C3 packages/cli/src/services rg -n "availableRuntimes" -C3 packages/cli/src/servicesLength of output: 10352
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "1. Display top of project-parser.ts (1-200):" sed -n '1,200p' packages/cli/src/services/project-parser.ts || true echo echo "2. Display file scanning logic around globbing/includes (200-350):" sed -n '200,350p' packages/cli/src/services/project-parser.ts || true echo echo "3. Search for 'globby' usage:" rg -n "globby" -C3 packages/cli/src/services/project-parser.ts || true echo echo "4. Search for 'require' invocations:" rg -n "require(" -C3 packages/cli/src/services/project-parser.ts || true echo echo "5. Search for '.ts' handling or ts-node mentions:" rg -n "ts-node" -C3 packages/cli/src/services || true rg -n ".ts" -C3 packages/cli/src/services/project-parser.ts || trueLength of output: 15160
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Displaying TypeScript fixture check file content:" cat packages/cli/src/services/__tests__/project-parser-fixtures/typescript-project/ts-test.check.ts || echo "⚠️ File not found"Length of output: 686
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "1. Inspect loadFile implementation in util.ts:" rg -n -C5 "export async function loadFile" packages/cli/src/services/util.ts echo echo "2. Inspect findFilesWithPattern in util.ts:" rg -n -C5 "export async function findFilesWithPattern" packages/cli/src/services/util.tsLength of output: 986
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "1. Locate loadTsFileDefault definition and surrounding logic:" rg -n -C5 "async function loadTsFileDefault" packages/cli/src/services/util.ts || true echo echo "2. Display loadTsFileDefault implementation (approx lines 130-200):" sed -n '130,200p' packages/cli/src/services/util.ts || trueLength of output: 3101
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "1. Dump the findFilesWithPattern function body:" sed -n '267,350p' packages/cli/src/services/util.ts || true echo echo "2. Search for globby invocation within findFilesWithPattern:" rg -n "globby" -C5 packages/cli/src/services/util.ts || true echo echo "3. Check if brace expansion is enabled for patterns:" rg -n "brace" -C5 packages/cli/src/services/util.ts || trueLength of output: 937
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Extracting loadFile implementation in util.ts:" sed -n '55,150p' packages/cli/src/services/util.ts || trueLength of output: 2918
Ignore TypeScript fixture parsing—no code change needed
Thetypescript-projectfixture (ts-test.check.ts) is correctly picked up by the default glob (**/*.check.{js,ts}) and loaded vialoadFile(which uses Jiti or ts-node to handle.tsfiles). The import path (../../../../constructs) resolves properly, andparseProjectalready supports TypeScript checks. Please verify that your CI environment has the requiredjitiorts-nodedependency installed so the test runner can compile TS files.Likely an incorrect or invalid review comment.
🧰 Tools
🪛 GitHub Actions: test
[error] 125-125: Test failure in parseProject() > should parse a project with TypeScript check files due to construct creation error.
packages/cli/src/services/util.ts (1)
61-63: Improved handling of default exports when importing non-TypeScript filesThis change improves robustness by:
- Storing the entire imported module in a temporary variable
- Safely accessing the default export with the nullish coalescing operator
- Falling back to an empty object if the default export is missing
This prevents
exportedfrom beingundefinedwhen there's no default export.packages/create-cli/src/utils/fileloader.ts (1)
10-12: Improved handling of default exports when importing non-TypeScript filesThis change matches the similar improvement in
packages/cli/src/services/util.ts, creating consistency across the codebase in how default exports are handled for dynamic imports.The change makes the code more robust by:
- Storing the entire imported module in a temporary variable
- Safely accessing the default export with the nullish coalescing operator
- Falling back to an empty object if the default export is missing
packages/create-cli/package.json (1)
84-85: Verify TypeScript and Vitest version alignment
You’ve added"typescript": "^5.3.3"and pinned"vitest": "3.1.2". Confirm that these versions are compatible with the rest of the monorepo (to avoid multiple TS instances) and that Vitest’s version is consistent across all packages.packages/create-cli/src/utils/terminal.ts (1)
2-2: Approve default import forora.
Switching from a namespace import toimport ora from 'ora'is correct givenesModuleInteropis enabled, and it aligns with the package’s default export.packages/cli/src/services/__tests__/project-parser-fixtures/empty-script-project/src/empty-envvar.check.js (1)
2-2: Migrate fixture to ES module import.
Replacing the CommonJSrequirewith an ESimportforBrowserCheckandCheckGroupaligns with the Vitest migration and maintains consistent import syntax across the codebase.packages/create-cli/tsconfig.json (1)
6-8: Enable ES module interoperability.
Adding"esModuleInterop": trueis necessary to support default imports from CommonJS modules and underpins the new ESM-style import patterns used throughout the package.packages/create-cli/src/actions/git.ts (2)
1-2: Standardize built-in module imports withnode:prefixes.
Switching toimport fs from 'node:fs'andimport path from 'node:path'enhances clarity and aligns with modern ESM conventions.
4-4: Adopt default import forexeca.
Usingimport execa from 'execa'matches ES module conventions and synergies with the new Vitest setup.
| const path = require('node:path') | ||
| import { BrowserCheck, ApiCheck } from '../../../../../../constructs' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent module system usage.
The file mixes CommonJS (require) and ES Module (import) syntax. While modernizing imports, it's best to use a consistent approach.
Fully convert to ES Module syntax:
-const path = require('node:path')
+import path from 'node:path'
import { BrowserCheck, ApiCheck } from '../../../../../../constructs'📝 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.
| const path = require('node:path') | |
| import { BrowserCheck, ApiCheck } from '../../../../../../constructs' | |
| import path from 'node:path' | |
| import { BrowserCheck, ApiCheck } from '../../../../../../constructs' |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
30-41: Enforce consistent shell across OS runners (optional)
Onwindows-latest, GitHub Actions defaults to PowerShell, which may break bash-centric commands. To maintain a uniform environment, explicitly set the shell tobashfor eachrunstep:jobs: test: steps: - run: npm run prepack + shell: bash - run: npm run test + shell: bash - run: npm run test:e2e + shell: bash🧰 Tools
🪛 actionlint (1.7.4)
31-31: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/test.yml (1)
26-29: Add Windows runner to test matrix
This expands CI coverage to include Windows. Verify that all existing test, prepack, and lint steps execute successfully onwindows-latest.
Jest lacks native TypeScript support and is annoying to deal with. This PR replaces Jest with Vitest which is largely compatible with the way tests are defined in Jest.
Some code changes had to be made to make the switch possible.
Unfortunately, even Vitest is not completely pain free. This PR explores the feasibility of the switch and if doable, will be merged.
One test had to be disabled for now. The issue is that Vitest has a TypeScript loader of its own which conflicts with ours. For TypeScript check files that make references back to the uncompiled project (i.e.
../../../../constructsand notcheckly/constructs), theproject.tsseen by the check file and theproject.tsseen by the test executor are not the same. I am not sure if fixing this is feasible without significantly reworking the affected tests. In my opinion the tests are sort of useless when they reference the uncompiled codebase as the environment doesn't match the actual environment seen by users. For example, one easy fix for the issue would be to detect a TypeScript-capable environment and simplyimportinstead of usingjitiorts-node, but then that wouldn't match the actual environment at all. However, at least e2e tests cover the use case well enough.Affected Components
Notes for the Reviewer
New Dependency Submission