test: run full test suite in npm test, not just smoke tests#18
Open
dmchaledev wants to merge 1 commit into
Open
test: run full test suite in npm test, not just smoke tests#18dmchaledev wants to merge 1 commit into
dmchaledev wants to merge 1 commit into
Conversation
The calculate.test.mjs suite (59 tests covering VM sizing, timing, costs, ROI, and recommendations) was added in dafee26, but the npm test script still pointed only at smoke.test.mjs, so those tests never ran in CI. Switch to node --test auto-discovery, which finds every *.test.mjs file in test/ on Node 18+. npm test now runs all 65 tests (was 6).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The comprehensive
test/calculate.test.mjssuite — 59 tests covering VM sizing, timing, cost, ROI, and recommendation logic — was added in dafee26, whose commit message stated it "Updated test script to glob." But thetestscript inpackage.jsonwas never actually updated; it still ran onlytest/smoke.test.mjs:As a result,
npm test— and therefore CI (.github/workflows/ci.ymlrunsnpm test) — only executed 6 smoke tests. The 59 calculation tests, the ones that actually validate the calculator's math, have never run in CI. A regression in the core sizing/cost engine would pass green.Fix
Switch to Node's built-in test-file auto-discovery:
node --testwith no path argument discovers every*.test.mjsundertest/on Node 18+ (the package's declared floor, and all three versions in the CI matrix). This avoids shell-glob portability issues and automatically picks up any future test files.Verification
npm testAll tests pass. Zero-dependency, no devDeps added — still uses the built-in
node:testrunner.https://claude.ai/code/session_01PPx1tAHPfaY1BasyRLtUz6
Generated by Claude Code