-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Integrate Vitest for testing and coverage in the temporal worker #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Added Vitest configuration for testing with coverage reporting. - Created initial test for the `run` function to ensure it returns true. - Updated `package.json` and `package-lock.json` to include necessary dependencies for testing. - Configured SonarQube to include JavaScript coverage reports and exclude test files from analysis. These changes enhance the testing framework for the temporal worker, ensuring better code quality and coverage tracking.
📝 Walkthrough""" WalkthroughThis update introduces a new Node.js package in Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Node.js (workers/main)
participant SonarQube
GitHub Actions->>Node.js (workers/main): Install dependencies (npm ci)
GitHub Actions->>Node.js (workers/main): Run tests with coverage (npm run coverage)
Node.js (workers/main)->>Node.js (workers/main): Generate coverage report (lcov.info)
GitHub Actions->>SonarQube: Run SonarQube scan (importing coverage and applying exclusions)
Possibly related PRs
Suggested reviewers
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 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 (
|
🔍 Vulnerabilities of
|
| digest | sha256:00f0aff67492700391e8439dcf2afc1a9062e155537ac4aa76185c6b809b3131 |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 243 MB |
| packages | 1628 |
📦 Base Image node:20-alpine
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
…index - Added a newline at the end of the vitest.config.ts file to adhere to best practices. - Added a newline at the end of the index.ts file to ensure proper file termination. These changes improve code quality and maintain consistency across files.
- Added resolved URLs and integrity hashes for various dependencies in package-lock.json to ensure consistent installations. - Removed unnecessary nested dependencies for improved clarity and maintenance. These changes enhance the reliability of package installations and maintain consistency across environments.
There was a problem hiding this 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
🧹 Nitpick comments (6)
workers/main/package.json (1)
13-15: Consider consistent version pinning strategySome dependencies use fixed versions (vitest, @vitest/coverage-v8) while others use caret ranges (^). Consider adopting a consistent versioning strategy across all dependencies to ensure predictable builds.
- "ts-node": "^10.9.1", - "typescript": "^5.0.0", + "ts-node": "10.9.1", + "typescript": "5.0.0",workers/main/src/__tests__/index.test.ts (1)
4-8: Consider adding test documentationAs this is the first test in a new testing infrastructure, consider adding a brief comment explaining the purpose of the
runfunction and what specifically this test is verifying.describe('run', () => { + // Verifies that the worker's main entry point function resolves correctly it('should return true', async () => { await expect(run()).resolves.toBe(true); }); });.github/workflows/code-quality.yml (1)
28-31: Consider adding Node.js setup and dependency cachingThe implementation correctly adds steps to install dependencies and run tests with coverage. However, consider:
- Adding a
setup-nodestep to ensure a consistent Node.js version- Implementing dependency caching to speed up the workflow
- Setting appropriate error handling for the test step
+ - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '18' + cache: 'npm' + cache-dependency-path: 'workers/main/package-lock.json' - name: Install dependencies run: cd workers/main && npm ci - name: Run tests with coverage - run: cd workers/main && npm run coverage + run: cd workers/main && npm run coverage + continue-on-error: falsesonar-project.properties (1)
4-4: Scope exclusion patterns narrowly
The exclusions**/src/__tests__/**,**/src/dist/**will apply globally to anysrcdirectories in the repo. If the intent is to ignore only the tests and build artifacts for theworkers/mainpackage, consider more specific patterns, e.g.:sonar.exclusions=workers/main/src/__tests__/**,workers/main/src/dist/**workers/main/vitest.config.ts (2)
8-16: Validate coverage include/exclude patterns
Coverage is configured withall: true,include: ['src/**/*.ts'], and excludessrc/__tests__/**andsrc/dist/**. Ensure these globs match your project layout (e.g., if compiled output lives indist/at the project root, you may need to adjust the exclude pattern todist/**).
11-12: Beware of full-file coverage enforcement
Enablingall: truemeans untested files undersrcwill count toward overall coverage. If you prefer to enforce metrics only on exercised modules, consider disablingallor narrowing theincludepatterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
workers/main/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/workflows/code-quality.yml(1 hunks)sonar-project.properties(1 hunks)workers/main/package.json(1 hunks)workers/main/src/__tests__/index.test.ts(1 hunks)workers/main/src/index.ts(1 hunks)workers/main/vitest.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
workers/main/src/__tests__/index.test.ts (1)
workers/main/src/index.ts (1)
run(1-3)
🔇 Additional comments (6)
workers/main/package.json (1)
9-17:Details
✅ Verification successful
Verify package dependencies are complete
The package only includes development dependencies. If this is a worker package that needs to run in production, you might need to add runtime dependencies like the Temporal SDK client.
🏁 Script executed:
#!/bin/bash # Check if there are imports in the source files that require external packages cd workers/main grep -r "from '[^\.][^\/]" --include="*.ts" src/ | grep -v "vitest"Length of output: 88
🏁 Script executed:
#!/bin/bash # Re-run import scan excluding all relative paths to catch any runtime dependencies cd workers/main echo "Checking for external ES module imports in TS files..." grep -R "import .* from ['\"][^\.\/]" --include="*.ts" -n src/ || echo "No external imports found via import statements." echo "Checking for external CommonJS requires in TS files..." grep -R "require(['\"][^\.\/]" --include="*.ts" -n src/ || echo "No external requires found."Length of output: 523
No external runtime dependencies detected; devDependencies only is correct
A scan of all
.tsfiles (excluding tests) found no external ES module imports or CommonJS requires—only thevitestimport in your tests. Since there are no runtime imports in use, keeping everything underdevDependenciesis appropriate here.workers/main/src/__tests__/index.test.ts (1)
1-8: Good initial test implementationThe test correctly verifies the current behavior of the
runfunction. As the function implementation grows in complexity, consider expanding these tests accordingly..github/workflows/code-quality.yml (1)
28-31:Details
✅ Verification successful
Verify SonarQube configuration for coverage reports
The workflow now runs tests with coverage, but you should verify that SonarQube is correctly configured to import this coverage data. Make sure the
sonar-project.propertiesfile includes the JavaScript coverage report path.
🏁 Script executed:
#!/bin/bash # Check if sonar-project.properties file properly includes the JavaScript coverage report path grep -i "sonar.javascript.lcov.reportPaths" sonar-project.properties || echo "Coverage report path not configured in sonar-project.properties"Length of output: 134
SonarQube coverage report path is correctly configured
The
sonar-project.propertiesfile already includes:sonar.javascript.lcov.reportPaths=workers/main/coverage/lcov.infoNo further changes are needed to ensure coverage reports are imported.
sonar-project.properties (1)
3-3: Confirm LCOV report path
The propertysonar.javascript.lcov.reportPaths=workers/main/coverage/lcov.infoshould point to where Vitest emits the LCOV report. Please verify that runningnpm run coverageinworkers/mainactually generatescoverage/lcov.infoat this relative path before the SonarQube scan.workers/main/vitest.config.ts (2)
1-7: Approve core test configuration
The Vitest setup—includingglobals: true, the Node.js environment, and the explicitincludeglob—correctly targets the TypeScript tests undersrc/__tests__.
17-22: Approve coverage thresholds
Defining 80% minimum for statements, branches, functions, and lines establishes a strong baseline. Make sure your CI command (vitest --coverage) is set to fail the build when thresholds aren’t met.
- Updated the `@types/node` dependency version in both `package.json` and `package-lock.json` to ensure compatibility and improved performance. - Updated the `undici-types` dependency version in `package-lock.json` for better stability. - Added documentation comments in `index.ts` to clarify the purpose and return type of the `run` function. These changes improve dependency management and enhance code clarity through better documentation.
- Updated TypeScript version from `^5.0.0` to `5.8.3` in both `package.json` and `package-lock.json` for better performance and compatibility. - Updated the version of `@vitest/pretty-format` from `3.1.3` to `3.1.4` in `package-lock.json` to ensure the latest features and fixes are included. - Modified the import statement in the test file `index.test.ts` for consistency and clarity. These changes enhance the development environment by ensuring up-to-date dependencies and improving code readability in tests.
|
killev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Approved



runfunction to ensure it returns true.package.jsonandpackage-lock.jsonto include necessary dependencies for testing.These changes enhance the testing framework for the temporal worker, ensuring better code quality and coverage tracking.