Skip to content

test(report): make color helpers testable and add colour-output tests - #7

Merged
Truta446 merged 1 commit into
Truta446:mainfrom
blut-agent:main
Aug 10, 2026
Merged

test(report): make color helpers testable and add colour-output tests#7
Truta446 merged 1 commit into
Truta446:mainfrom
blut-agent:main

Conversation

@blut-agent

@blut-agent blut-agent commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

src/report.ts has the colour-check logic as a module-level constant computed once at import time:

const useColor =
  process.env["NO_COLOR"] === undefined &&
  process.env["TERM"] !== "dumb" &&
  process.stderr.isTTY === true;

This makes it impossible to test the coloured code paths by mutating process.env.NO_COLOR, process.env.TERM, or process.stderr.isTTY inside a test — the value is already frozen. Current branch coverage on report.ts is only ~62%, and the coloured path is entirely untested.

Fix

Refactor: Convert useColor from a module-level const into a function that re-reads the environment on every call. This is trivially testable — each test can mutate env / stub isTTY and the functions see the new values.

The refactoring itself is minimal (move the boolean expression into useColor() and update the five c.* helpers to call it). A no-op _resetColorState() export is also added as a coordination hook for future tests that may need it.

Test Plan

  • All 120 existing tests pass (npm test)
  • New test/report.test.ts — 6 tests covering:
    • NO_COLOR set → no escape codes
    • TERM=dumb → no escape codes
    • isTTY=false (CI case) → no escape codes
    • TTY with no restrictions → codes present
    • Both n_plus_one and duplicate findings with/without colours
    • Multi-finding summary with and without colours
  • Branch coverage on report.js improved from ~62% to ~97%

Testing

npm install
npm test        # 120 tests, 0 failures
npm run coverage # report.js: 96.77% branch coverage (up from ~62%)

Closes #4

Truta446 added a commit that referenced this pull request Aug 10, 2026
@Truta446
Merge PR #7: make colour helpers testable and cover them
00857fd 
Turns the module-level `useColor` constant into a function that re-reads the
environment, which is what made the coloured paths testable at all. Branch
coverage on report.ts goes from ~62% to ~97%: NO_COLOR, TERM=dumb, non-TTY (the
CI case) and TTY are all exercised, for both finding types and for summaries.

Verified locally against current main: 130 tests pass and no other file's
coverage regressed.

Closes #4
@Truta446
Truta446 merged commit 5e795ba into Truta446:main Aug 10, 2026
@Truta446

Truta446 commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Merged into main — thank you, this was a good contribution.

I validated it locally against current main rather than just reading the diff:

  • 130 tests pass (the suite grew since you opened this — autoScope landed in 0.5.0)
  • report.js branch coverage: 62% → 96.7%, which clears the 90% the issue asked for
  • No other file's coverage regressed. I checked config.js at 32% specifically, and confirmed it reads the same on main without your changes — it is an artefact of global-state.test.ts importing module copies via query string, which the coverage tool counts as separate files. Not something you introduced.

Two adjustments I made on top, both explained in the merge commit:

ScopeSummary gained an inferred field after you opened this, so makeSummary no longer compiled. A semantic conflict rather than a textual one, which is why GitHub showed no conflict and only tsc caught it. Added inferred: false.

Removed _resetColorState(). The reasoning in the comment is sound — a hook for a future cached variant — but as it stands it is an exported no-op in the public API, and the tests pass without it. Easier to add it back alongside the cache that would justify it than to carry a function that does nothing. No criticism intended; it was a defensible call, I just landed on the other side of it.

The refactor itself is exactly right. Making useColor re-read the environment is what made the paths testable at all, and it is the kind of change that looks trivial but is the whole reason the tests could exist.

Released in v0.5.1.

@Truta446 Truta446 mentioned this pull request Aug 13, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cover the colour-output logic in report.ts with tests

2 participants

@blut-agent @Truta446