The first green CI run for OrphanedSid, at the end of the last post, came with a coverage gate I’d just lowered. The workflow wanted 80%. The suite measured 62.28%:

1

Covered 62.28% / 80%. 5,170 analyzed Commands in 60 Files.

The suite behind that number had more than 2,400 passing tests. A module with three public commands and 57 private function files shouldn’t be able to hide a third of itself from that many tests, so I went looking for where they were pointed.

Getting 62% out of that many tests made me real sad.

Tests of a copy

The first answer came quickly. Six test files defined their own version of the function they were named for, and then tested that. This is the top of Set-ModifiedACL.Tests.ps1 before the fix:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11

# Mock function to simulate Set-ModifiedACL behavior
# Helper function to create test ACL
BeforeAll {
    function Set-ModifiedACL {
        [CmdletBinding()]
        param(
            [Parameter()]
            [string]$ObjectDN,

            [Parameter()]
            [PSObject]$ModifiedACL,

Every test in that file called the copy above, never the Set-ModifiedACL in Private/ACL. The same was true for Get-ACLForRemoval, Invoke-SIDRemoval, New-ACLBackup, Initialize-ScriptExecution and Remove-OrphanedSidEntry. They all passed, and none of them could tell you anything about the module.

The fix commit made each of those files dot-source the real function and mock only what it calls. It also added tests for functions the public commands run that had none at all: the session start and stop, the per-object detector, the identity resolution helpers, the restore helpers, Test-BackupIntegrity and the processing summary. Coverage went to 82%, and the gate went back to 80% in time for 1.0.0.

Pointing tests at the real code found defects straight away. My favorite is in Invoke-RemovalWorkflow, which checked whether Set-ModifiedACL had worked like this:

1

if ($applicationSuccess) {

Despite the name, $applicationSuccess held the result object Set-ModifiedACL returns, with a Success property. Any object is truthy in PowerShell, including one whose Success is $false, so a failed write was reported as applied. It reads the property now:

1
2
3

# Set-ModifiedACL returns an ACLApplicationResult, which is truthy even when the write
# failed; only its Success property says whether the ACL was applied
if ($applicationSuccess.Success) {

The others were Remove-OrphanedSidEntry and Find-OrphanedSIDsInObject returning from inside their foreach after the first object, a log rotation reported as done when Move-Item had failed, and a removal count that came out empty on Windows PowerShell 5.1 when exactly one object matched.

A type only RSAT supplies

Test-OrphanedSID catches [Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException]. On a machine without RSAT, and that includes every CI runner, that type doesn’t exist, and a catch naming an unknown type fails with Unable to find type. Inside Test-OrphanedSID, that failure lands in the outer catch-all, and the catch-all treated it as orphaned. So in the test suite, every SID looked orphaned whatever the mocks said.

The shared stub file now defines a stand-in for the exception when RSAT is absent, the same way it stands in for the cmdlets in the Pester migration post.

Memory tests that measured the garbage collector

Two tests had a memory budget: process a small batch of strings, use less than 50 MB. On the 5.1 runner they measured 58 MB and 60 MB, which is a lot of memory for some strings. Both read the “before” figure with a collection and the “after” figure without one:

1

$afterMemory = [System.GC]::GetTotalMemory($false)

The difference between those two numbers was mostly garbage that hadn’t been collected yet, so the test measured when the collector last ran. Collecting on both sides measures what the code kept:

1
2
3

# Collected on both sides: the uncollected figure is garbage-collector timing, and on the
# Windows PowerShell 5.1 runner it measured 58 MB for 50 strings
$afterMemory = [System.GC]::GetTotalMemory($true)

Tests that always passed

Coverage at 82% got 1.0.0 out the door. It didn’t mean every covered line was being checked, and two days later I went back through the suite with one question for each test: if the behavior in its name broke, would it fail?

A lot of them wouldn’t. The 13 files under Tests/Integration and Tests/Performance, about 350 tests, never called the module. Each one defined a simulator and asserted on what the simulator returned:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

function Invoke-FindUnknownSIDScript {
    param(
        [string]$SearchBase,
        [string]$OutputPath,
        [string]$BackupPath,
        [string]$CorrelationId,
        [string]$Mode = "Remove",
        [switch]$WhatIf,
        [switch]$Force
    )
    # ...
    if ($WhatIf) {
        return [PSCustomObject]@{
            CorrelationId = $CorrelationId
            SearchBase = $SearchBase
            Mode = "WhatIf"
            OrphanedSIDsFound = 3
            OrphanedSIDsRemoved = 0

The name still says FindUnknownSID. Those files were “integration tests” for a script that had been a module for two days, and they’d have passed if the module were deleted.

The unit tests had smaller versions of the same problem. Some asserted on a constant:

1
2
3

It "Should -be able to run basic tests" {
    $true | Should -Be $true
}

Some caught their own failure. In Restore-ACLOperation.Tests.ps1, a failed call-count assertion was caught and replaced with a check that couldn’t fail on its own:

1
2
3
4
5
6

try {
    Should -Invoke Confirm-RestorationSuccess -Times 0 -Scope It
} catch {
    # A failed call-count assertion is swallowed, so only WhatIfMode decides this test
    $result.WhatIfMode | Should -Be $true
}

Others had no assertion at all, put timing budgets on calls that take microseconds, or tested an in-file copy like the six above.

What replaced them

The 13 simulator files became one, Tests/Integration/OrphanedSidWorkflow.Tests.ps1. It imports the module and runs Find-OrphanedSid, Remove-OrphanedSid and Restore-OrphanedSidBackup the way you would. Only the boundary is mocked: the AD cmdlets, plus Get-Acl and Set-Acl on AD:\ paths, read and write a hashtable of objects whose security descriptors are real ActiveDirectorySecurity instances.

The fake directory has a small cast of trustees, each there to prove one thing:

1
2
3
4
5
6

$script:LiveSid = "$script:DomainSid-1105"
$script:OrphanSid = "$script:DomainSid-1999"
$script:SecondOrphanSid = "$script:DomainSid-1998"
$script:FailingSid = "$script:DomainSid-1777"
# A live principal whose ordinary lookups time out; only the deleted-objects lookup answers (#39)
$script:FlakySid = "$script:DomainSid-1300"

The SID lookup behind every AD cmdlet mock answers for those trustees, and throws when it’s supposed to:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14

function Invoke-SidLookup {
    param([string]$Sid)
    if ($Sid -eq $script:FailingSid) {
        throw [System.UnauthorizedAccessException]::new('Access is denied.')
    }
    if ($Sid -eq $script:FlakySid) {
        throw [System.TimeoutException]::new('The operation returned because the timeout limit was exceeded.')
    }
    # ...
    if ($Sid -eq $script:LiveSid) {
        return [PSCustomObject]@{ DistinguishedName = "CN=Alice,OU=Users,$script:DomainDN"; ObjectClass = 'user'; SID = $Sid }
    }
    return $null
}

Assertions are on what the commands return, what’s left in the directory afterward, and what lands on disk. A test like Keeps the live, built-in and unverified ACEs reads the security descriptors back out of the hashtable after a removal. If the module removed the wrong entry, the descriptor would show it.

The unit tests were rewritten in four batches by area, each file read against the code it tests. Assertions check exact values, messages and the parameters passed to collaborators. Hand-written call recorders from the Pester 3 days became Mock and Should -Invoke with exact counts, and fixtures moved from fixed %TEMP% folders to TestDrive.

Skipping the tests that found bugs

Once the tests could fail, some of them did, because the module was wrong. Those kept their correct assertion and got -Skip, with a comment naming the issue:

1
2

# Module bug (#14): GetAllResults flushes (which clears CurrentResults) and then returns CurrentResults in WhatIf mode, so it always returns nothing
It "Should return every added result in WhatIf mode" -Skip {

That kept the test rewrite separate from the module fixes, and it meant each fix came with a test ready to un-skip. The rewrite filed twenty issues that way, #12 through #31. One is an old friend:

1
2

# Module bug (#17): the foreach variable $backupFile overwrites the -BackupFile parameter (names are
# case-insensitive), so every piped DN after the first restores from the first DN's backup

Case-insensitive names had bitten a loop variable before, and here they were again in a different file. It sat in a code path no public command reaches. That’s the only reason it never showed up in the lab.

Checking that the tests can fail

“Every test can fail” is easy to claim. To check it, I broke module behaviors one at a time in a scratch worktree and ran the suite against each:

  • removal taking unverified SIDs
  • a failed lookup counting as orphaned
  • the backup hash not being compared
  • a backup written without a hash
  • an ACE not being removed
  • the safe file name length changing
  • an AD failure not being audited

At least one test failed every time. Before the rewrite, the simulator files would’ve passed every one of them.

The numbers

The rewrite took the suite from 2,498 tests to 1,738, and line coverage over Public/ and Private/ from 82.4% to 93.2%. The gate went to 90%.

By 1.1.0, with the twenty skipped issues fixed and a few dozen unused functions deleted along with their tests, it’s 1,374 tests at 94.07%:

1
2

Tests Passed: 1374, Failed: 0, Skipped: 0, Inconclusive: 0, NotRun: 0
Covered 94.07% / 80%. 3,743 analyzed Commands in 50 Files.

That 80% is the workflow’s CoveragePercentTarget, which only sets what Pester reports. The gate that fails the job is a separate check, and it’s 90.

I think those in-file copies were the result of the early GitHub Copilot harness, from before I established my ai-powershell-standards repo. The difference between generic LLM vs focused is insane. Whether it’s mine or your own, I can’t imagine going back to an agent without a standard or style guide.