Repository navigation
test(config): pin the type-guard, 24h-bound and fetch-timeout survivors and fail toThrow on a nullish throw - #595
Conversation
…24h delay bound and the fetch timeout `toThrow(message)` passes when `undefined` or `null` is thrown, because chai compares the message only against a truthy error (vitest-dev/vitest#7260). A Stryker block mutant on `malformedSection` in src/utils/config.ts, which turns `throw malformedSection(key)` into `throw undefined`, therefore survived every message assertion in the suite. `__tests__/setup.ts` now fails `toThrow` and `.rejects.toThrow` on a nullish throw; `__tests__/setupToThrowGuard.test.ts` pins that and the unchanged negated, Error and string forms. `__tests__/utils/configTypeGuards.test.ts` pins five survivors of the same run whose existing cases only exercised the other half of each guard: a yaml-number `color: 123456` is refused (config.ts:472), a non-string `missing_label` (config.ts:512) and `hold.label` (config.ts:578) are refused, `blunderbuss.delay: 24h` is accepted while `86401s` is not (config.ts:621), and an https:// source is fetched under `AbortSignal.timeout(5000)` (config.ts:342). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
hivecommons-hive
Bot
added
the
hold
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an Hive will keep the |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Read the diff against the tree at merge-base 9acb046. Looks correct to me.
Verified:
- Every cited guard site exists and the new cases target it: the
typeof value.color === 'string'type half (src/utils/config.ts:471),malformedSection(:484),missing_labelnon-empty-string (:512),hold.label(:577), the inclusivems > maxBlunderbussDelayMs24h bound (:620), andAbortSignal.timeout(5000)infetchUrl(:342). - The
setup.tschaioverwriteMethodguard addresses a real hole (vitest-dev/vitest#7260: chai compares the message only against a truthy error, sothrow undefinedsatisfiestoThrow('msg')). The negated path is left untouched (negateshort-circuits both the wrap and the final check), the.rejectspath reads the already-settled rejection value from theobjectflag, andsetupToThrowGuard.test.tspins Error/string/negated behavior as unchanged. - One deliberate tightening worth noting for the record: bare
toThrow()(no expected message) now also fails on a nullish throw. That is stricter than the minimum fix for #7260, but it's documented indocs/contributing.md, pinned insetupToThrowGuard.test.ts:18, and the full suite (2486 tests) passed with the guard active per CI, so nothing relied on the old behavior. - Imports in
configTypeGuards.test.tsall resolve against the existing__tests__/testUtils.tshelpers (failTestOnUnhandledRequests,noOrgOrRepoConfigExcept,MockContext,setupActionsEnv), and__tests__/setup.tsis already wired viasetupFilesinvitest.config.ts:8. - No
src/ordist/change; disjoint from #591 and #593 as the body claims.Refs #594with an explicit needs-human remainder (the scheduled Stryker workflow) is the right link form here.
CI is green at head da5625e. No findings.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
Heads-up from #599 (type-checks 🐝 Hive Agent: — hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
/kind cleanup 🐝 Hive Agent: — hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
github-actions
Bot
added
kind/cleanup
|
Resolves the __tests__/setup.ts conflict with #607 by keeping both the toThrow guard and the per-test platform reinstall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
Merged 🐝 Hive Agent: — hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Verified locally at head 6b88af4 (fresh clone, npm ci):
- The chai
overwriteMethodguard in__tests__/setup.tsis mechanically sound: it wraps the flagged function to capture the thrown value (sync form), reads the rejection value from thepromise === 'rejects'flag, and only fails after_superpasses and the value is nullish — so negated forms, real Errors and thrown strings are untouched, exactly what__tests__/setupToThrowGuard.test.tspins. - Reproduced the underlying hole:
expect(() => { throw undefined }).toThrow('msg')passes on main under vitest 5.0.5 (chai checks the message only for a truthy error; vitest-dev/vitest#7260 is still open upstream). npx vitest run __tests__/setupToThrowGuard.test.ts __tests__/utils/configTypeGuards.test.ts— 13/13 pass.- The cited guard sites exist where claimed in
src/utils/config.ts(malformedSection throw ~:485,missing_label must be a non-empty string~:512,hold.label~:578, blunderbuss 24h bound,AbortSignal.timeout(5000)on the https source fetch). - CI green at this head;
docs/contributing.mdupdated.
Tests-only (no src//dist/ change), and the new files are disjoint from the other open quality PRs. Looks correct to me — a maintainer can take this on the strength of the suite.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
jeefy
left a comment
jeefy
left a comment
There was a problem hiding this comment.
The nullish-throw hole in chai's toThrow is real and the per-.rejects handling looks right. Note for maintainers: this changes assertion semantics globally for the suite (throw undefined now fails toThrow(msg)). That is the desired direction, and the paragraph in docs/contributing.md records it; flagging so it is a conscious choice rather than a surprise.
jeefy
removed
the
hold
|
/kind cleanup |
…to quality/test-typecheck-tests Applies the three one-line type fixes the PR body predicted for #593 and #595: meowImageEdges.test.ts types the spy as MockInstance, setup.ts types overwriteMethod's _super, configTypeGuards.test.ts reads labels?.kind?. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ests__/ meowImageEdges.test.ts types the spy as MockInstance<typeof comments.createComment>, setup.ts types overwriteMethod's _super parameter, and configTypeGuards.test.ts reads labels?.kind? off parseProwConfig's Partial<ProwConfig>. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Bob Killen <bkillen@linuxfoundation.org>
… and fix the 29 errors it finds (#599) * test: make the unit suite order-independent under vitest --sequence.shuffle `npx vitest run --sequence.shuffle` fails 11 tests in 8 files on main under 4 of 12 seeds tried; the default order passes only because each leaking test happens to run before a neighbour that would not notice. Three mechanisms, all within a file: - a shared imported JSON fixture mutated and never restored: assign, unassign and handleIssueComment rewrite `comment.body`; the cron lgtm test pushed a `hold` label onto the list every other test is served. Clone the fixture or reset the field in beforeEach. - loadProwConfig probes the organization and repository tiers under Promise.all, so when the tier a test exercises rejects, the other tier's sequential probe chain keeps issuing requests after the test returned, and the next test's unhandled-request guard or request:start listener records them. Waiting for the in-flight count to reach zero is satisfied in the gap between two probes. `utils.configFoundAt` serves an empty configuration at the other tier's first probe, so the only sibling request is already in flight when the rejection lands. - module memos not reset: auth.test.ts's outer describes never reset the configuration cache and inherited a `trusted` policy; owners.test.ts's expandOwnersSet describe never reset the group memo; mergeQueueGuards imported @actions/core fresh after a sibling's vi.resetModules while still calling the statically imported queueState, so the assertion spied on a different core instance than the one the module warns through. Adds `npm run test:shuffle` and a "Test order" section to docs/contributing.md describing the three patterns. 18 seeds pass after the change; the bundle suite was already order-independent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(typecheck): type-check __tests__ with tsc -p tsconfig.test.json and fix the 29 errors it finds tsconfig.json excludes __tests__/, and Vitest's esbuild transform strips types without checking them, so nothing in the repository type-checks the test suite. Running tsc over src/ and __tests__/ together reports 29 errors in 17 files, several of them fixtures that drifted from the production types they claim to be: - PullRequestOwners gained headSha; the pullOwners() fixture in approve.test.ts, approveErrorPaths.test.ts and approveGuards.test.ts still builds the old shape - ProwConfig gained approve, lgtm, sweep and authorization; the hand-built configs in blunderbuss.test.ts and requireMatchingLabel.test.ts still carry only the original six keys (now built from mergeProwConfig) - msw 3 renamed SetupServerApi to SetupServer (testUtils.ts) - missingNumberMessage.test.ts imports @actions/github/lib/context, which docs/contributing.md forbids; it now imports src/utils/context - lgtmBinding.test.ts builds a sender without the type field the payload type requires - the remainder are untyped spy declarations (tide.test.ts, tideGuards.test.ts), an untyped it.each table (ownersGuards.test.ts), an unresolved this (fakeGithub.ts), a delete on a required field (sweepArms.test.ts), an over-narrow createLabel parameter (labelSync.test.ts), a milestone type intersection that can never be null (milestone.test.ts), a spyOn over a module/name union (handleIssueComment.test.ts) and index-signature mismatches (configurationDocs.test.ts) Adds tsconfig.test.json (extends tsconfig.json, noEmit, includes src/ and __tests__/, excludes the coverageEntry build which has its own tsconfig) and an npm run typecheck script that runs it. npm run build is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> * test(typecheck): fix the three type errors #593 and #595 added to __tests__/ meowImageEdges.test.ts types the spy as MockInstance<typeof comments.createComment>, setup.ts types overwriteMethod's _super parameter, and configTypeGuards.test.ts reads labels?.kind? off parseProwConfig's Partial<ProwConfig>. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Bob Killen <bkillen@linuxfoundation.org> --------- Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Signed-off-by: Bob Killen <bkillen@linuxfoundation.org> Co-authored-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Bob Killen <bkillen@linuxfoundation.org>
Test Improvement
Pins the six behavioural Stryker survivors of the label/config cluster (
main@ 9acb046, 1357 mutants, 95.36 %, 63 survived, each hand-applied at its exact column span) and closes the assertion hole that let one of them hide. The finding is recorded on #594 as the<!-- hive-finding-folded -->comment "[quality] config.ts type-guard and 24h-bound Stryker survivors, and toThrow passing on a nullish throw"; both suites already reach every site (unit 100 / 99.76 / 100 / 100; e2e 100 / 99.82 / 100 / 100), so this is assertion strength only.Files / cluster claimed — one edited setup file, two new test files, one doc paragraph; no
src/,dist/or existing test case is changed, so this is disjoint from held #591 (plugin tests) and #593 (dispatcher/cron/command tests):__tests__/setup.ts(edited)overwriteMethodguard:toThrow(...)/.rejects.toThrow(...)now fail when the thrown value isundefinedornull. Without it,expect(() => { throw undefined }).toThrow('msg')passes (chai checks the message only for a truthy error; vitest-dev/vitest#7260), which is how the block mutant onmalformedSection(config.ts:485) survived seven full-message assertions__tests__/setupToThrowGuard.test.ts__tests__/utils/configTypeGuards.test.tsparseProwConfig: yaml-numbercolor: 123456/12refused while"123456"is kept (config.ts:472); number/list/mappingmissing_labelrefused (:512); number/list/booleanhold.labelrefused (:578);delay: 24hand1440maccepted,86401srefused (:621);loadProwConfigfetches anhttps://source underAbortSignal.timeout(5000)and the abort surfaces in the load error (:342)docs/contributing.mdtoThrowguardEach mutant, applied by hand at its exact span, fails the case that names it and passes on
main(re-verified this kick for:472and:621: 2 failed / 7 passed under the mutants, 9 passed restored).Testing
npx vitest run __tests__/setupToThrowGuard.test.ts __tests__/utils/configTypeGuards.test.ts— 13 passednpx vitest runwith the guard active — 193 files, 2486 passed, 75 skipped (no existing assertion relied on a nullish throw)npx eslinton the touched files — clean;npx tsc --noEmit— cleanRelated Issue
Refs #594 (needs-human: the scheduled Stryker run proposed there is a maintainer decision and lives in
.github/workflows/; this PR lands only the folded config.ts/toThrow finding recorded on it and leaves the gate question open). The finding was first filed standalone and folded onto #590 and then #594 by the relay's shared-file-citation rule, so this PR cites the tracker the relay chose.Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88