[jsweep] Clean expired_entity_cleanup_helpers.cjs

[github-actions]

Summary

Cleans and adds comprehensive test coverage for expired_entity_cleanup_helpers.cjs.

Context type

GitHub Actions (github-script) context — uses core global injected by the actions runtime (or shim.cjs).

Changes

expired_entity_cleanup_helpers.cjs

• Fix /// <reference> triple-slash directive: The file had // <reference types="@actions/github-script" /> (two forward slashes), which is a plain comment and does not work as a TypeScript triple-slash directive for type resolution. Fixed to /// <reference ...> (three slashes).
• Simplify redundant slice conditional: buildNotExpiredSection had a notExpiredEntities.length > 10 ? notExpiredEntities.slice(0, 10) : notExpiredEntities guard that is redundant — Array.slice is safe when the array is shorter than the requested count. Simplified to notExpiredEntities.slice(0, 10) directly.

expired_entity_cleanup_helpers.test.cjs (new file)

Added 26 test cases covering all exported functions:

Function	Tests
delay	2
validateCreationDate	5
categorizeByExpiration	6
processExpiredEntities	5
buildExpirationSummary	6
constants	2

Validation

• Formatting: npm run format:cjs ✅
• Linting: npm run lint:cjs ✅
• Type checking: npm run typecheck ✅
• Tests: all 26 tests pass ✅

Generated by jsweep - JavaScript Unbloater · ● 2.8M · ◷

• expires on May 7, 2026, 4:53 AM UTC

[Copilot]

Pull request overview

This PR cleans up expired_entity_cleanup_helpers.cjs and adds a new Vitest suite for its exported helpers.

Changes:

• Fixed the @actions/github-script triple-slash type reference in expired_entity_cleanup_helpers.cjs.
• Simplified the buildNotExpiredSection truncation logic by always using slice(0, 10).
• Added a new expired_entity_cleanup_helpers.test.cjs suite covering the exported helpers and constants.

Show a summary per file

File	Description
actions/setup/js/expired_entity_cleanup_helpers.cjs	Small cleanup to type-reference metadata and not-expired summary list truncation.
actions/setup/js/expired_entity_cleanup_helpers.test.cjs	New Vitest coverage for delay, date validation, expiration categorization, processing, summaries, and constants.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

actions/setup/js/expired_entity_cleanup_helpers.test.cjs:145

• Because the mock always resolves to the same record and this test only checks array lengths, it would still pass if processExpiredEntities accidentally reused the first result for every entity or otherwise lost per-entity data. Returning distinct records per call would verify that each loop iteration preserves the individual processEntity result.

 const processEntity = vi.fn().mockResolvedValue({ status: "closed", record: { number: 1 } });
const { closed, skipped, failed } = await processExpiredEntities(entities, {
entityLabel: "Issue",
delayMs: 0,
processEntity,
});
expect(closed).toHaveLength(2);
expect(skipped).toHaveLength(0);
expect(failed).toHaveLength(0);
});

• Files reviewed: 2/2 changed files
• Comments generated: 2

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 61/100

⚠️Acceptable — with suggestions

Metric	Value
New/modified tests analyzed	26
✅ Design tests (behavioral contracts)	24 (92%)
⚠️ Implementation tests (low value)	2 (8%)
Tests with error/edge cases	12 (46%)
Duplicate test clusters	0
Test inflation detected	⚠️ Yes — 275 test lines vs. 2 production lines changed (137:1 ratio)
🚨 Coding-guideline violations	None

---

Test Classification Details

View all 26 test classifications

