Should-BeFasterThan and Should-BeSlowerThan dispatched on the type of Actual, handled [scriptblock] and [timespan], and both branches ended in return. Anything else fell out of the bottom of the function, so the assertion returned having asserted nothing and the test passed:
Actual = [String] -> SILENTLY PASSED
Actual = [Int32] -> SILENTLY PASSED
Actual = [null] -> SILENTLY PASSED
Actual = [Object[]] -> SILENTLY PASSED
#3032 added the missing branch to both. Nothing prevents the next assertion from being written the same way.
Detecting it
The shape is distinguishable from a normal assertion with the AST. Most assertions correctly end with if ($Actual -ne $Expected) { $assert.Fail(...) }, where falling through means the assertion passed. The defective shape is a trailing if with no else whose branch contains a return: a return means "this branch fully handled the input", so falling past it means the input was never handled at all.
Parsing every file under src/functions/assert and applying that rule finds the two cases and nothing else:
scanned 52 assertion files
Should-BeFasterThan Should-BeFasterThan.ps1:73 trailing: if ($Actual -is [timespan])
Should-BeSlowerThan Should-BeSlowerThan.ps1:80 trailing: if ($Actual -is [timespan])
No false positives across the other 50 files.
Where it belongs
A meta test, next to the ones in tst/Pester.Tests.ps1 that already assert properties of the source rather than behaviour, for example Set-StrictMode for all tests files. It needs no tooling beyond [System.Management.Automation.Language.Parser], and it fails naming the function and line.
Not a PSScriptAnalyzer rule.
Why it matters beyond the two cases
A silent pass is worse than a wrong assertion message. It also hid a CI flake for a long time: Should-BeFasterThan.Throws when scriptblock is slower than expected failed on Windows PS7 with "no assertion failure error was thrown", and the whole test took 3ms while its scriptblock is { Start-Sleep -Milliseconds 10 }. The scriptblock was never run, and because nothing reported that, it read as a scriptblock that ran impossibly fast, and got treated as a timing flake.
Related
Should-BeFasterThanandShould-BeSlowerThandispatched on the type ofActual, handled[scriptblock]and[timespan], and both branches ended inreturn. Anything else fell out of the bottom of the function, so the assertion returned having asserted nothing and the test passed:#3032 added the missing branch to both. Nothing prevents the next assertion from being written the same way.
Detecting it
The shape is distinguishable from a normal assertion with the AST. Most assertions correctly end with
if ($Actual -ne $Expected) { $assert.Fail(...) }, where falling through means the assertion passed. The defective shape is a trailingifwith noelsewhose branch contains areturn: areturnmeans "this branch fully handled the input", so falling past it means the input was never handled at all.Parsing every file under
src/functions/assertand applying that rule finds the two cases and nothing else:No false positives across the other 50 files.
Where it belongs
A meta test, next to the ones in
tst/Pester.Tests.ps1that already assert properties of the source rather than behaviour, for exampleSet-StrictMode for all tests files. It needs no tooling beyond[System.Management.Automation.Language.Parser], and it fails naming the function and line.Not a PSScriptAnalyzer rule.
Why it matters beyond the two cases
A silent pass is worse than a wrong assertion message. It also hid a CI flake for a long time:
Should-BeFasterThan.Throws when scriptblock is slower than expectedfailed on Windows PS7 with "no assertion failure error was thrown", and the whole test took 3ms while its scriptblock is{ Start-Sleep -Milliseconds 10 }. The scriptblock was never run, and because nothing reported that, it read as a scriptblock that ran impossibly fast, and got treated as a timing flake.Related