Skip to content

Decide the creation plan in a function, so it can be tested - #99

Merged
ALERTua merged 1 commit into
mainfrom
bugfix/79
Aug 26, 2026
Merged

Decide the creation plan in a function, so it can be tested#99
ALERTua merged 1 commit into
mainfrom
bugfix/79

Conversation

@ALERTua

@ALERTua ALERTua commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Closes #79.

What was wrong

The plan summary is the screen the user reads and approves before any disk is touched — the one place consent is given. It was also the least testable thing in the product:

Before
Lines of inline Write-Host in the linear body ~70
Print calls 27
Branches 13
Variables read 28
Times the mode was tested 3, in two different shapes

The suite lifts functions out of the syntax tree and cannot reach the body, so nothing could check what a line said — only that some string sat somewhere near another. #68 is the precedent: a whole mode shipped broken because its decisions lived where no test could see them.

What it does now

Format-CreationPlan returns one object per line, carrying its text and the colour it prints in. The body assembles the answers and prints what comes back:

foreach ($planLine in (Format-CreationPlan -Answers $planAnswers)) {
    Write-Host $planLine.Text -ForegroundColor $planLine.Colour
}

Thirteen branches became three, and all three are now about what to pass, not what to say.

Why a hashtable rather than an object with fields. Mode-specific variables — $VhdxPath, $DriveLetter, $DiskNumber — are assigned only inside the branch that asked for them, and under Set-StrictMode -Version Latest reading an unset variable throws. Pre-initialising them would print values nobody chose, which AGENTS.md forbids in so many words. So a key exists only where its branch asked the question, and the function refuses a mode it has no plan for rather than falling into the physical branch.

What can be asserted now, and could not be before

  • a mode's plan names only what that mode does
  • a skipped step says it is skipped rather than quietly being absent
  • the size, the label, the disk and the .vhdx path are the ones that were answered
  • the write-access warning reaches the plan line by line, in yellow
  • every kind of line carries the colour that kind is printed in
  • the keys the body sets, the function reads, and the test fixture builds all agree

Proved to have teeth

Each defect was reintroduced in the source and the suite run against it. Both files were restored byte for byte afterwards.

real files                                        -> 737 passed,  0 failed

the vhdx branch is taken in every mode            ->   3 passed,  8 failed
a skipped step vanishes instead of saying so      ->  10 passed,  1 failed
the size named is a literal, not the answer       ->  10 passed,  1 failed
the body stops setting a key the plan reads       -> 736 passed,  1 failed
the fixture answers a key nobody reads            -> 736 passed,  1 failed
a warning turns from yellow to white              -> 735 passed,  2 failed
the write-access warning is dropped from the plan -> 734 passed,  3 failed

Review

An independent review raised 20 points; 19 were applied. Two of them were holes this change had opened or left open:

The write-access warning was covered by nothing. The test fixture always answered WritePolicy = 'Allow', under which that advice is silent — so the line telling the user the drive will mount read-only and the run will stop could have been deleted and the whole suite stayed green. It is the highest-stakes line on the consent screen. There are now cases for Deny and Unknown asserting every line of the advice and its colour.

One retargeted test had become vacuous. It looked for Resolve-WriteAccessPolicyAdvice after the "Skip BitLocker" line and found a different call entirely — the one behind the BitLocker question. Three assertions passed for the wrong reason. It was the eighth test needing retargeting and had not got it.

Also applied: no test asserted a specific colour, so every warning could have turned white unnoticed; the fixture answered GPT, the same value hardcoded in the script, so a literal would have passed for an answer; the .vhdx path and the shrunk drive's label were never positively asserted; an IndexOf comparison had no guard, and -1 is less than everything; the call-argument assertion had weakened to checking parameter names only; the registry policy path had become a second literal in the tests, and is now read out of the script; [string]$Colour accepted anything and is now [System.ConsoleColor], failing on the way in rather than halfway through printing.

Adapted rather than applied: the suggestion to restructure the mode branches as a switch with a throwing default. A guard at the function's entrance gives the same protection — a fourth mode fails loudly instead of falling into the physical branch — for far less churn, and it has its own test.

Declined, one: renaming Format-CreationPlan to Resolve-CreationPlan because it returns objects rather than strings. It renders the plan, and the colour is part of the rendering rather than a decision left to the caller; Format-CreationPlan is also the name #79 itself asks for. The docstring that made it sound like a decision function was reworded instead.

🤖 Generated with Claude Code

The plan summary is the screen the user reads and approves before any disk is
touched, and it was the least testable thing in the product: about seventy
lines of inline Write-Host in the linear body, 27 print calls, 13 branches, 28
variables, and the mode tested three times in two different shapes. The suite
lifts functions out of the syntax tree and cannot reach the body, so nothing
could check what a line said - only that some string sat somewhere.

Format-CreationPlan now returns one object per line, carrying its text and the
colour it prints in. The body assembles the answers and prints what comes back.

Answers travel as a hashtable rather than an object with fields: mode-specific
variables are assigned only inside the branch that asked for them, and under
strict mode reading an unset variable throws. Pre-initialising them would print
values nobody chose, which the repository forbids. So a key exists only where
its branch asked the question.

What can be asserted now, and could not be before:

- a mode's plan names only what that mode does
- a skipped step says it is skipped rather than being absent
- the size, the label, the disk and the file are the ones answered
- the write-access warning reaches the plan, line by line, in yellow
- every kind of line carries the colour that kind is printed in
- the keys the body sets, the function reads, and the fixture builds agree

Seven existing tests anchored on the block's position and were retargeted; four
became assertions on the function instead. One of them, counting calls to
Format-DedupModeChoice, had never counted the fourth.

Every new assertion was proved to have teeth by mutating the source in place and
running the suite against it - a leaked branch, a vanished step, a hardcoded
size, a dropped key, a stray key, a warning turned white, and the write-access
warning removed. All seven were caught, and both files restored byte for byte.

Closes #79

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ALERTua
ALERTua merged commit 559bf8e into main Aug 26, 2026
3 checks passed
@ALERTua
ALERTua deleted the bugfix/79 branch August 26, 2026 13:32
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.

The plan the user approves is inline Write-Host and cannot be tested

1 participant