DimitriGilbert / DimitriGilbert/git-moar

Test & quality alignment plan

Offen
#2 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
JavaScript
Sterne
1
Forks
0
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

Test & Quality Alignment Plan — git-snitch

Revised 2026-09-05: CI removed by owner decision — quality gates are local commands run by the owner/agents; no GitHub Actions.


1. Current state snapshot (grounded in the repo)

  • 12 source files, 5,864 LOC: bin/cli.js (66) + 11 modules in lib/ (largest: lib/charts.js 1,622; lib/templates.js 1,063; lib/scattered.js 582).
  • Runtime deps: chalk ^4.1.2, sloc ^0.3.2. Zero devDependencies.
  • "test": "echo \"No tests yet\" && exit 0" (package.json line 11) — fake-green runner. Zero test files, zero CI (no .github/), zero hooks, no lint/format config.
  • engines.node >=14.0.0; local Node is v24.20.0. package-lock.json present (npm; no pnpm/bun artifacts).
  • Git access is hard-coupled to child_process.execSync at 6 invocation sites (the audit's "~10" counts these plus their require('child_process') destructures):
    • lib/utils.js:66git config --get remote.origin.url (getRepoInfo)
    • lib/utils.js:208git branch -a --contains <hash> (getCommitBranchInfo)
    • lib/snitch.js:309git log --pretty=format… --numstat (snitch run)
    • lib/snitch.js:364git config --get remote.origin.url (snitch run)
    • lib/scattered.js:486git log … with cwd: repoPath (scattered run)
    • lib/templates.js:1048xdg-open/open/start (openInBrowser — not git)
  • Pure, dependency-free logic already exported and unit-testable without any refactor: parseArgs (lib/snitch.js), all of lib/commit-classifier.js, lib/quality-metrics.js, lib/productivity.js, lib/hotspots.js, and in lib/utils.js: parsePeriodToGitSince, parseGitLog, calculateTimingStats, generateUserStats, generateProjectStats, createCommitUrl, createBranchUrl.
  • HTML generation (templates.js + charts.js = 46% of LOC) is string templating over a reportData object — ideal for golden-file tests.
  • .npmignore already excludes *.test.js and test/, so colocated tests will not ship in the package. .gitignore only has node_modules and *.tgz.
  • Known hygiene defects: package name/bin/URLs say git-moar, dir/remote say git-snitch, sibling repo /home/didi/workspace/gitsnitch holds the reverse; "author": "Your Name" scaffold placeholder.

2. Adopted decisions (synthesis D1–D7, translated to plain-CJS)

Decision Applies? Translation for this repo
D1 — runner & layout Yes, with the repo-specific ruling Synthesis explicitly assigns node:test + node --test to git-snitch (zero devDeps fits the repo's philosophy). Explicit deviation from the fleet-default vitest, per the synthesis itself — vitest would be the only test dep in a zero-devDep CJS repo for no benefit at this scale. Layout: tests colocated as lib/*.test.js (per D1 colocation rule; .npmignore already excludes them) plus a test/ dir for integration/golden fixtures. Root-level headless one-command entry: npm test. Requires engines.node bump >=14>=18 (D1 sanctions this; package is unpublished at v0.0.1).
D2 — coverage Yes, state 3 Zero-test repo → coverage reported, no thresholds until seeds exist, then ratchet. Analogy for the vitest/v8 provider: node --test --experimental-test-coverage (report-only; keeps zero-devDep style; c8 only if the flag proves insufficient). Baseline recorded in AGENTS.md once measured. No threshold ever lowered.
D3 — lint & format Yes, JS analogy ESLint 9 flat config (eslint.config.js, CommonJS module.exports form) with @eslint/js recommended + node globals. typescript-eslint / recommendedTypeChecked / verbatimModuleSyntax are N/A (no TS). Prettier: deferred in v1, stated in AGENTS.md rather than pretended (D3 allows this). Gates: zero errors.
D4 — quality metrics Partial, report-only knip is nominated for publishing repos "later git-snitch" — adopt as a report-only script here (export-surface drift is real: 11 module.exports blocks). jscpd/CRAP: not in this repo's v1 — they arrive later as local report scripts when needed; sequencing coverage (D2) before CRAP is respected.
D5 — no CI by design (owner decision 2026-09-05) Yes No GitHub Actions, no reusable-workflow caller, no local workflow fallback — the earlier "thin caller of DimitriGilbert/.github (with a minimal local-workflow fallback)" plan is dropped entirely. Enforcement = the root-level headless verification entrypoint (npm run verify: lint → test, composed in Phase 5) run by the owner and by agents; the plan's phase validation gates are run by implementer/validator agents before any phase is declared done. Gates in v1: lint + tests; typecheck N/A. Coverage stays a report-only on-demand local command (D2 state 3).
D6 — shared config distribution Analogy only @dg/config ships TS consumables (tsconfigs, vitest factory) — N/A for a plain-CJS repo. Analogy adopted: keep this repo's configs local and minimal (eslint.config.js, renovate.json), converge later if a JS ESLint preset is published to npm. No config-churn PR for its own sake (D6 rule).
D7 — hooks Yes No hooks. The local gate commands run by the owner/agents are the only authoritative gate; AGENTS.md will state it. A repo with no hooks and a green local npm run verify is compliant.

3. Synthesis Definition of Done — checklist with current status

# DoD item Current status Target
1 Root-level headless test command, documented in AGENTS.md Missing — fake echo script; no AGENTS.md npm test = node --test; documented (Phase 1, 6)
2 Headless verification entrypoint running typecheck + test + lint in one local command; agent records green before a phase is done None (no aggregate command) npm run verify; typecheck N/A (JS) — lint + tests are the static/dynamic gates (Phase 5)
3 Coverage measured & reported; baseline in AGENTS.md; thresholds only per D2 state None D2 state 3: report-only via node --test --experimental-test-coverage, baseline recorded, no thresholds (Phase 5–6)
4 Language-appropriate lint, zero errors, wired into the headless entrypoint; dead lint task implemented or deleted None ESLint 9 flat, zero errors, in npm run verify; dead lint task N/A (no turbo/makefile) (Phase 4, 5)
5 AGENTS.md "Testing" section matches reality No AGENTS.md Created in Phase 6 after infra exists — no phantom docs
6 Dead scaffold test deps removed or used N/A — zero devDeps, no scaffold; principle restated: devDeps stay minimal (ESLint toolchain only) Hold the line
7 Renovate one-liner; no "latest" deps No Renovate; no "latest" pins (chalk/sloc are normal ranges) renovate.json extending local>DimitriGilbert (Phase 6)
8 Quality-report script (knip + jscpd, CRAP where coverage) runnable locally, report-only None knip report-only locally (Phase 6)
9 Nothing satisfied by a fake pass Violated todayecho "No tests yet" && exit 0 is exactly the banned pattern Deleted in Phase 1; validators enforce no tautological assertions fleet-wide

4. Phased plan

Execution model per the subagent-orchestration skill: each phase (or sub-phase) is executed by an implementer subagent that runs the gatekeeping commands before reporting done, then validated by a different validator subagent that must actually read the code and enforce NO-SLOP; failures go to a fixer (all errors at once) and re-validation, max 3 attempts. Phases are sequential unless marked parallelizable.

NO-SLOP policy for this repo (JS adaptation): no TODO/FIXME/placeholder code; no unused imports/variables (ESLint will enforce this from Phase 4 onward); no console.log hacks to suppress errors (note: console.log is this CLI's legitimate output channel — the ban applies to error suppression, not product output); no fake or tautological assertions (every test asserts real behavior of a real import from lib/); deterministic tests (fixed GIT_AUTHOR_DATE/GIT_COMMITTER_DATE, fs.mkdtemp tmpdirs, no network); no package-manager switches; no hand-edited lockfile. Gatekeeping commands (this repo has no typecheck/build — the skill's check-types mandate maps to these): npm test, and npm run lint once Phase 4 lands.

Phase 0 — Preflight hygiene (effort: S)

Goal: repo is ready to receive a test runner without carrying known blockers.

Tasks (implementer):

  • package.json: engines.node">=18.0.0" (D1 prerequisite for node:test).
  • package.json: author"Dimitri Gilbert" (audit P3 scaffold leftover; safe, unrelated to the naming-swap decision).
  • .gitignore: add coverage/ and test scratch dirs if any are introduced later.
  • .npmignore: add coverage/ (once .npmignore exists it overrides .gitignore for packing, so new dev artifacts must be listed explicitly).
  • Sanity: npm ci (or npm install) completes; node bin/cli.js prints help; node --version ≥ 18 confirmed.

Validation gate (validator): npm ci clean; node bin/cli.js exits 0 printing usage; engines/author correct in package.json; no other files touched (git status).

Phase 1 — Real test runner + pure-logic seed suite (effort: M)

Goal: the fake-green script is dead; npm test runs real assertions over the ~1,300 LOC of pure logic. Per D1's named seed targets.

Tasks (implementer — parallelizable as 2 sub-phases per the skill: (a) runner + parseArgs/utils tests, (b) classifier/metrics/productivity/hotspots tests; each with its own validator, then a phase-wide validator):

  • package.json: "test": "node --test lib/ test/" (explicit paths, no default-glob surprises); the echo script is deleted in the same commit — the repo is never left without a working test script.
  • Colocated unit tests (.npmignore already keeps them out of the package):
    • lib/snitch.test.jsparseArgs: every flag/short form, defaults, invalid --sort-by/--sort-order fallback to defaults (assert the returned object, not the console warning), --all-branches + --branch conflict resolution, unknown-option tolerance.
    • lib/utils.test.jsparsePeriodToGitSince (all period formats), parseGitLog against a committed fixture string of real git log --pretty=format:"%H|%an|%ae|%ad|%s" --numstat output (edge cases: renames, binary lines, empty diff), calculateTimingStats, generateUserStats, generateProjectStats, createCommitUrl/createBranchUrl for https/SSH/enterprise remotes.
    • lib/commit-classifier.test.jsclassifyCommit across conventional-commit/fix/feature/docs types, generateCommitTypeBreakdown, extractIssueReferences (with and without config), analyzeCommitMessageQuality.
    • lib/quality-metrics.test.js — known-value math: calculateChurnRate, calculateBusFactor (single-author vs distributed), calculateGiniCoefficient (perfect equality = 0, concentration → 1), calculateAvgCommitSize, calculateCodeStability, calculateHealthScore + getHealthScoreRating boundaries, calculateTechnicalDebt.
    • lib/productivity.test.jsfindPeakHours/findPeakDays on fixed-date fixtures, calculateVelocity, analyzeDevelopmentRhythm, calculateCollaborationScore, calculateFocusTime, calculateTimeToValue.
    • lib/hotspots.test.jsfindFileHotspots, calculateRiskLevel matrix, findKnowledgeSilos, analyzeFileTypeVolatility, calculateHotspotStats.
    • lib/config.test.js — the exported singleton's load/save/get/set behavior against a tmpdir (it writes config files — keep it hermetic).
  • Tests import only real modules; no mocks needed at this phase (that is the point of seeding pure logic first).

Validation gate (validator — reads every test file): npm test exits 0 with a real node:test summary (> 0 suites, target ≥ 60 test cases across the 7 files); zero assertions on literals the test itself constructs (tautology check); every test file imports the production module under test; fake script gone (grep '"test"' package.json).

Phase 2 — execSync seam + integration tests over real temp repos (effort: L)

Goal: git invocation moves behind one injectable seam; the two run() pipelines get integration coverage against deterministic throwaway git repos.

Sub-phase 2a — seam refactor (implementer → validator):

  • New lib/git-runner.js: a single runGit(command, options) thin wrapper over execSync (encoding utf8, stdio: 'pipe', optional cwd), plus a documented injection hook: call sites accept an optional runner parameter (or module-level override) so tests can substitute a fake without touching child_process.
  • Behavior-preserving rewires — the 5 git sites: lib/utils.js:66, lib/utils.js:208, lib/snitch.js:309, lib/snitch.js:364, lib/scattered.js:486. No logic changes, no formatting churn. Note for the implementer: lib/snitch.js line 377 does a lazy require('./utils') inside run() — hoisting it to the top-level requires is acceptable if behavior-identical.
  • lib/templates.js:1048 (openInBrowser) stays out of the git seam — P3; optionally give it its own openInBrowser injection later.
  • Phase-2a gate: npm test still green (seeds must not move); grep -n "execSync" lib/ bin/ returns hits only in lib/git-runner.js (+ lib/templates.js openInBrowser); node bin/cli.js help still works.

Sub-phase 2b — integration tests (implementer → validator, then phase-wide validator over 2a+2b):

  • test/helpers/git-fixture.js: creates a temp repo via fs.mkdtemp + git init, sets local user.name/user.email, makes N deterministic commits using fixed GIT_AUTHOR_DATE/GIT_COMMITTER_DATE, files with known line counts (so --numstat math is assertable), an optional second branch and a remote.origin.url.
  • test/snitch.run.test.js: run the snitch pipeline end-to-end in the fixture repo with --output into the tmpdir (never opening a browser) → assert the HTML file is created, is non-trivial, contains known commit hashes/authors/stats.
  • test/scattered.run.test.js: same over 2–3 fixture repos → assert comparison output written.
  • test/git-runner.test.js: fake-runner injection proves the seam (command strings passed through, cwd honored, non-zero exits surface as the call sites expect).
  • Tests skip gracefully with a clear message only if git itself is absent from PATH (CI images have git; local robustness, not a fake pass).

Validation gate (validator — reads the diff and the harness): npm test green including integration suite; injected-fake test proves call sites go through the seam; fixture repos are created under os.tmpdir() and cleaned up; no test writes into the repo tree; total runtime reasonable (< ~30s).

Phase 3 — Golden-file tests for HTML generation (effort: M)

Goal: pin the 2,685 LOC of templates.js + charts.js (46% of the codebase) behind committed golden outputs. Parallelizable with Phase 2 (touches disjoint files; golden fixtures use reportData objects directly, no git needed).

Tasks (implementer):

  • test/golden/reportData.fixture.js: one representative reportData (commits, users, locDetails, options) exercising most chart generators.
  • test/templates.golden.test.js / test/charts.golden.test.js: render each exported generator; concatenate generateHTMLDocument output; compare against committed goldens in test/golden/.
  • Determinism pass (required): audit the rendered HTML for embedded new Date()/timestamps/locale-dependent formatting; normalize (inject a fixed clock argument where the function accepts one, or canonicalize known-volatile substrings before compare) so goldens are stable across machines. Any injected-clock parameter must default to the current behavior.
  • Regeneration flow: UPDATE_GOLDEN=1 npm test rewrites goldens; documented in AGENTS.md (Phase 6) with the rule: goldens change only in a commit whose diff shows the intended output change.
  • Keep goldens readable: one file per generator group rather than one monolith, so diffs localize.

Validation gate (validator): npm test green including goldens; deliberately mutate one generator string locally → golden test fails (mutation check), revert → passes; UPDATE_GOLDEN=1 round-trip works; two consecutive runs byte-identical (determinism check).

Phase 4 — ESLint flat config (effort: S)

Goal: lint installed, zero errors, wired as npm run lint (the JS analogue of the fleet's check-types gate).

Tasks (implementer):

  • devDependencies (minimal, npm): eslint@^9, @eslint/js, globals — nothing else.
  • eslint.config.js (CommonJS): @eslint/js recommended; languageOptions.ecmaVersion: "latest", sourceType: "commonjs", node globals; ignores: node_modules/, coverage/, test/golden/, package-lock.json.
  • package.json: "lint": "eslint ."; "test" stays the headless gate.
  • Fix every reported error at the cause — expected finds in a legacy codebase: no-case-declarations in parseArgs's switch (fix: braces around the case blocks), unused variables/imports (fix: remove), no-unused-vars on caught error params in intentional swallow paths (fix: rename to error with caughtErrors handled per rule config documented in the config file — choose the smallest config that reflects intent, never blanket-disable the rule).
  • Policy: no rule silently disabled. Any downgrade needs a comment in eslint.config.js stating why, scoped as narrowly as possible.

Validation gate (validator — reads the config and each fix diff): npm run lint exits 0 with zero errors and zero suppress-happy config; npm test still green (fixes must not change behavior); spot-check that behavior-changing "fixes" would have been caught (any fix that alters runtime behavior must be flagged, reverted, and re-done as a true no-op or accompanied by a new test).

Phase 5 — Local gate composition (effort: S)

Goal: one headless local command gates lint + tests — the fleet's single biggest gap (audit P1), realized locally per D5.

Tasks (implementer):

  • package.json: "verify": "npm run lint && npm test" — the headless verification entrypoint (D5).
  • Coverage stays a report-only on-demand local command: node --test --experimental-test-coverage documented in AGENTS.md (Phase 6), not wired into verify by default (keeps the fast path fast; D2 state 3).
  • No workflow file is created anywhere; nothing depends on DimitriGilbert/.github (D5).

Validation gate (validator): npm run verify exits 0 locally and both sub-commands exist in package.json scripts; fail-fast proven once (validator introduces a failing assertion in a scratch copy and confirms verify exits non-zero at the failing step); the implementer/validator agents record the green output in the phase report before the phase is declared done (D5); no workflow file exists.

Phase 6 — Renovate, knip (report-only), AGENTS.md (effort: S)

Goal: DoD items 1, 5, 7, 8 closed; docs match reality; the repo is left maintainable.

Tasks (implementer):

  • renovate.json: {"$schema": "https://docs.renovatebot.com/renovate-schema.json", "extends": ["local>DimitriGilbert"]} (D4 org-preset one-liner). Confirms no "latest" deps (none exist today).
  • knip report-only: devDep knip + "quality:knip": "knip" with a minimal knip config (entry: bin/cli.js, project: lib/**/*.js, test files ignored as entry-adjacent); report-only — never a gate in v1 (D4: nothing gated on an unmeasured number). Surface the first report's findings as information, not fixes.
  • AGENTS.md (new): Testing section — runner (node --test), command (npm test, npm run lint, npm run verify), layout (colocated lib/*.test.js + test/), integration/golden strategies, UPDATE_GOLDEN=1 flow, coverage status + measured baseline (D2), knip report-only; gate section — "the headless local verification command (npm run verify) is the quality gate, run by the owner/agents; hooks are local convenience" (D7), no hooks installed; the engines floor (Node ≥ 18); a pointer that AGENTS.md conventions here are this repo's own (not BTS-derived).
  • Verify .npmignore still keeps tests, goldens, coverage out of the packed tarball (npm pack --dry-run inspection).

Validation gate (validator — final full gate): npm ci && npm run lint && npm test all green; renovate.json parses and matches the preset convention; AGENTS.md audited claim-by-claim against the repo (no phantom docs — DoD 5); npm pack --dry-run contains no *.test.js, test/, or coverage/.


5. Risks & notes specific to this repo

  • execSync seam strategy: the seam is deliberately tiny — one runGit() wrapper plus an injection parameter; tests use (a) fake runners for unit-level seam proofs and (b) real git in fs.mkdtemp repos with pinned dates for integration. Mocking git output strings wholesale is rejected: it would freeze today's command construction (string-concatenated git log …) into the mocks; the integration fixtures catch real command regressions instead. Shell-escaping of user-supplied options (--branch, --start-date interpolated into command strings, e.g. snitch.js:298-307) is worth a dedicated look once the seam exists — options flow into execSync strings today.
  • Naming swap — FLAGGED, NOT EXECUTED: package.json name/bin/repository.url/bugs/homepage say git-moar; the directory and git remote say git-snitch; sibling repo /home/didi/workspace/gitsnitch packages itself as git-snitch (a TS/BTS CLI). This is a pre-existing owner-level hygiene decision (synthesis §5); no phase of this plan renames anything, and no publishing step may proceed before the collision is resolved. The only metadata this plan touches is author (Phase 0) and engines (Phase 0).
  • Lazy require('./utils') inside lib/snitch.js run() (line 377) — keep or hoist during Phase 2a, but do not let the refactor silently change module-load side effects (lib/config.js exports a singleton that may read/write config at require time; config.test.js must cover that).
  • Golden determinism: HTML output may embed dates/locale-formatted numbers; the determinism pass in Phase 3 is load-bearing — a flaky golden suite would be worse than none.
  • No shared-workflow dependency: under D5 there is none — the gate is the local npm run verify and nothing depends on DimitriGilbert/.github.
  • Dependency aging (P3, out of scope): sloc ^0.3.2 is effectively unmaintained and chalk is pinned to CJS-era v4. Neither is swapped in this effort (synthesis non-goal: don't churn what works); Renovate (Phase 6) will surface them. getLinesOfCode already defends against sloc failures.
  • Fake-pass ban is the headline: DoD 9 is violated by the repo's current test script; Phase 1 removes it in the same change that introduces the real runner so npm test is never absent and never fake.

6. Non-goals

  • No TypeScript migration, no bundler, no package-manager switch (npm stays), no monorepo conversion.
  • No renaming of package/dir/bin (owner decision, see risk above); no npm publish.
  • No Prettier in v1 (deferred and stated in AGENTS.md, per D3).
  • No coverage thresholds, jscpd gates, or CRAP gates (D2 state 3; D4 report-only).
  • No git hooks (D7).
  • No replacement of chalk/sloc.

7. Effort summary

Phase Goal Effort Role flow
0 Preflight hygiene S implementer → validator
1 Real runner + pure-logic seeds M 1–2 parallel implementers → per-slice validators → phase-wide validator
2 execSync seam + integration tests L 2a implementer → validator; 2b implementer → validator → phase-wide validator
3 Golden files for templates/charts M implementer → validator (parallelizable with 2)
4 ESLint flat config S implementer → validator
5 Local gate composition S implementer → validator
6 Renovate + knip + AGENTS.md S implementer → validator (final full gate)

Total: ~3–4 focused sessions. The repo ends at full DoD compliance modulo the flagged owner decision (naming swap).

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne mit package.json und der Checkliste für Phase 0–1; führe npm ci aus, bestätige, dass Node mindestens Version 18 hat, und prüfe die genannten Module mit reiner Logik sowie die geplanten Testdateien. Fahre mit den aufgeführten Phasen fort und verwende npm test und npm run verify als Prüfungen. Als abgeschlossen gilt die Aufgabe, wenn eine echte Abdeckung mit node:test, Linting und lokale Verifizierungsbefehle sowie die dokumentierte Qualitätsbaseline ohne CI eingerichtet sind.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
eslint, git, javascript, node.js
Bereich
developer-experience, testing-qa, tooling
Issue-Typ
Feature
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Aktiv
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
32/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.