added test runner for typescript tests #1349
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughUpdates a package.json script to use bunx tsx for running a TypeScript utility and adds a new Bun-based TypeScript test runner script that discovers and executes .test.ts files under the script/ directory. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
100-101: Align test:ts with the same convention (use bunx tsx).Our guideline says “script/**/*.ts: execute with bunx tsx”. Update the runner invocation accordingly.
- "test:ts": "bun script/runTypescriptTests.ts", + "test:ts": "bunx tsx script/runTypescriptTests.ts",
🧹 Nitpick comments (3)
script/runTypescriptTests.ts (3)
26-27: Use Dirent to avoid extra stat calls and simplify traversal.Less I/O and cleaner logic.
-import { readdirSync, statSync } from 'fs' +import { readdirSync } from 'fs' @@ - try { - const items = readdirSync(dir) - - for (const item of items) { - const fullPath = join(dir, item) - const stat = statSync(fullPath) - - if (stat.isDirectory()) { - // Skip lib/ and node_modules/ directories - if (item === 'lib' || item === 'node_modules') continue - - testFiles.push(...findTestFiles(fullPath)) - } else if (item.endsWith('.test.ts')) testFiles.push(fullPath) - } + try { + const items = readdirSync(dir, { withFileTypes: true }) + for (const entry of items) { + const fullPath = join(dir, entry.name) + if (entry.isDirectory()) { + // Skip common build/vendor dirs + if (entry.name === 'lib' || entry.name === 'node_modules' || entry.name === 'dist') continue + testFiles.push(...findTestFiles(fullPath)) + } else if (entry.isFile() && entry.name.endsWith('.test.ts')) { + testFiles.push(fullPath) + } + }Also applies to: 32-45
54-55: Deterministic output: sort discovered files before printing/running.Improves reproducibility in CI logs.
-const testFiles = findTestFiles(scriptDir) +const testFiles = findTestFiles(scriptDir).sort()
39-44: Consider skipping additional generated dirs (dist/build/coverage) if they ever appear under script/.Future-proofing to avoid accidental test pickup from compiled outputs or reports.
📜 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 (2)
package.json(1 hunks)script/runTypescriptTests.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.ts
📄 CodeRabbit inference engine (conventions.md)
Execute TypeScript scripts with bunx tsx
Files:
script/runTypescriptTests.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.784Z
Learning: Applies to script/**/*.ts : Execute TypeScript scripts with bunx tsx
Learnt from: 0xDEnYO
PR: lifinance/contracts#1139
File: package.json:81-82
Timestamp: 2025-05-14T03:33:25.089Z
Learning: Bun allows direct invocation of npm scripts defined in package.json without requiring the `bun run` prefix (e.g., `bun scriptName` works, not just `bun run scriptName`), unlike some other package managers like npm.
Learnt from: 0xDEnYO
PR: lifinance/contracts#1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
📚 Learning: 2025-08-29T09:56:59.784Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.784Z
Learning: Applies to script/**/*.ts : Execute TypeScript scripts with bunx tsx
Applied to files:
package.jsonscript/runTypescriptTests.ts
📚 Learning: 2025-05-14T03:33:25.089Z
Learnt from: 0xDEnYO
PR: lifinance/contracts#1139
File: package.json:81-82
Timestamp: 2025-05-14T03:33:25.089Z
Learning: Bun allows direct invocation of npm scripts defined in package.json without requiring the `bun run` prefix (e.g., `bun scriptName` works, not just `bun run scriptName`), unlike some other package managers like npm.
Applied to files:
package.jsonscript/runTypescriptTests.ts
📚 Learning: 2025-08-29T09:56:59.784Z
Learnt from: CR
PR: lifinance/contracts#0
File: conventions.md:0-0
Timestamp: 2025-08-29T09:56:59.784Z
Learning: Applies to script/demoScripts/**/*.ts : TypeScript demo scripts must follow .eslintrc.cjs, use async/await, try/catch error handling, proper logging, env via getEnvVar(), citty for CLI, consola for logging, and exit with correct codes
Applied to files:
script/runTypescriptTests.ts
🧬 Code graph analysis (1)
script/runTypescriptTests.ts (1)
script/troncast/postinstall-tronweb-fix.mjs (2)
items(44-44)fullPath(47-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (1)
package.json (1)
91-91: Good switch to bunx tsx for TS execution.This aligns with our convention for script/**/*.ts.
| @@ -0,0 +1,76 @@ | |||
| #!/usr/bin/env bun | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Execute this script with bunx tsx per repo convention.
Replace the Bun shebang and usage notes to match our standard.
-#!/usr/bin/env bun
+#!/usr/bin/env -S bunx tsx
@@
- * - npm run test:ts (runs all TypeScript tests)
- * - bun script/runTypescriptTests.ts (direct execution)
+ * - npm run test:ts (runs all TypeScript tests)
+ * - bunx tsx script/runTypescriptTests.ts (direct execution)Note: If env -S is unavailable in some environments, drop the shebang and rely on package.json’s script.
Also applies to: 16-18
🤖 Prompt for AI Agents
In script/runTypescriptTests.ts around lines 1 and 16-18, replace the current
Bun shebang and accompanying usage notes with the repo-standard invocation using
bunx tsx: either change the shebang/launcher to use the env wrapper that runs
"bunx tsx" per convention or, if env -S is not supported in target environments,
remove the shebang entirely and update the repository scripts (package.json) and
the file's usage notes to instruct running via "bunx tsx
script/runTypescriptTests.ts"; ensure the usage text clearly reflects the chosen
approach.
Comment on lines
+25
to
+27
| import { execSync } from 'child_process' | ||
| import { readdirSync, statSync } from 'fs' | ||
| import { join } from 'path' |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid shell quoting pitfalls; use spawnSync with args instead of execSync with a string.
Prevents issues if paths contain spaces/quotes and removes reliance on a shell.
-import { execSync } from 'child_process'
+import { spawnSync } from 'child_process'
@@
-// Run all test files together with a specific pattern
-const testFilePatterns = testFiles.map((file) => `"${file}"`).join(' ')
-console.log(`Running all tests with: bun test ${testFilePatterns}`)
-
-try {
- execSync(`bun test ${testFilePatterns}`, { stdio: 'inherit' })
-} catch (error) {
- console.error('Some tests failed')
- process.exit(1)
-}
+console.log('Running all tests with: bun test', testFiles.join(' '))
+const { status } = spawnSync('bun', ['test', ...testFiles], { stdio: 'inherit' })
+if (status !== 0) {
+ console.error('Some tests failed')
+ process.exit(status ?? 1)
+}Also applies to: 65-74
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which Jira task belongs to this PR?
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)