test: run the real code instead of hand-copied mirrors (sable-1drb) - #228
Closed
Rome-1 wants to merge 2 commits into
Closed
test: run the real code instead of hand-copied mirrors (sable-1drb)#228Rome-1 wants to merge 2 commits into
Rome-1 wants to merge 2 commits into
Conversation
The composite action's results step coerced every jq failure into findings_count=0: a body that was not JSON, a 200 carrying an error object, or a parseable payload with no vulnerabilities key passed every severity threshold and rendered "No security findings detected". Validate the shape first; on failure exit 1 with status=unreadable, an actionable message, and no count outputs at all. Once the shape holds the counts cannot fail, so the fallbacks are removed rather than moved. rafter issues create from-scan (Node and Python) had the same shape: data.vulnerabilities || [] turned a processing or failed scan, an error object, or a keyless payload into "No findings to create issues for". Both now refuse with exit 1 and name the scan status. An empty array is still a clean result. Regression coverage: two end-to-end CI jobs drive the real action against the mock backend (a 3-way matrix of unreadable shapes asserting the build fails with status=unreadable and an EMPTY findings-count, plus an exact counts job asserting 3/1/1/0/1); three drift assertions; unit tests on the real functions in both runtimes. Every new guard was mutation-verified by hand: restoring the old behaviour fails the tests that exist to catch it.
The action's threshold-eval and PR-comment-tip tests each carried their own transcription of the bash they tested, so they could pass in full while action.yml was broken. The logic now lives once in github-action/lib/severity.sh, sourced by both action.yml steps and by both tests. Behaviour unchanged. A new end-to-end job runs the real action with findings above the threshold and asserts status=completed AND outcome=failure, which proves the counts reach the gate; drift check 17 asserts action.yml sources the library in both steps and carries no inline copy, so the tests cannot be silently detached again. node/tests/issues.test.ts mirrored four modules (dedup, issue-builder, from-text, from-scan) and tested the mirrors; one had already drifted. The copies are removed and every describe imports the shipped function. Three source functions gained `export` for that. Every guard was mutated by hand: each mutation fails named tests that were untouched by the same mutation before this change.
Collaborator
Author
|
Superseded by a clean rebase onto main. The branch behind this PR still carried #224's commit, which was squash-merged, so it conflicted. Reopening was not possible because the original base ref was deleted. Same commit, rebased, continues in the replacement PR. |
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
1 participant
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.
Replaces #225, which I closed by accident: I merged #224 with
--delete-branch, and deleting that branch closed the PR stacked on it. GitHub refuses to reopen a PR whose base ref is gone, so this is the same head branch retargeted onto main. No code changed.What this does
Deletes the hand-copied test mirrors and runs the real implementations instead. The bash tests now
sourcethe samelib/severity.shthataction.ymlsources, rather than asserting against a transcribed copy of it.Coverage is provably unchanged
The diff removes 324 lines, which is exactly where coverage tends to vanish quietly, so it was checked rather than assumed.
node/tests/issues.test.tsbefore vs after:it()96 → 96,describe()25 → 25,expect()159 → 159, and acommover the sorted test titles is empty on both sides — not one title added, dropped or renamed. Every deletion is a mirrored implementation ofdedup,issue-builder,from-textordraftsFromLocalScan; all four imported symbols are real exports.The gates were mutation-tested rather than read: breaking the
higharm ofseverity.shproduced three named failures, and check 17 correctly failed when bothsourcelines were stripped.test-threshold-gate-end-to-endsupplies the wiring proof a unit test cannot.Known follow-up, filed separately
rafter_threshold_failstreats an empty or non-numeric count as zero findings and passes the build — verified by execution for every threshold. That is pre-existing behavior this PR relocates rather than introduces, and it is tracked as its own bead along with retargeting the severity-gateif:fromsteps.poll.outputs.statustosteps.results.outputs.status.Follows #224.