-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Adds better debugging support #229
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds comprehensive debugging support for bin-tester, enabling developers to debug the bins they're testing with Node's inspector. Key features include zero-config debugging via environment variables, replay functionality for re-running previous invocations, and a CLI tool for enhanced workflow.
- Zero-config debugging using
BIN_TESTER_DEBUGenvironment variable to attach inspector to child processes - Replay system that persists execution artifacts and allows re-running with debugging capabilities
- New CLI tool for replaying and inspecting previous test runs
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/index-test.ts | Adds comprehensive test coverage for debugging features, replay functionality, and environment variable behaviors |
| tests/fixtures/print-exec-argv.js | Test fixture that outputs process.execArgv for validating inspector flags |
| src/replay.ts | Core replay functionality for reading and re-executing persisted run artifacts |
| src/create-bin-tester.ts | Enhanced runBin with debug support, artifact persistence, and new runBinDebug method |
| src/cli.ts | New CLI tool for replay and info commands with inspector options |
| package.json | Updated build config, added CLI binary, new dependency, and Node version requirement |
| eslint.config.js | Disabled unicorn rule for top-level await |
| README.md | Comprehensive documentation for debugging and replay features |
| CHANGELOG.md | Cleaned up duplicate changelog entries |
| .github/workflows/ci-build.yml | Updated CI to test only Node 22 and 24 |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| timestamp: new Date().toISOString(), | ||
| } as const; | ||
| writeFileSync(artifactPath, JSON.stringify(artifact, undefined, 2)); | ||
| console.log(`Replay with: bin-tester replay '${artifactPath}'`); |
Copilot
AI
Sep 17, 2025
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.
Console.log should not be used in library code as it creates unwanted output during tests and library usage. Consider using a debug utility or making this logging configurable.
| const previous = process.env.BIN_TESTER_DEBUG; | ||
| if (!previous) { | ||
| process.env.BIN_TESTER_DEBUG = 'attach'; | ||
| } | ||
| const result = runBin(...args); | ||
| if (!previous) { | ||
| delete process.env.BIN_TESTER_DEBUG; | ||
| } | ||
| return result; |
Copilot
AI
Sep 17, 2025
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.
Modifying process.env in runBinDebug creates a race condition in concurrent test environments. The environment variable could be modified by another test between setting and unsetting it.
| const previous = process.env.BIN_TESTER_DEBUG; | |
| if (!previous) { | |
| process.env.BIN_TESTER_DEBUG = 'attach'; | |
| } | |
| const result = runBin(...args); | |
| if (!previous) { | |
| delete process.env.BIN_TESTER_DEBUG; | |
| } | |
| return result; | |
| // Find if the last argument is an execa.Options object | |
| let options: execa.Options<string> | undefined; | |
| if (args.length > 0 && typeof args[args.length - 1] === 'object' && !Array.isArray(args[args.length - 1])) { | |
| options = args[args.length - 1] as execa.Options<string>; | |
| } | |
| // Clone the options and its env, or create new if not present | |
| const newOptions: execa.Options<string> = { | |
| ...(options || {}), | |
| env: { | |
| ...(options && options.env ? options.env : process.env), | |
| BIN_TESTER_DEBUG: 'attach', | |
| }, | |
| }; | |
| let newArgs: RunBinArgs; | |
| if (options) { | |
| // Replace the last argument with the new options | |
| newArgs = [...args.slice(0, -1), newOptions] as RunBinArgs; | |
| } else { | |
| // Add the new options as the last argument | |
| newArgs = [...args, newOptions] as RunBinArgs; | |
| } | |
| return runBin(...newArgs); |
| }, | ||
| "engines": { | ||
| "node": ">=18" | ||
| "node": ">=22" |
Copilot
AI
Sep 17, 2025
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.
Requiring Node >=22 is a breaking change that should be documented more prominently. Many projects still use Node 18 or 20, and this requirement significantly limits adoption.
| "node": ">=22" | |
| "node": "^18.0.0 || ^20.0.0 || >=22.0.0" |
Better debugging for bin-tester: zero-config inspector + replay
How to try it
Then attach your debugger:
VS Code: “Attach to Node Process”
DevTools: chrome://inspect → “Open dedicated DevTools for Node”
Break on the first line
BIN_TESTER_DEBUG=1 pnpm test:watch # or BIN_TESTER_DEBUG=true pnpm test:watchOne-off in test code
Replay the last run (with debugging)
After tests run, you’ll see:
Run:
Notes
--stdio inheritfor replay).