Conversation
Add 15 test cases covering all exported error code constants, type verification, value correctness, usage as message prefixes, and uniqueness guarantees. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Vitest coverage for the standardized error code constants used by safe-outputs handlers.
Changes:
- Added a new Vitest test suite covering
actions/setup/js/error_codes.cjsexports, expected string values, and basic usage/uniqueness assertions.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/error_codes.test.cjs | Adds unit tests validating the error code constants’ exports, values, and uniqueness. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
| it("exports string values only", () => { | ||
| const codes = [ERR_VALIDATION, ERR_PERMISSION, ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_SYSTEM, SAFE_OUTPUT_E001, SAFE_OUTPUT_E099]; | ||
| for (const code of codes) { | ||
| expect(typeof code).toBe("string"); | ||
| } |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd based on the test-only nature of this PR.
Key Themes
- Tautological value assertions: Tests like
expect(ERR_VALIDATION).toBe("ERR_VALIDATION")verify that a constant equals a string matching its own name. These are low-signal — they pass trivially and would still pass if both the constant name and its value were renamed together. - Mechanics tests: The "can be used as a prefix" tests verify JavaScript template literal interpolation, not module behaviour. They don't surface real failure modes.
- No negative/boundary tests: There is no test that the module does not export unexpected extras, or that the exported set is complete and stable.
Positive Highlights
- ✅ Excellent
describegrouping — the test structure reads as a clear specification. - ✅ Uniqueness tests (
Set.size === array.length) are genuinely valuable — they would catch accidental duplicate values. - ✅ Comprehensive export coverage: all 9 exports verified to exist and be strings.
- ✅ Clean separation between primary codes and legacy
E00xcodes.
Suggestion
The individual value assertions could be consolidated into a data-driven table test to reduce repetition and make future additions a one-liner:
it.each([
["ERR_VALIDATION", ERR_VALIDATION],
["ERR_PERMISSION", ERR_PERMISSION],
["ERR_API", ERR_API],
// ...
])("%s has the correct string value", (expected, actual) => {
expect(actual).toBe(expected);
});No blocking issues — commenting only.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 2.1M
🧪 Test Quality Sentinel ReportTest Quality Score: 55/100🔶 Needs Improvement — score bounded by module nature (pure constants; no error paths exist to test)
Test Classification DetailsView all 15 tests
Suggestions (Non-blocking)Minor: Duplicate cluster in
|
| Component | Points | Notes |
|---|---|---|
| Behavioral coverage (40 pts) | 40 | 15/15 design tests (100%) |
| Error/edge case coverage (30 pts) | 0 | No error paths in a constants module |
| Low duplication (20 pts) | 15 | 1 cluster: 7 near-identical per-constant tests |
| Proportional growth (10 pts) | 0 | 90 test lines added, 0 production lines changed |
| Total | 55 |
i️ The low score reflects structural artefacts of this module (constants-only, no error paths, test-retrofit). The 0% implementation-test rate confirms all tests enforce genuine behavioral contracts.
Language Support
- 🟨 JavaScript (
*.test.cjs): 15 tests (vitest) — no mocking - 🐹 Go (
*_test.go): 0 tests in this PR
Verdict
✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). No coding-guideline violations detected.
References: §25537761217
🧪 Test quality analysis by Test Quality Sentinel · ● 5.9M · ◷
There was a problem hiding this comment.
✅ Test Quality Sentinel: 55/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 15 tests enforce genuine behavioral contracts. The numeric score is bounded by the module's nature (pure constants; no error paths to test), not by test quality issues.
Summary
Adds comprehensive test coverage for
error_codes.cjs, the module that provides standardized error code constants used throughout the safe-outputs handlers.File Cleaned
actions/setup/js/error_codes.cjs— Node.js context (no GitHub Actions globals needed)The file itself was already clean (
@ts-checkpresent, well-typed, good JSDoc). The missing piece was test coverage.Changes
actions/setup/js/error_codes.test.cjswith 15 test casesTest Coverage Added
E001andE099valuesTotal: 15 tests added
Validation ✅
npx prettier --check error_codes.cjs error_codes.test.cjs✓npm run typecheck✓npx vitest run error_codes.test.cjs→ 15/15 passed ✓