Test	File	Classification	Notes
delay — resolves after specified time	expired_entity_cleanup_helpers.test.cjs	✅ Design	Verifies timing promise
delay — resolves immediately for 0 ms	expired_entity_cleanup_helpers.test.cjs	✅ Design	Edge case: zero duration
validateCreationDate — valid ISO 8601 date	expired_entity_cleanup_helpers.test.cjs	✅ Design	Happy path
validateCreationDate — valid date-only string	expired_entity_cleanup_helpers.test.cjs	✅ Design	Format variant
validateCreationDate — invalid date string	expired_entity_cleanup_helpers.test.cjs	✅ Design	Error path
validateCreationDate — empty string	expired_entity_cleanup_helpers.test.cjs	✅ Design	Edge case
validateCreationDate — purely numeric string	expired_entity_cleanup_helpers.test.cjs	✅ Design	Edge case
categorizeByExpiration — puts expired in expired array	expired_entity_cleanup_helpers.test.cjs	✅ Design	Core behavioral contract
categorizeByExpiration — puts non-expired in notExpired array	expired_entity_cleanup_helpers.test.cjs	✅ Design	Core behavioral contract
categorizeByExpiration — skips invalid creation dates + warns	expired_entity_cleanup_helpers.test.cjs	✅ Design	Error path + side-effect
categorizeByExpiration — skips missing expiration markers + warns	expired_entity_cleanup_helpers.test.cjs	✅ Design	Error path + side-effect
categorizeByExpiration — returns now Date	expired_entity_cleanup_helpers.test.cjs	✅ Design	Output shape contract
categorizeByExpiration — attaches expirationDate	expired_entity_cleanup_helpers.test.cjs	✅ Design	Output shape contract
processExpiredEntities — processes all and returns closed	expired_entity_cleanup_helpers.test.cjs	✅ Design	Core behavioral contract
processExpiredEntities — tracks skipped separately	expired_entity_cleanup_helpers.test.cjs	✅ Design	Status routing
processExpiredEntities — tracks failed when throws	expired_entity_cleanup_helpers.test.cjs	✅ Design	Error handling
processExpiredEntities — respects maxPerRun limit	expired_entity_cleanup_helpers.test.cjs	✅ Design	Boundary: call-count maps to observable side-effect limit
processExpiredEntities — empty input returns empty arrays	expired_entity_cleanup_helpers.test.cjs	✅ Design	Edge case: empty input
buildExpirationSummary — includes heading and scan summary	expired_entity_cleanup_helpers.test.cjs	✅ Design	Output content contract
buildExpirationSummary — lists successfully closed entities	expired_entity_cleanup_helpers.test.cjs	✅ Design	Output content contract
buildExpirationSummary — shows failed entities when present	expired_entity_cleanup_helpers.test.cjs	✅ Design	Error-path output
buildExpirationSummary — shows remaining count when > maxPerRun	expired_entity_cleanup_helpers.test.cjs	✅ Design	Boundary condition output
buildExpirationSummary — shows skipped when includeSkippedHeading true	expired_entity_cleanup_helpers.test.cjs	✅ Design	Flag behavior
buildExpirationSummary — omits skipped when includeSkippedHeading false	expired_entity_cleanup_helpers.test.cjs	✅ Design	Flag behavior toggle
constants — DEFAULT_MAX_UPDATES_PER_RUN is 100	expired_entity_cleanup_helpers.test.cjs	⚠️ Implementation	Pins a specific constant value
constants — DEFAULT_GRAPHQL_DELAY_MS is 500	expired_entity_cleanup_helpers.test.cjs	⚠️ Implementation	Pins a specific constant value

---

Flagged Tests — Suggestions

⚠️constants — exports DEFAULT_MAX_UPDATES_PER_RUN as 100 / DEFAULT_GRAPHQL_DELAY_MS as 500

Classification: Implementation tests
Issue: These tests pin specific constant numeric values. They will fail whenever a developer intentionally changes a default, even if all behavioral contracts remain satisfied.
What design invariant does this test enforce? The specific exported values — not the behavior those values produce when used downstream.
What would break if deleted? Only an explicit value change would be missed, not a behavioral regression.
Suggested improvement: Either remove these tests (constants are trivially verified by reading the source), or reframe them as behavioral assertions — for example: test that processExpiredEntities with no explicit maxPerRun argument defaults to processing no more than DEFAULT_MAX_UPDATES_PER_RUN entities, which ties the constant to observable behavior.

---

Test Inflation Note

The formula flags test inflation because this PR adds 275 test lines while modifying only 2 production lines (ratio: 137:1 > 2:1 threshold). However, this is expected context: the PR ([jsweep]) is retroactively adding test coverage for an existing production module (expired_entity_cleanup_helpers.cjs), not testing newly written production code. The inflation penalty (−10 pts) is applied per formula but does not indicate a quality problem — adding retroactive coverage is a positive contribution.

---

Mocking Assessment

global.core (core.warning, core.info, etc.) is mocked via mockCore — this is the GitHub Actions runtime, which is the correct and accepted target for mocking (external I/O). The injected processEntity callback in processExpiredEntities tests is also legitimately mocked since it represents an external side-effecting function passed by callers. No internal business-logic functions are mocked without behavioral assertions. ✅

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 0 tests
• 🟨 JavaScript (*.test.cjs): 26 tests (vitest)

---

Verdict

✅ Check passed. Only 8% of new tests are implementation tests (threshold: 30%). The test suite provides strong behavioral coverage for expired_entity_cleanup_helpers.cjs, including error paths, boundary conditions, and side-effect verification. Minor suggestion: consider converting the two constant-value tests into behavioral assertions.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal values (specific constant numbers, internal call counts without behavioral effect)
• Give false assurance: they pass even when the system is wrong
• Break during legitimate changes even when behavior is correct

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

---

References:§25359488983

🧪 Test quality analysis by Test Quality Sentinel · ● 658.9K · ◷

[github-actions]

Test Quality Sentinel: 61/100. Test quality is acceptable.

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Addressed both review findings in commit 6936fea:

1. delay tolerance too loose — raised the lower bound from 5ms to 10ms so the assertion actually verifies the requested wait was honored (same issue on line 136 of the original draft no longer exists in the current file since processExpiredEntities tests use delayMs: 0).

2. Truncation not exercised — added a new buildExpirationSummary test that passes 12 notExpired entries and asserts the output contains "showing first 10", includes entry Add HTTP transport capability validation and time MCP server for agentic engines #19 (the last of the first 10), and excludes entry Make compute-text step lazily inserted only when needed #21 (the 12th item).