Skip to content

test: pin the bulk-action filter scope, the backup failure cause, and the window-open spread hole - #385

Merged
Zaldaryon merged 1 commit into
devfrom
test/pin-three-seams
Sep 5, 2026
Merged

test: pin the bulk-action filter scope, the backup failure cause, and the window-open spread hole#385
Zaldaryon merged 1 commit into
devfrom
test/pin-three-seams

Conversation

@Pixnop

@Pixnop Pixnop commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Three behaviours that are correct in production today and that nothing fails on. Each one can be taken back out in a single line with the whole suite still green, and I checked that first: with the four production mutants below applied together and every test file back at its origin/dev version, npx vitest run reported 165 files, 2124 passed, 2 skipped. So this is test work only, plus one guard inside a test helper that #378 asks for.

Manage Mods bulk actions honour the dropdown filters (#357)

ManageMods builds textFiltered from the search box, then visibleMods from the three dropdowns on top of it, and hands visibleMods to useBulkUpdateMods and to the action bar. The suite pinned the search half of #228 and nothing else, so reading textFiltered instead agreed with every existing case: with no dropdown set the two lists are the same list.

The fixture the filter tests already use has three updatable Mods, Alpha, Gamma and Delta, and each axis leaves a different subset of them on screen. Author "Ann" leaves Alpha. Tag "qol" leaves Alpha and Gamma. VS Version "1.19.4" leaves Beta and Gamma, and Beta has no compatible release, so Gamma is the whole run. A bulk update that touches all three is acting on the folder rather than on what the player is looking at, which is the thing #228 asked for.

The action bar is the same seam one line further down, so the modpack export gets a case too: with Author "Cal" picked, the export ships Gamma and Delta and nothing else.

Mutant Result
useBulkUpdateMods(installation, visibleMods) reads textFiltered red, all three axis tests
ManageModsActionBar installedMods={visibleMods} reads textFiltered red, the export test

The backup failure keeps naming its cause (#358)

Two seams on the path #345 fixed, one per layer.

The first is the describer compressWorker ships. It is written inline in its serveTasks call and nothing imported it, so tests/ipc/workerHost.test.ts was passing a describer of its own on every case and collapsing the shipped one to a constant went unnoticed. The worker module is now imported for real over the same fake parent port the rest of that file uses, and two tasks are driven through it: a source folder that does not exist, which the filesystem answers with a real ENOENT, and a source that is a file rather than a folder, which compression.ts answers with its own sentence. Two distinct throws have to arrive as two distinct messages, which is exactly what a full disk (ENOSPC) or a denied write (EACCES) needs from this layer.

The second is the call site in useMakeInstallationBackup. tests/renderer/describeBackupFailure.test.ts pins the mapping from cause to sentence, including the arm where there is no cause, so dropping the argument reads as a supported path. The hook is now driven end to end to a real compression failure, a destination without room for the archive, and the assertion is on the sentence a player reads: the full-drive one, not the generic "the archive could not be written".

Mutant Result
compressWorker describer collapsed to () => "Compression failed" red, the new worker test, expected the filesystem's own reason, got: Compression failed
describeBackupFailure(result.reason, result.detail) drops result.detail red, the new hook test

The window-open contract rejects a trailing spread (#378)

assertWindowOpenHandlerDenies read action off the object literal and stopped there. Later keys win in an object literal, so return { action: "deny", ...override } hands Electron whatever that object carries while the contract still reads "deny" off the literal in front of it. A spread written before action is harmless for the same reason: action is the later key and wins.

The object-literal check now finds the position of the action property and refuses any SpreadAssignment after it. Fixtures both ways, in the return form and in the expression-body form, plus a mutant built from the shipped handler's own source. The mutant test asserts the shipped denial is still written as a plain object return before mutating it, so a reflow in index.ts fails loudly rather than turning the mutant into a no-op.

Mutant Result
spread appended to the real handler in src/main/index.ts red, "denies every window the renderer asks Electron to open", the window open handler stopped denying renderer-created windows
the new spread check removed from the helper red, both new #378 tests
control: handler parameter renamed to request, call reflowed across lines survives, as it should

Testing

On head 4c0f152:

  • npm run typecheck: passed (node, web, tests).
  • npm run lint:ci: 0 errors, 15 pre-existing React Hooks warnings.
  • npm run format:check: passed.
  • npm run test:coverage: 165 files, 2132 passed, 2 skipped; 93.5% statements, 90.25% branches, 92.99% functions, 95.06% lines locally, floors 87 / 85 / 85 / 89. The CI run on this head is the reference figure if the two differ by a hundredth.
  • npm run build:unpack: passed, Electron 44.1.1 Linux.

Every mutant above was applied alone against the committed tree, checked with git status before the run, and restored after.

Related issues

Type

  • Bug fix
  • Feature
  • Performance
  • Refactor or cleanup
  • Tests only
  • Docs or build

Checklist

  • Targets dev, not main.
  • npm run typecheck passes.
  • npm run lint:ci passes.
  • npm run format:check passes.
  • npm run test:coverage passes, coverage at or above the floor in vitest.config.ts.
  • npm run build:unpack passes.

Closes #357, Closes #358, Closes #378

… the window-open spread hole

Three seams that are correct in production today and that no test fails on.

Manage Mods (#357): the bulk update takes the visible list, but the suite only
pins the search half of it, so reading the text-filtered list instead stays
green with any dropdown filter active. Three cases, one per filter axis, plus
the modpack export, each asserting the run touches only the rows on screen.

Backup failure (#358): the describer the compress worker ships is written
inline and nothing imported it, and the hook's call site can drop the cause it
passes without a test noticing. The worker is now driven for real over a fake
parent port with two distinct filesystem failures, and the hook is driven to a
full-drive compression failure with the sentence a player reads asserted.

Window open handler (#378): assertWindowOpenHandlerDenies read action off the
object literal, so a spread written after it granted at runtime while the
contract stayed green. The object-literal check now rejects a SpreadAssignment
that sits after action, and accepts one before it, where action still wins.
@Pixnop
Pixnop requested a review from Zaldaryon September 5, 2026 18:27

@Zaldaryon Zaldaryon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the follow-up to the note I left on #376, plus two more seams, and all three pins bite where they should.

The window-open spread check is correct. uniqueProperty returns the node out of object.properties, so object.properties.indexOf(actionProperty) matches by reference, and the check rejects only a SpreadAssignment at a later index. { action: "deny", ...override } is refused, { ...override, action: "deny" } is accepted because action is the later key and wins at runtime, and the shipped return { action: "deny" } still passes. The mutant built from MAIN_SOURCE proves the check bites on the real file, and it asserts the shipped denial is still a plain object return before mutating, so a reflow in index.ts fails loudly rather than turning the mutant into a no-op. One gap for a later pass, not this one: only a spread is rejected, so a computed ["action"] key after the literal action would still slip through. The general rule would reject any property after action that is not a statically named assignment whose name is not action.

The bulk-action pins are real. Author "Ann" leaves Alpha, tag "qol" leaves Alpha and Gamma, version "1.19.4" leaves Beta and Gamma of which only Gamma has a compatible release. Each is a strict subset of the three updatable mods, so a bulk update or export reading the unfiltered list fails every assertion. The action-bar export gets the same case one seam further down.

The backup-cause pins drive the real path: inspectSafeCompressionTree runs before the directory check in compression.ts, so a missing source surfaces ENOENT and a file source surfaces "Compression source must be a directory", two distinct sentences, and the hook test carries a real full-drive failure end to end through describeBackupFailure, which is the seam that had gone slack.

Full local gate on 4c0f152 is green: typecheck, lint:ci (0 errors, 15 pre-existing hook warnings), format:check, test:coverage at 93.51% statements and 90.28% branches above the 87/85 floors, build:unpack on Electron 44.1.1. npx vitest run on the four changed files: 47 passing.

Approving.

@Zaldaryon
Zaldaryon merged commit 0af1dd7 into dev Sep 5, 2026
9 checks passed
@Zaldaryon
Zaldaryon deleted the test/pin-three-seams branch September 5, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants