diff --git a/.ci/js-mutation-scope-baseline.json b/.ci/js-mutation-scope-baseline.json index dd396eddf..a3b13c9f5 100644 --- a/.ci/js-mutation-scope-baseline.json +++ b/.ci/js-mutation-scope-baseline.json @@ -357,13 +357,9 @@ "app/ui/src/hooks/useDebouncedValue.js", "app/ui/src/hooks/useDocsUrl.js", "app/ui/src/hooks/useElapsedTimer.js", - "app/ui/src/hooks/useEntityPage.js", "app/ui/src/hooks/useExpandableGraph.js", "app/ui/src/hooks/useFeatures.js", "app/ui/src/hooks/useFetch.js", - "app/ui/src/hooks/useMatrix.js", - "app/ui/src/hooks/useMatrixRowOrder.js", - "app/ui/src/hooks/useNestedGroupExpand.js", "app/ui/src/hooks/usePermissions.js", "app/ui/src/hooks/usePersistedState.js", "app/ui/src/hooks/useRecentChanges.js", diff --git a/.github/workflows/js-mutation.yml b/.github/workflows/js-mutation.yml index b202bc4e2..8344a0b1a 100644 --- a/.github/workflows/js-mutation.yml +++ b/.github/workflows/js-mutation.yml @@ -19,7 +19,7 @@ # time; that is the deliberate trade for the runtime, same as ps-mutation.yml. # test/ci-scripts/test-gate-wiring.sh enforces this for every *-mutation.yml. # -# SCOPE IS SMALL AND THAT IS THE POINT OF PUBLISHING IT. Twenty API/UI files are +# SCOPE IS SMALL AND THAT IS THE POINT OF PUBLISHING IT. 24 API/UI files are # mutation-tested out of ~410 eligible; the remaining backlog is counted in # .ci/js-mutation-scope-baseline.json and guarded by app/api/src/mutationScope. # guard.test.js, which fails when new code is added without a decision. Publishing @@ -59,7 +59,17 @@ jobs: mutation: name: 'Mutation: ${{ matrix.scope.label }}' runs-on: ubuntu-latest - timeout-minutes: 45 + # 60. The UI scopes are an order of magnitude more expensive per mutant than + # the API ones -- every mutant re-renders React under jsdom -- but no scope is + # near this now: measured locally, the slowest is 14 minutes (UI list hook, + # 402 mutants) against ~5 for the 1,122-mutant API matrix scope. This was + # briefly 120, when the UI hooks lived in one config and took 58 minutes; the + # fix was splitting that config rather than buying it more time, because a + # mutant costs one run of EVERY test in its include set. Keep the margin for + # slower CI hardware and for tests yet to be written -- better-covered code is + # slower to mutate -- but treat a scope approaching this as a signal to split + # it, not to raise the number. + timeout-minutes: 60 strategy: # A scope that lands below its floor must not cancel the other three. The # point of the weekly run is a complete picture; losing three numbers to one @@ -71,7 +81,9 @@ jobs: - { pkg: api, name: effectiveaccess, label: 'API effective access' } - { pkg: api, name: accountlinking, label: 'API account linking' } - { pkg: api, name: matrix, label: 'API matrix' } - - { pkg: ui, name: pilot, label: 'UI' } + - { pkg: ui, name: pilot, label: 'UI permissions + filter' } + - { pkg: ui, name: hooks, label: 'UI matrix hooks' } + - { pkg: ui, name: listhooks, label: 'UI list-page hook' } steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/app/api/stryker.matrix.config.json b/app/api/stryker.matrix.config.json index ed78cce5d..2259672c8 100644 --- a/app/api/stryker.matrix.config.json +++ b/app/api/stryker.matrix.config.json @@ -1,6 +1,6 @@ { "$schema": "./node_modules/@stryker-mutator/core/schema/stryker-schema.json", - "_comment": "The matrix layer -- the code that decides which access appears in the matrix once inheritance, context rollups and attribute cuts are applied. Chosen as the next scope because it is the closest analogue of the two worst surprises measured so far: src/effectiveAccess/engine.js read 93% line coverage and scored 69% under mutation, src/accountlinking/classifier.js read 97% and scored 68%. Six of these eight files sit at 100% line coverage with their own dedicated test file, which is the same profile, and inheritedAccess.js decides whether a grant is shown as held directly or through a group -- a wrong answer there is silent in both directions and is exactly what a reviewer reads off the matrix. Run: cd app/api && npm run test:mutation:matrix. TEST INCLUDE IS WIDER THAN src/matrix -- see vitest.stryker.matrix.config.js for why: most of these modules are also driven through the route layer, and excluding those tests would manufacture false survivors. MUTATOR SET: everything enabled -- deliberately NOT copying the auth config's StringLiteral/ObjectLiteral exclusions, and the code decided it rather than the assumption. These are SQL builders and app/api/CLAUDE.md notes the unit mocks are SQL-blind, which suggested mutated SQL text would be unkillable; but the tests here assert on the emitted SQL directly (`expect(sql).toContain(\"COUNT(*) FILTER (WHERE t.governed)...\")`), so a mutated literal changes text a test is pinning and dies. That is the opposite of permissions.js, where the strings were prose and 68 of 72 mutants survived. Carry the caveat with the number: pinning SQL text proves it is UNCHANGED, not that the query returns the right rows. Query correctness is the contract tests' job and a high score here does not cover it. MEASURED 63.44% AT A SUITE LINE COVERAGE OF 94.3% -- the widest coverage-to-fault-detection gap recorded in this repo, and the reason this scope was picked. rollupBuilders.js is the sharpest case: 100% of lines, 100% of branches and 100% of functions, and 38% of injected faults still go unnoticed. THE INCLUDE SET IS NOT THE CAUSE: running the full API suite with coverage limited to src/matrix reproduces these per-file figures exactly (94.26% lines / 76.24% branch, same uncovered lines), so the 103 no-coverage mutants are real gaps, not tests this config left out. FLOOR ratchets up only -- 61 -> 65 -> 70 -> 78 as the scope went 63.44 -> 67.02 -> 72.91 -> 80.93. Never lower it to make a red run green. rollupBuilders.js finishes at 96.67 with two survivors that are PROVABLY EQUIVALENT rather than unexamined: both are the ' AND ' separator in `where.join(' AND ')` inside buildRolesAsRowsSql and buildRolesDrillSql, and each of those has exactly one `where.push` in its body, so the list can never hold two conditions and `[x].join(sep) === x` for every sep. They are left visible instead of silenced: the floor sits below them, so they cost nothing, and a future second condition would make them killable again -- which a disable comment would hide. scopeHistory.js followed, 51.66 -> 94.31. Remaining targets, both still on their first measurement: contextRollup.js (68.80) and attributeCut.js (70.93).", + "_comment": "The matrix layer -- the code that decides which access appears in the matrix once inheritance, context rollups and attribute cuts are applied. Chosen as the next scope because it is the closest analogue of the two worst surprises measured so far: src/effectiveAccess/engine.js read 93% line coverage and scored 69% under mutation, src/accountlinking/classifier.js read 97% and scored 68%. Six of these eight files sit at 100% line coverage with their own dedicated test file, which is the same profile, and inheritedAccess.js decides whether a grant is shown as held directly or through a group -- a wrong answer there is silent in both directions and is exactly what a reviewer reads off the matrix. Run: cd app/api && npm run test:mutation:matrix. TEST INCLUDE IS WIDER THAN src/matrix -- see vitest.stryker.matrix.config.js for why: most of these modules are also driven through the route layer, and excluding those tests would manufacture false survivors. MUTATOR SET: StringLiteral and ObjectLiteral excluded, matching every other API config. They were enabled for the first measurements and did pay here -- these tests assert on the emitted SQL, so a mutated literal changes text a test pins and dies, which is the opposite of permissions.js where the strings were prose and 68 of 72 survived. But that yield is mostly one-off, while the cost recurs on every weekly run: string/object mutants were 291 of 1,122 here, 26% of the work. The assertions written to kill them stay and keep earning; only the weekly re-measuring of them stops. Note the score RISES on exclusion, 80.93 -> 83.79, because those mutants were killed at a lower rate than the rest -- read the floor below as measured over a smaller mutant set, not as an improvement. Two files move most: contextRollup.js 68.80 -> 82.76 and attributeCut.js 70.93 -> 74.24, which says most of what was unkilled in them was text. 83.79 is a re-run, not arithmetic on the old report: deriving it by dropping string mutants from the previous report gave 83.39, and up to 3 points out per file, so excluding a mutator does not simply subtract its mutants. Carry this caveat regardless: pinning SQL text proves it is unchanged, not that the query returns the right rows. Query correctness is the contract tests' job. MEASURED 63.44% AT A SUITE LINE COVERAGE OF 94.3% -- the widest coverage-to-fault-detection gap recorded in this repo, and the reason this scope was picked. rollupBuilders.js is the sharpest case: 100% of lines, 100% of branches and 100% of functions, and 38% of injected faults still go unnoticed. THE INCLUDE SET IS NOT THE CAUSE: running the full API suite with coverage limited to src/matrix reproduces these per-file figures exactly (94.26% lines / 76.24% branch, same uncovered lines), so the 103 no-coverage mutants are real gaps, not tests this config left out. FLOOR ratchets up only -- 61 -> 65 -> 70 -> 78 as the scope went 63.44 -> 67.02 -> 72.91 -> 80.93 with string mutation on, then 81 against the 83.79 the same tests score with it off. Never lower it to make a red run green. rollupBuilders.js finishes at 96.67 with two survivors that are PROVABLY EQUIVALENT rather than unexamined: both are the ' AND ' separator in `where.join(' AND ')` inside buildRolesAsRowsSql and buildRolesDrillSql, and each of those has exactly one `where.push` in its body, so the list can never hold two conditions and `[x].join(sep) === x` for every sep. They are left visible instead of silenced: the floor sits below them, so they cost nothing, and a future second condition would make them killable again -- which a disable comment would hide. scopeHistory.js followed, 51.66 -> 94.31. Remaining targets, both still on their first measurement: contextRollup.js (68.80) and attributeCut.js (70.93).", "packageManager": "npm", "testRunner": "vitest", "vitest": { @@ -23,10 +23,16 @@ "src/matrix/rollupBuilders.js", "src/matrix/scopeHistory.js" ], + "mutator": { + "excludedMutations": [ + "StringLiteral", + "ObjectLiteral" + ] + }, "thresholds": { "high": 90, "low": 70, - "break": 78 + "break": 81 }, "concurrency": 4, "timeoutMS": 60000, diff --git a/app/ui/package.json b/app/ui/package.json index b2b2488f6..55eb95c10 100644 --- a/app/ui/package.json +++ b/app/ui/package.json @@ -17,8 +17,10 @@ "test:e2e:sql": "bash scripts/e2e-sql.sh", "test:e2e:headed": "npx playwright test --headed", "test:e2e:ui": "npx playwright test --ui", - "test:mutation": "npm run test:mutation:pilot", - "test:mutation:pilot": "stryker run stryker.pilot.config.json" + "test:mutation": "npm run test:mutation:pilot && npm run test:mutation:hooks && npm run test:mutation:listhooks", + "test:mutation:pilot": "stryker run stryker.pilot.config.json", + "test:mutation:hooks": "stryker run stryker.hooks.config.json", + "test:mutation:listhooks": "stryker run stryker.listhooks.config.json" }, "dependencies": { "@azure/msal-browser": "^5.18.0", diff --git a/app/ui/src/hooks/useEntityPage.test.jsx b/app/ui/src/hooks/useEntityPage.test.jsx index 7a303e670..aa13bd762 100644 --- a/app/ui/src/hooks/useEntityPage.test.jsx +++ b/app/ui/src/hooks/useEntityPage.test.jsx @@ -407,3 +407,77 @@ describe('useEntityPage', () => { expect(groups.result.current.search).toBe(''); }); }); + +// ── When the sidecar fetches fail ─────────────────────────────────────────── +// Columns and tags are loaded alongside the list, and each has the same two-step +// guard: `res.ok ? res.json() : null`, then `if (data) setX(data)`. Neither +// failure arm was reachable from this file -- only a mount test elsewhere in the +// suite happened to drive them, which is a poor place for the contract to live +// and left these mutants unkilled here. +// +// What must NOT happen is the interesting part: a failed columns fetch must leave +// the previous columns alone rather than blanking them, because the filter bar is +// built from that list. Overwriting it with null or [] silently removes every +// filter the user could apply, and an empty filter bar looks like a page with +// nothing to filter on rather than a page whose request failed. +describe('useEntityPage sidecar failures', () => { + // KEY ORDER MATTERS: makeAuthFetch matches by substring, and the columns URL + // ('/api/users/columns') contains the list URL ('/api/users'). List-first would + // answer the columns request with the list payload -- an object where an array + // is expected, which fails deep inside getFilterFields rather than at the mock. + const listOnly = (extra = {}) => ({ + ...extra, + [LIST]: { data: [{ id: '1', displayName: 'Bob' }], total: 1 }, + }); + + it('keeps the list usable when the columns request fails', async () => { + const { result } = setup({ + handler: listOnly({ + [COLUMNS]: jsonResponse({ error: 'nope' }, { ok: false, status: 500 }), + '/api/tags': [{ id: 't1', name: 'VIP' }], + }), + }); + + await waitFor(() => expect(result.current.loading).toBe(false)); + // The rows still arrive — one failed sidecar does not take the page down. + expect(result.current.items).toEqual([{ id: '1', displayName: 'Bob' }]); + // ...and the loading flag still settles, or the filter bar spins forever. + await waitFor(() => expect(result.current.columnsLoading).toBe(false)); + // Tags are unaffected: the two fetches fail independently. + expect(result.current.tags).toEqual([{ id: 't1', name: 'VIP' }]); + }); + + it('keeps the list usable when the tags request fails', async () => { + const { result } = setup({ + handler: listOnly({ + [COLUMNS]: [{ column: 'department', values: ['Sales'] }], + '/api/tags': jsonResponse({ error: 'nope' }, { ok: false, status: 503 }), + }), + }); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.items).toEqual([{ id: '1', displayName: 'Bob' }]); + // Columns are unaffected — again, independent failures. Asserted through + // getOptionsForField rather than getFilterFields: the latter dereferences the + // `fieldLabels` prop with no default, so it throws for any caller that omits + // one. Worth knowing, but not this test's subject. + await waitFor(() => expect(result.current.columnsLoading).toBe(false)); + expect(result.current.getOptionsForField('department')).toEqual(['Sales']); + }); + + it('leaves both sidecars at their defaults when both fail', async () => { + const { result } = setup({ + handler: listOnly({ + [COLUMNS]: jsonResponse({ error: 'nope' }, { ok: false, status: 500 }), + '/api/tags': jsonResponse({ error: 'nope' }, { ok: false, status: 500 }), + }), + }); + + await waitFor(() => expect(result.current.loading).toBe(false)); + await waitFor(() => expect(result.current.columnsLoading).toBe(false)); + // Both stay empty rather than becoming null: every consumer maps over them. + expect(result.current.tags).toEqual([]); + expect(result.current.getOptionsForField('department')).toEqual([]); + expect(result.current.items).toEqual([{ id: '1', displayName: 'Bob' }]); + }); +}); diff --git a/app/ui/src/hooks/useMatrix.test.jsx b/app/ui/src/hooks/useMatrix.test.jsx index e8c00add4..9b89155b7 100644 --- a/app/ui/src/hooks/useMatrix.test.jsx +++ b/app/ui/src/hooks/useMatrix.test.jsx @@ -134,3 +134,154 @@ describe('useMatrix', () => { expect(result.current.loading).toBe(false); }); }); + +// ── Response shape ────────────────────────────────────────────────────────── +// Every field of a roll-up or counts payload is read as `body.x || `. +// That single expression carries two failures and both are silent. Lose the +// default and `undefined` reaches the renderer, which draws an empty matrix that +// looks exactly like "this person has no access". Let the default win over a +// present value and real data disappears the same way. +// +// The existing roll-up test asserted two of the seventeen fields, so the other +// fifteen could have defaulted over live data with nothing failing. +describe('useMatrix response mapping', () => { + const REFERENCE = { + '/api/access-package-groups': [], + '/api/entity-tags': [], + '/api/user-columns': [], + '/api/admin/dashboard-stats': { hasData: true }, + '/api/matrix/default-filter': null, + }; + const FILTER = { conditions: [{ field: 'x', value: 'y' }] }; + + const runWith = (matrixBody) => renderHook(() => useMatrix(FILTER), { + wrapper: makeWrapper({ + auth: { authFetch: makeAuthFetch({ '/api/matrix/data': matrixBody, ...REFERENCE }) }, + }).wrapper, + }); + + // Each roll-up field paired with (what the server sent, what it defaults to when + // the server omits it). One table drives both tests below: previously they were + // two 18-line object literals differing only in values, which is a clone by any + // measure and drifts the moment a field is added to one and not the other. + // + // Every populated value is DIFFERENT from its default, and deliberately so -- + // booleans are true, maxDepth is 4 -- because a surviving `|| false` is invisible + // against a fixture that also says false. The test below asserts that property of + // the table itself, so the fixture cannot quietly stop discriminating. + const ROLLUP_FIELDS = { + rollupKind: ['context', 'attribute'], + rollupContextId: ['ctx-9', null], + focusId: ['node-7', null], + breadcrumb: [[{ id: 'b1' }], []], + nodes: [[{ id: 'n1' }], []], + rollupContent: ['roles-only', 'resources-and-roles'], + layered: [true, false], + layeredAttributes: [true, false], + maxDepth: [4, 1], + resources: [[{ resourceId: 'r1' }], []], + groupValues: [['IT'], []], + groupTotals: [[{ groupValue: 'IT', total: 3 }], []], + counts: [[{ resourceId: 'r1', groupValue: 'IT', directCount: 4 }], []], + businessRoles: [[{ roleId: 'br1' }], []], + roleCounts: [[{ roleId: 'br1', count: 2 }], []], + roleRows: [[{ roleId: 'br1', groupValue: 'IT' }], []], + cells: [[{ resourceId: 'r1', groupValue: 'IT' }], []], + }; + const pick = (i) => Object.fromEntries( + Object.entries(ROLLUP_FIELDS).map(([k, pair]) => [k, pair[i]])); + const SENT = pick(0); + const DEFAULTS = pick(1); + + it('uses a fixture where no field can pass by coincidence', () => { + // Guards the table, not the hook. If a populated value ever equals its own + // default, the passthrough test below still passes while proving nothing about + // that field -- the mutant that swaps them would be undetectable. + const indistinguishable = Object.entries(ROLLUP_FIELDS) + .filter(([, [sent, dflt]]) => JSON.stringify(sent) === JSON.stringify(dflt)) + .map(([k]) => k); + expect(indistinguishable).toEqual([]); + }); + + it('passes every roll-up field through untouched', () => { + // `attribute` is the one field read from a differently-named key (body.rollup), + // so it sits outside the table. + const { result } = runWith({ rollup: 'department', ...SENT }); + + return waitFor(() => expect(result.current.rollup).not.toBe(null)).then(() => { + expect(result.current.rollup).toEqual({ attribute: 'department', ...SENT }); + }); + }); + + it('fills in a documented default for every roll-up field the server omits', async () => { + // A minimal payload — only the field that selects the roll-up branch at all. + // Collections must become empty arrays rather than undefined: the renderer maps + // over them, so undefined is a crash or a blank grid, not a default. + const { result } = runWith({ rollup: 'department' }); + + await waitFor(() => expect(result.current.rollup).not.toBe(null)); + expect(result.current.rollup).toEqual({ attribute: 'department', ...DEFAULTS }); + }); + + it('passes the five headline counts through, and zeroes the ones omitted', async () => { + // Five different non-zero values: a mapper reading the wrong key swaps two of + // them, which identical numbers would hide. + const { result } = runWith({ + rows: [], subjectCount: 3, subjectTotal: 11, resourceCount: 5, + resourceTotal: 17, assignmentCount: 23, + }); + await waitFor(() => expect(result.current.counts.subjectCount).toBe(3)); + expect(result.current.counts).toEqual({ + subjectCount: 3, subjectTotal: 11, resourceCount: 5, + resourceTotal: 17, assignmentCount: 23, + }); + + const { result: sparse } = runWith({ rows: [], subjectCount: 3 }); + await waitFor(() => expect(sparse.current.counts.subjectCount).toBe(3)); + // Zero, not undefined — these render straight into the header counters. + expect(sparse.current.counts).toEqual({ + subjectCount: 3, subjectTotal: 0, resourceCount: 0, + resourceTotal: 0, assignmentCount: 0, + }); + }); +}); + +// ── Reference-data failures ───────────────────────────────────────────────── +// Four reference fetches run on mount, each with its own catch. None was ever +// made to fail, so every fallback was unexecuted — and they are not all the same +// fallback, which is the point: one fails CLOSED (no tags) and one fails OPEN +// (assume data exists). Getting either backwards is silent. +describe('useMatrix when reference data fails', () => { + const rejectFor = (needle) => makeAuthFetch(async (url) => { + if (String(url).includes(needle)) throw new Error('network'); + if (String(url).includes('/api/admin/dashboard-stats')) return { hasData: true }; + if (String(url).includes('/api/matrix/default-filter')) return { conditions: [] }; + return []; + }); + + const run = (authFetch) => renderHook(() => useMatrix(null), { + wrapper: makeWrapper({ auth: { authFetch } }).wrapper, + }); + + it('falls back to no tags when the tag fetch fails', async () => { + const { result } = run(rejectFor('/api/entity-tags')); + // An empty Map, not null: null means "still loading" to every consumer. + await waitFor(() => expect(result.current.groupTagMap).not.toBe(null)); + expect(result.current.groupTagMap.size).toBe(0); + }); + + it('assumes data exists when the has-data check fails', async () => { + // Fails OPEN on purpose. hasData=false routes the user to an empty-state + // screen telling them to import data; a transient failure must not do that + // to a tenant whose data is fine. + const { result } = run(rejectFor('/api/admin/dashboard-stats')); + await waitFor(() => expect(result.current.hasData).toBe(true)); + }); + + it('falls back to no default filter when that fetch fails', async () => { + // null, not undefined: undefined is the "not yet loaded" sentinel the matrix + // waits on, so returning it would hang the view rather than show it unfiltered. + const { result } = run(rejectFor('/api/matrix/default-filter')); + await waitFor(() => expect(result.current.defaultFilter).toBe(null)); + }); +}); diff --git a/app/ui/stryker.hooks.config.json b/app/ui/stryker.hooks.config.json new file mode 100644 index 000000000..b0c4abe2d --- /dev/null +++ b/app/ui/stryker.hooks.config.json @@ -0,0 +1,35 @@ +{ + "$schema": "./node_modules/@stryker-mutator/core/schema/stryker-schema.json", + "_comment": "The first real UI scope beyond the two-file pilot, and it is deliberately the hooks rather than the components: these three decide WHICH rows and WHICH access a reviewer is shown, and each is wrong in a way nobody can see from the screen. useNestedGroupExpand walks a group to the groups it is a member of, up to MAX_NEST_LEVEL, which is how inherited access appears at all; stop one level short and access simply is not displayed. useMatrix gates whether the matrix fetches anything and debounces the filter, so a fault there shows stale or empty results that look like 'no access'. useMatrixRowOrder persists custom row order under a version key -- forget to invalidate on a version bump and the user sees an order the current sort logic no longer produces. Run: cd app/ui && npm run test:mutation:hooks. TEST INCLUDE IS WIDER THAN THE THREE HOOKS' OWN TESTS and the width was measured, not guessed -- see vitest.stryker.hooks.config.js: the hooks' own tests alone reach 70.63% branch, the full UI suite 75.46%, and the chosen include 75.46% with identical uncovered lines. MUTATOR SET: StringLiteral and ObjectLiteral excluded, matching every API config. String mutation is expensive -- it was 20-43% of the mutants in each scope measured so far -- and that cost is paid on every weekly run forever, while the yield is uneven: on prose it is near zero (68 of 72 survivors on permissions.js), and where it does pay it usually pays once. Some strings here ARE behaviour (a localStorage key, a version suffix, a query-parameter name) and those mutants go unmeasured as a result; that is the accepted trade, not an oversight. If a specific string turns out to carry a decision worth pinning, write the assertion for it rather than re-enabling the mutator wholesale. KNOWN FLAKINESS: the UI suite is stable on its own (157 files, 1273 tests green) but produced 2 then 6 different failures across two runs under coverage instrumentation at full parallelism. If this run reports scattered timeouts rather than a consistent set, lower `concurrency` before believing the number. MEASURED 54.27% AGAINST 90.3% LINE COVERAGE -- a 36-point gap, the widest of any scope here, and useMatrix.js alone reads 97.3% line and 41.22% mutation. That 56-point spread is more than double the previous worst (effectiveAccess/engine.js at 93 -> 69). The UI was the part of this codebase we knew least about and the answer is the least reassuring. RUNTIME: 489 mutants in 11 minutes. It was 58 minutes before the split, for four files and 891 mutants -- and the split is why it fell, not the smaller mutant count alone: a mutant costs one run of EVERY test in the include set, so dropping EntityListPage.mount from this config made each of these 489 mutants cheaper as well as fewer. Watch this number when adding tests: better-covered code is slower to mutate, and a React render under jsdom is an order of magnitude dearer per run than an API test. THE SPLIT HAPPENED: useEntityPage moved to stryker.listhooks.config.json, because it needs only EntityListPage.mount while these three need MatrixView.mount and App.mount. Each config's mutants now load fewer tests, which cuts per-mutant cost rather than merely dividing the same work between two jobs. Keep that seam when adding a hook: put it where its mount test already is, or give it its own config. FLOOR 68, from the narrowed scope's own measurement of 70.07. The combined four-file scope scored 65.18 and that number does not carry over -- the denominator changed when useEntityPage left. Ratchets up only.", + "packageManager": "npm", + "testRunner": "vitest", + "vitest": { + "configFile": "vitest.stryker.hooks.config.js" + }, + "reporters": [ + "clear-text", + "json" + ], + "jsonReporter": { + "fileName": "reports/stryker-ui-hooks.json" + }, + "mutate": [ + "src/hooks/useMatrix.js", + "src/hooks/useMatrixRowOrder.js", + "src/hooks/useNestedGroupExpand.js" + ], + "mutator": { + "excludedMutations": [ + "StringLiteral", + "ObjectLiteral" + ] + }, + "thresholds": { + "high": 90, + "low": 70, + "break": 68 + }, + "concurrency": 4, + "timeoutMS": 60000, + "disableTypeChecks": false +} diff --git a/app/ui/stryker.listhooks.config.json b/app/ui/stryker.listhooks.config.json new file mode 100644 index 000000000..c4d57d717 --- /dev/null +++ b/app/ui/stryker.listhooks.config.json @@ -0,0 +1,33 @@ +{ + "$schema": "./node_modules/@stryker-mutator/core/schema/stryker-schema.json", + "_comment": "useEntityPage — the search, filter, tag and pagination state behind every list page (Users, Groups, Identities). A wrong filter here silently shows a different population, and nothing on screen says the list is not what was asked for. Split out of stryker.hooks.config.json rather than added to it: measured together the two scopes took 58 minutes against a 60-minute CI budget, because every mutant pays for every test in the include set, and useEntityPage's ~320 mutants were each loading MatrixView.mount and App.mount for nothing. Splitting on which mount test each hook actually needs cuts per-mutant cost instead of dividing the same work. Run: cd app/ui && npm run test:mutation:listhooks. TEST INCLUDE is useEntityPage.test.jsx plus EntityListPage.mount.test.jsx, and parity was checked rather than assumed -- coverage under those two matches the full UI suite exactly (79.69/71.71/84.31/80.24, same uncovered lines). It was 4 branches short until the sidecar-failure tests were written: the failure arms of the columns and tags fetches had only ever been reached incidentally by a mount test elsewhere in the suite, which is both the wrong home for that contract and precisely the dependency that becomes a false survivor when a scope narrows. MUTATOR SET: StringLiteral and ObjectLiteral excluded, matching every other config here -- string mutation was 20-43% of the mutants in each scope measured and is re-paid every week for an uneven return. MEASURED 57.50% over 402 mutants in 14 minutes; FLOOR 55, ratcheting up only. 64 of its mutants are unreached, so there is a lot left here -- the file is 162 lines of search, filter, tag and pagination state and the least covered of the UI hooks at 80.2% line. RUNTIME CAVEAT WORTH KEEPING: an overnight run of this same config reported 585 minutes. That was the machine sleeping, not the work -- one full pass of its two test files takes 5.5 seconds, and re-running it awake gave 14 minutes. If a mutation run ever reports hours, check whether the host stayed awake before believing it or tuning anything.", + "packageManager": "npm", + "testRunner": "vitest", + "vitest": { + "configFile": "vitest.stryker.listhooks.config.js" + }, + "reporters": [ + "clear-text", + "json" + ], + "jsonReporter": { + "fileName": "reports/stryker-ui-listhooks.json" + }, + "mutate": [ + "src/hooks/useEntityPage.js" + ], + "mutator": { + "excludedMutations": [ + "StringLiteral", + "ObjectLiteral" + ] + }, + "thresholds": { + "high": 90, + "low": 70, + "break": 55 + }, + "concurrency": 4, + "timeoutMS": 60000, + "disableTypeChecks": false +} diff --git a/app/ui/vitest.stryker.hooks.config.js b/app/ui/vitest.stryker.hooks.config.js new file mode 100644 index 000000000..01d1f3195 --- /dev/null +++ b/app/ui/vitest.stryker.hooks.config.js @@ -0,0 +1,49 @@ +import { defineConfig } from 'vite'; +import react from '@vitejs/plugin-react'; +import path from 'path'; + +// Vitest config used only by the hooks mutation run (stryker.hooks.config.json). +// +// Standalone rather than spreading vite.config.js, for the same reasons as +// vitest.stryker.config.js: Stryker sandboxes app/ui alone, so the normal test +// `include` reaching ../../tools/crawlers/** resolves nothing there and aborts the +// run before a mutant is evaluated, and the tailwind plugin plus the '@crawlers' +// alias are build concerns that buy the mutation run nothing. +// +// WIDER THAN THE THREE HOOKS' OWN TESTS, AND MEASURED RATHER THAN GUESSED. The +// obvious include — just the *.test.jsx files next to the hooks — leaves branches +// unexercised that the component mount tests do reach, and every one of those +// would have come back a SURVIVOR that no test could ever kill. Measured before +// choosing: coverage of the three hooks under the list below is 94.33% stmts / +// 82.94% branch / 87.83% funcs / 99.52% lines — identical to the full UI suite, +// with the same uncovered lines. So the narrowing costs nothing. +// +// MatrixView.mount.test.jsx drives useMatrixRowOrder + useNestedGroupExpand +// App.mount.test.jsx the only test that mounts the useMatrix caller +// nestedRows.helpers.test.js the row builder that consumes the expand cache +// +// pageRegistry.test.jsx is deliberately absent: it was measured too and moved none +// of these numbers, so it cannot be any mutant's only killer here. +// +// Re-measure this comparison when adding a hook to the `mutate` list — the answer +// is per-file, not a property of the directory. And prefer a new config over a +// longer include: every mutant pays for every test listed here, which is why +// useEntityPage moved out to vitest.stryker.listhooks.config.js. + +export default defineConfig({ + plugins: [react()], + resolve: { + alias: { '@ui': path.resolve(import.meta.dirname, 'src') }, + }, + test: { + include: [ + 'src/hooks/useMatrix.test.jsx', + 'src/hooks/useNestedGroupExpand.test.jsx', + 'src/hooks/useMatrixRowOrder.test.js', + 'src/components/MatrixView.mount.test.jsx', + 'src/App.mount.test.jsx', + 'src/components/matrix/nestedRows.helpers.test.js', + ], + exclude: ['**/node_modules/**'], + }, +}); diff --git a/app/ui/vitest.stryker.listhooks.config.js b/app/ui/vitest.stryker.listhooks.config.js new file mode 100644 index 000000000..7f39a0e08 --- /dev/null +++ b/app/ui/vitest.stryker.listhooks.config.js @@ -0,0 +1,37 @@ +import { defineConfig } from 'vite'; +import react from '@vitejs/plugin-react'; +import path from 'path'; + +// Vitest config for the list-page hook mutation run (stryker.listhooks.config.json). +// +// Standalone rather than spreading vite.config.js, for the same reason as the other +// vitest.stryker.* configs: Stryker sandboxes app/ui alone, so the normal test include +// reaching ../../tools/crawlers/** resolves nothing there and aborts the run. +// +// TWO TEST FILES, AND THAT IS THE POINT OF THIS CONFIG EXISTING. useEntityPage was +// originally measured alongside the matrix hooks, which meant every one of its ~320 +// mutants also paid for MatrixView.mount and App.mount to load. Splitting on which +// mount test each hook actually needs cuts per-mutant cost rather than merely dividing +// the same work between two jobs. +// +// Parity checked, not assumed: coverage of useEntityPage under these two files is +// 79.69% stmts / 71.71% branch / 84.31% funcs / 80.24% lines, identical to the full UI +// suite with the same uncovered lines. It was 4 branches short until the sidecar-failure +// tests were added to useEntityPage.test.jsx — those arms had only ever been reached +// incidentally by a mount test elsewhere in the suite, which is both a poor place for +// the contract to live and exactly the kind of dependency that turns into a false +// survivor when a scope is narrowed. + +export default defineConfig({ + plugins: [react()], + resolve: { + alias: { '@ui': path.resolve(import.meta.dirname, 'src') }, + }, + test: { + include: [ + 'src/hooks/useEntityPage.test.jsx', + 'src/components/EntityListPage.mount.test.jsx', + ], + exclude: ['**/node_modules/**'], + }, +}); diff --git a/changes/ui-mutation-scope.md b/changes/ui-mutation-scope.md new file mode 100644 index 000000000..e6cdddd07 --- /dev/null +++ b/changes/ui-mutation-scope.md @@ -0,0 +1,4 @@ +- Started measuring the web interface properly, beginning with the four pieces of logic that decide which rows and which access a reviewer is shown: the search/filter/pagination behind every list page, the walk that expands a group into the groups it belongs to (which is how inherited access appears at all), the matrix's data loading, and the saved row order. Only 54% of injected faults are caught, against a line coverage of 90% — the widest gap found anywhere so far. The matrix data hook is the starkest: 97% of its lines run under test, and fewer than half of the faults planted in it are noticed. +- Stopped mutating text literals in the checks that were doing so. It was a quarter to nearly half of the work in every area measured, it is repeated every week, and on descriptive text it finds essentially nothing. The tests written to catch those faults remain; only the repeated re-checking of them stops. +- Closed the worst of the gaps found in the matrix's data loading. The seventeen fields of a matrix response are each read with a fallback for when the server omits them, and only two were ever checked — so fifteen could have been quietly replacing live data with an empty default, which on screen is indistinguishable from "this person has no access". The four pieces of reference data loaded when the matrix opens each have their own fallback for a failed request, and none had ever been made to fail; they are not all the same fallback, and one deliberately assumes data exists so that a momentary network problem cannot send someone to a "no data — import some" screen when their data is fine. +- Made the list pages' search and filter behaviour survive a failed request. The lists of columns and tags a page offers to filter by are loaded alongside the rows, and neither failure path had ever been exercised; a failed load must leave the previous choices in place, because blanking them removes every filter the user could apply and an empty filter bar reads as "nothing to filter on" rather than "that request failed".