From 6e5c16ed4e47e9492838ab7c226289d7586b5831 Mon Sep 17 00:00:00 2001 From: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Date: Tue, 10 Nov 2026 14:25:52 -0500 Subject: [PATCH] test(mutation): add approve.ts to the nightly Stryker set and pin the set and its break score to docs/contributing.md approve.ts decides the approved label tide waits on, but it was in neither Stryker set, so a behavioural survivor there would only be found by someone running --mutate by hand. The run at 9aa12b8 scores 502 killed / 1 timeout / 33 survived; every survivor is reviewed in docs/contributing.md and all 33 are equivalent. Combined with the scheduled job's 762/31 the nightly set reaches 95.18 %, so break moves from 95 to 94 by the file's own just-under-main rule. mutationConfig.test.ts keeps the two configs and the docs in step: the nightly set must extend the local one, add exactly the three merge files, name a break score the docs quote in both places, and be documented file for file. Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> --- __tests__/mutationConfig.test.ts | 46 ++++++++++++++++++++++++++++++++ docs/contributing.md | 34 ++++++++++++++++++++--- stryker.nightly.config.mjs | 23 ++++++++-------- 3 files changed, 88 insertions(+), 15 deletions(-) create mode 100644 __tests__/mutationConfig.test.ts diff --git a/__tests__/mutationConfig.test.ts b/__tests__/mutationConfig.test.ts new file mode 100644 index 00000000..dfc0190b --- /dev/null +++ b/__tests__/mutationConfig.test.ts @@ -0,0 +1,46 @@ +import type { PartialStrykerOptions } from '@stryker-mutator/api/core' +import { readFileSync } from 'node:fs' +import * as path from 'node:path' +import { describe, expect, it } from 'vitest' + +// the configs are JSDoc-typed .mjs files outside tsconfig's `include`, so they import as `any` +// @ts-expect-error TS7016 +import untypedBase from '../stryker.config.mjs' +// @ts-expect-error TS7016 +import untypedNightly from '../stryker.nightly.config.mjs' + +const base = untypedBase as PartialStrykerOptions +const nightly = untypedNightly as PartialStrykerOptions +const baseSet = (base.mutate ?? []).filter((file): file is string => file !== undefined) +const nightlySet = (nightly.mutate ?? []).filter((file): file is string => file !== undefined) + +const root = path.resolve(__dirname, '..') +const contributing = readFileSync(path.join(root, 'docs/contributing.md'), 'utf8') + +// The nightly set is the local merge-decision set plus the code that performs +// the merge (tide.ts, mergeQueue.ts) and the code that decides the `approved` +// half of it (approve.ts); docs/contributing.md names the same files and the +// same `break` score, so a change to either must move the other. +describe('stryker.nightly.config.mjs', () => { + it('extends the local merge-decision set', () => { + expect(nightlySet).toEqual(expect.arrayContaining(baseSet)) + }) + + it('adds the files that perform and approve the merge', () => { + const added = nightlySet.filter(file => !baseSet.includes(file)) + expect(added.sort()).toEqual(['src/plugins/approve.ts', 'src/plugins/tide.ts', 'src/utils/mergeQueue.ts']) + }) + + it('sets the break score docs/contributing.md documents', () => { + const score = nightly.thresholds?.break + expect(score).toEqual(expect.any(Number)) + expect(contributing).toContain(`a \`break\` score of ${score}`) + expect(contributing).toContain(`fails below a score of ${score}`) + }) + + it('is documented file for file', () => { + for (const file of nightlySet) { + expect(contributing, `${file} is in the nightly set but docs/contributing.md never names it`).toContain(`\`${file}\``) + } + }) +}) diff --git a/docs/contributing.md b/docs/contributing.md index 14993259..a02a13b4 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -66,7 +66,7 @@ label and reminder comment the bot left in the meantime. | `npx vitest run __tests__/bundle` | Only the bundle acceptance harness | | `npm run test:shuffle` | The whole suite with the tests of every file in a random order (`--sequence.shuffle`; pass `--sequence.seed=` to repeat an order); the `shuffle` job runs it on every pull request; see "Test order" below | | `npm run test:mutation` | StrykerJS over the merge-decision code, locally; see "Mutation checks" below | -| `npm run test:mutation:nightly` | The same set as the `mutation` job runs on the daily schedule (`stryker.nightly.config.mjs`): no incremental file, and a `break` score of 95 | +| `npm run test:mutation:nightly` | The same set as the `mutation` job runs on the daily schedule (`stryker.nightly.config.mjs`): no incremental file, and a `break` score of 94 | | `npm run test:coverage:e2e` | Only the bundle acceptance harness, run against a source-mapped bundle built from `__tests__/bundle/coverageEntry/` into `.coverage-bundle/` so its hits are reported against `src/`, written to `coverage-e2e/` so it never overwrites the unit run's `coverage/` (ncc's map is line-only, so the build also emits tsc's per-file maps into `.coverage-bundle/tsc/` and `coverageEntry/remap.mjs` composes their columns into it; without that, inner lines of a multi-line expression that tsc flattens onto one JS line report 0 hits). This is end-to-end-only coverage and is deliberately reported separately from `test:coverage`: the unit run already reaches every line, so a union would say nothing about what the bundle exercises, and the two runs' statement and branch maps come from different transforms (vite vs. tsc+webpack) and do not combine. Fails below 99 % lines / 99 % branches / 99 % functions / 99 % statements (`vitest.e2e-coverage.config.mjs`), a floor set just under what `main` reaches so a scenario that stops driving a `src/` path is caught | Unit tests under `__tests__/` import `src/` directly and mock the GitHub API @@ -184,9 +184,11 @@ gate: the suite takes minutes per mutant set, so the local run sets no `break` threshold and nothing fails a push. The `mutation` job of `test.yml` runs the same set on the daily schedule (and on `workflow_dispatch`) through `stryker.nightly.config.mjs`, together with `src/plugins/tide.ts` and -`src/utils/mergeQueue.ts` — the code that performs the merge, left out of the -local default only for its ~620 extra mutants — fails below a score of 95 — -just under what `main` reaches with its equivalent survivors — and publishes +`src/utils/mergeQueue.ts` — the code that performs the merge — and +`src/plugins/approve.ts` — the code that decides the `approved` label the +merge waits on — left out of the local default only for their ~1150 extra +mutants — fails below a score of 94 — just under what `main` reaches with its +equivalent survivors — and publishes the per-file tally in the job summary and `reports/mutation/` as an artifact. ```sh @@ -234,6 +236,30 @@ by the `calls).toHaveLength(1)` assertions of the two enqueuer-less tests): | `tide.ts:554` `numbers.length === 0 && lookup !== undefined` → `lookup` always taken | Every caller that can pass an empty list (`tideOnCheckSuite`) also passes `lookup`; the others pass one number | | `mergeQueue.ts:102` `entry.enqueuer.__typename === undefined ? {} : { enqueuerType }` → the `enqueuerType` key always spread | The spread then sets `enqueuerType: undefined`, which `enqueuedByBot` (`=== 'Bot'`) and `toEqual` cannot tell from an absent key | +Reviewed survivors of `approve.ts` (`npm run test:mutation -- --mutate +src/plugins/approve.ts` at 9aa12b8: 502 killed, 1 timeout, 33 survived, 93.84 % +on its own; with the 762/31 of the scheduled job at the same commit the nightly +set scores 95.18 %, hence the `break` of 94): + +| Mutant | Why it is left | +|--------|----------------| +| `approve.ts:99`, `:100` `comment.body ?? ''` and `body === ''` → a placeholder body / `false`; `:116` `review.body ?? ''`; `:464` `label?.name ?? ''`; `:490` `review?.body ?? ''` | A placeholder carries no `/approve` or `/lgtm` command, no review marker and is not the `approved` label, so `commandKind` returns `undefined` and the marker and label tests fail exactly as they do for the empty string | +| `approve.ts:138` `'add'` → `''` | Both callers only compare the result with `'cancel'` | +| `approve.ts:179` `default:` of the `effectiveKind` switch | The `default` arm and the `case 'remove'` arm before it both end in `break` | +| `approve.ts:206` `settings.require_self_approval && author !== ''` → `true` / `\|\|` / placeholder | An author who may self-approve is already in `approving`, so if they cover a file they are already in `covering`, and if they cover none they appear in no `coverage` entry; an empty or placeholder login matches no approver either way | +| `approve.ts:230`, `:232` `'ignore'` → `''` | The switch in `computeApproval` has no `''` arm, so it falls to `default` like `'ignore'` | +| `approve.ts:241` `remaining.size > 0` → `>= 0` | With nothing remaining `coverage` is empty and the `coverage.size === 0` guard breaks before `[0]` is read | +| `approve.ts:256` `excluded.add(best)` dropped | Every file `best` covers leaves `remaining` in the same iteration, so `best` appears in no later `coverage` map | +| `approve.ts:335` `approvers: []` of an uncoverable file → a placeholder | `renderNotifier` prints a `path === undefined` entry without reading its `approvers` | +| `approve.ts:341`, `:344-345` the `byPath` merge arm and its `.sort()` | Files under the same deepest OWNERS file share `set.approvers`, so every file's `coveredFiles` list is the same sorted list and overwriting, merging and re-sorting all produce it | +| `approve.ts:378`, `:379` `[]` → a placeholder review list | `approvalEvents` skips an entry whose `user` has no login | +| `approve.ts:383-384` the `log.info` text and its joiners | Log output is not asserted | +| `approve.ts:398` `present.length > 0` → `true` / `>= 0` | `removeLabels` with an empty list issues no request, so the guard only saves a loop | +| `approve.ts:414` `body.trim()` → `body` | The notifier `body` starts with text and ends with the marker, so it has nothing to trim | +| `approve.ts:460`, `:486` `action === undefined` → `false` | `Set.has(undefined)` is false, so the second half of the `\|\|` skips the same payloads | +| `approve.ts:491` `review?.id` → `review.id` in a debug line | Reached only after `review?.body` included the marker, so `review` is present | +| `approve.ts:514` `pull_request?.base` → `pull_request.base` | Both callers pass `pull_request?.number`, and `evaluateOnOwnersRepo` throws on `undefined` before this line | + Reviewed survivors of the default set: | Mutant | Why it is left | diff --git a/stryker.nightly.config.mjs b/stryker.nightly.config.mjs index c2f0d979..71db08df 100644 --- a/stryker.nightly.config.mjs +++ b/stryker.nightly.config.mjs @@ -3,24 +3,25 @@ import base from './stryker.config.mjs' /** * `npm run test:mutation:nightly` — the merge-decision set of - * stryker.config.mjs, plus the code that performs the merge, run as the - * scheduled `mutation` job of .github/workflows/test.yml. No incremental file - * (CI has no previous run to reuse), and a `break` score just under what + * stryker.config.mjs, plus the code that performs and approves the merge, run + * as the scheduled `mutation` job of .github/workflows/test.yml. No incremental + * file (CI has no previous run to reuse), and a `break` score just under what * `main` reaches (96 % at ed3bb42 over the default set, six equivalent * survivors; 96.1 % at 424fb95 with tide.ts and mergeQueue.ts, 24 more - * reviewed survivors) so a new behavioural survivor fails the nightly run - * instead of waiting for someone to run the check by hand. See - * docs/contributing.md ("Mutation checks"). + * reviewed survivors; 95.2 % at 9aa12b8 with approve.ts, 33 more) so a new + * behavioural survivor fails the nightly run instead of waiting for someone to + * run the check by hand. See docs/contributing.md ("Mutation checks"). * * @type {import('@stryker-mutator/api/core').PartialStrykerOptions} */ export default { ...base, - // tide.ts decides and performs the merge and mergeQueue.ts enqueues it; they - // stay out of the local default set only because they add ~620 mutants - // (~25 minutes), which the scheduled job can afford. - mutate: [...base.mutate, 'src/plugins/tide.ts', 'src/utils/mergeQueue.ts'], + // tide.ts decides and performs the merge, mergeQueue.ts enqueues it and + // approve.ts decides the `approved` label the merge waits on; they stay out + // of the local default set only because they add ~1150 mutants, which the + // scheduled job can afford. + mutate: [...base.mutate, 'src/plugins/tide.ts', 'src/utils/mergeQueue.ts', 'src/plugins/approve.ts'], incremental: false, reporters: ['clear-text', 'json'], - thresholds: { high: 90, low: 75, break: 95 }, + thresholds: { high: 90, low: 75, break: 94 }, }