test: run full test suite in npm test (was only smoke tests)#13
Open
dmchaledev wants to merge 1 commit into
Open
test: run full test suite in npm test (was only smoke tests)#13dmchaledev wants to merge 1 commit into
dmchaledev wants to merge 1 commit into
Conversation
The npm test script only ran test/smoke.test.mjs (6 tests), so the 59-test calculate.test.mjs suite covering core VM sizing, timing, cost, and ROI logic never executed in CI. Glob all *.test.mjs files so the full suite (65 tests) runs on every push, PR, and pre-publish check. Shell-expanded glob keeps this portable across the supported Node 18/20/22 matrix (npm runs scripts via sh).
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
2 participants
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
npm testscript only rantest/smoke.test.mjs:That meant the 59-test
calculate.test.mjssuite — added in #9 specifically to cover the core VM sizing, scan timing, cost, and ROI logic — never actually ran in CI. Every workflow usesnpm test:ci.yml(PR + push, Node 18/20/22 matrix)auto-tag.yml(gates the auto-tag onmain)publish.ymlchainSo all three checks were green while exercising only 6 smoke assertions and skipping the 59 tests that validate the actual calculations users depend on. A regression in
calculate()could ship to npm without any test catching it.The #9 commit message even claimed "Updated test script to glob", but the script in
mainstill points at the single smoke file — so this restores the intended behavior.Fix
A shell-expanded glob (npm runs scripts via
sh) keeps this portable across the supported Node 18/20/22 matrix — native test-runner globbing wasn't added until Node 21, so a quoted glob would silently match nothing on 18/20.Verification
npm testNo source or test changes — only the npm script. All 65 tests pass.
https://claude.ai/code/session_01KysAzJtSKywS8tMzTbs3SY
Generated by Claude Code