Dynamically count expected rules in GetScriptAnalyzerRule test by andyleejordan · Pull Request #2167 · PowerShell/PSScriptAnalyzer
Conversation
Had Copilot do this:
Replace hardcoded rule count with dynamic counting of
[Export(typeof(I...Rule))]attributes in C# source files. This prevents the test from breaking every time a new rule is added.
Seems like it works. I noticed every time a PR that added a rule was merged, all PRs after it failed because this test had a hardcoded assumption.
Replace hardcoded rule count with dynamic counting of [Export(typeof(I...Rule))] attributes in C# source files. This prevents the test from breaking every time a new rule is added.
Copilot AI review requested due to automatic review settings
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Updates the Get-ScriptAnalyzerRule Pester test to avoid hardcoding the expected number of built-in rules by deriving the expected count dynamically from the repository’s C# rule sources.
Changes:
- Replaces the fixed expected rule count (
72) with a dynamic count based on scanningRules/**/*.csforExport(typeof(I*Rule))occurrences. - Adds comments explaining the dynamic rule-counting approach.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines +66 to +71
| # Dynamically count the expected number of rules from source files | ||
| # by finding all C# files with [Export(typeof(I...Rule))] attributes | ||
| $rulesRoot = Resolve-Path "$PSScriptRoot/../../Rules" | ||
| $expectedNumRules = (Get-ChildItem -Path $rulesRoot -Filter '*.cs' -Recurse | | ||
| Select-String -Pattern 'Export\(typeof\s*\(I\w+Rule\)\)' | | ||
| Select-Object -ExpandProperty Path -Unique).Count |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow bad Copilot, that suggestion would mean that we don't end up testing anything at all.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Claude did the actual PR...lol)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition but it can still be of value to keep a hard=coded check (in addition to this), how about we make it less troublesome by just asserting greater than instead of equals so this test doesn't need updating for every new rule?
