Files
msd-core/CONTRIBUTING.md
Tom Boucher 616c1fa753 refactor: replace try/finally with beforeEach/afterEach + add CONTRIBUTING.md
Test suite modernization:
- Converted all try/finally cleanup patterns to beforeEach/afterEach hooks
  across 11 test files (core, copilot-install, config, workstream,
  milestone-summary, forensics, state, antigravity, profile-pipeline,
  workspace)
- Consolidated 40 inline mkdtempSync calls to use centralized helpers
- Added createTempDir() helper for bare temp directories
- Added optional prefix parameter to createTempProject/createTempGitProject
- Fixed config test HOME sandboxing (was reading global defaults.json)

New CONTRIBUTING.md:
- Test standards: hooks over try/finally, centralized helpers, HOME sandboxing
- Node 22/24 compatibility requirements with Node 26 forward-compat
- Code style, PR guidelines, security practices
- File structure overview

All 1382 tests pass, 0 failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 15:45:39 -04:00

197 lines
5.3 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Contributing to GSD
## Getting Started
```bash
# Clone the repo
git clone https://github.com/gsd-build/get-shit-done.git
cd get-shit-done
# Install dependencies
npm install
# Run tests
npm test
```
## Pull Request Guidelines
- **One concern per PR** — bug fixes, features, and refactors should be separate PRs
- **No drive-by formatting** — don't reformat code unrelated to your change
- **Link issues** — use `Fixes #123` or `Closes #123` in PR body for auto-close
- **CI must pass** — all matrix jobs (Ubuntu, macOS, Windows × Node 22, 24) must be green
## Testing Standards
All tests use Node.js built-in test runner (`node:test`) and assertion library (`node:assert`). **Do not use Jest, Mocha, Chai, or any external test framework.**
### Required Imports
```javascript
const { describe, it, test, beforeEach, afterEach, before, after } = require('node:test');
const assert = require('node:assert/strict');
```
### Setup and Cleanup: Use Hooks, Not try/finally
**Always use `beforeEach`/`afterEach` for setup and cleanup.** Do not use `try/finally` blocks for test cleanup — they are verbose, error-prone, and can mask test failures.
```javascript
// GOOD — hooks handle setup/cleanup
describe('my feature', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('does the thing', () => {
// test body focuses only on the assertion
assert.strictEqual(result, expected);
});
});
```
```javascript
// BAD — try/finally is verbose and masks failures
test('does the thing', () => {
const tmpDir = createTempProject();
try {
// test body
assert.strictEqual(result, expected);
} finally {
cleanup(tmpDir);
}
});
```
### Use Centralized Test Helpers
Import helpers from `tests/helpers.cjs` instead of inlining temp directory creation:
```javascript
const { createTempProject, createTempGitProject, createTempDir, cleanup, runGsdTools } = require('./helpers.cjs');
```
| Helper | Creates | Use When |
|--------|---------|----------|
| `createTempProject(prefix?)` | tmpDir with `.planning/phases/` | Testing GSD tools that need planning structure |
| `createTempGitProject(prefix?)` | Same + git init + initial commit | Testing git-dependent features |
| `createTempDir(prefix?)` | Bare temp directory | Testing features that don't need `.planning/` |
| `cleanup(tmpDir)` | Removes directory recursively | Always use in `afterEach` |
| `runGsdTools(args, cwd, env?)` | Executes gsd-tools.cjs | Testing CLI commands |
### Test Structure
```javascript
describe('featureName', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
// Additional setup specific to this suite
});
afterEach(() => {
cleanup(tmpDir);
});
test('handles normal case', () => {
// Arrange
// Act
// Assert
});
test('handles edge case', () => {
// ...
});
describe('sub-feature', () => {
// Nested describes can have their own hooks
beforeEach(() => {
// Additional setup for sub-feature
});
test('sub-feature works', () => {
// ...
});
});
});
```
### Node.js Version Compatibility
Tests must pass on:
- **Node 22** (LTS)
- **Node 24** (Current)
Forward-compatible with Node 26. Do not use:
- Deprecated APIs
- Version-specific features not available in Node 22
Safe to use:
- `node:test` — stable since Node 18, fully featured in 22+
- `describe`/`it`/`test` — all supported
- `beforeEach`/`afterEach`/`before`/`after` — all supported
- `t.plan()` — available since Node 22.2
- Snapshot testing — available since Node 22.3
### Assertions
Use `node:assert/strict` for strict equality by default:
```javascript
const assert = require('node:assert/strict');
assert.strictEqual(actual, expected); // ===
assert.deepStrictEqual(actual, expected); // deep ===
assert.ok(value); // truthy
assert.throws(() => { ... }, /pattern/); // throws
assert.rejects(async () => { ... }); // async throws
```
### Running Tests
```bash
# Run all tests
npm test
# Run a single test file
node --test tests/core.test.cjs
# Run with coverage
npm run test:coverage
```
## Code Style
- **CommonJS** (`.cjs`) — the project uses `require()`, not ESM `import`
- **No external dependencies in core** — `gsd-tools.cjs` and all lib files use only Node.js built-ins
- **Conventional commits** — `feat:`, `fix:`, `docs:`, `refactor:`, `test:`, `ci:`
## File Structure
```
bin/install.js — Installer (multi-runtime)
get-shit-done/
bin/lib/ — Core library modules (.cjs)
workflows/ — Workflow definitions (.md)
references/ — Reference documentation (.md)
templates/ — File templates
agents/ — Agent definitions (.md)
commands/gsd/ — Slash command definitions (.md)
tests/ — Test files (.test.cjs)
helpers.cjs — Shared test utilities
docs/ — User-facing documentation
```
## Security
- **Path validation** — use `validatePath()` from `security.cjs` for any user-provided paths
- **No shell injection** — use `execFileSync` (array args) over `execSync` (string interpolation)
- **No `${{ }}` in GitHub Actions `run:` blocks** — bind to `env:` mappings first