Skip to content

Dynamically count expected rules in GetScriptAnalyzerRule test#2167

Merged
andyleejordan merged 1 commit intomainfrom
fix-dynamic-rule-count
Mar 18, 2026
Merged

Dynamically count expected rules in GetScriptAnalyzerRule test#2167
andyleejordan merged 1 commit intomainfrom
fix-dynamic-rule-count

Conversation

@andyleejordan
Copy link
Member

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 March 18, 2026 01:24
@andyleejordan andyleejordan requested review from a team and bergmeister as code owners March 18, 2026 01:24
Copy link
Contributor

Copilot AI left a comment

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 scanning Rules/**/*.cs for Export(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
Copy link
Member Author

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.

Copy link
Member Author

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)

@andyleejordan andyleejordan enabled auto-merge (squash) March 18, 2026 01:32
@andyleejordan andyleejordan merged commit f3e9134 into main Mar 18, 2026
11 checks passed
@andyleejordan andyleejordan deleted the fix-dynamic-rule-count branch March 18, 2026 17:04
Copy link
Collaborator

@bergmeister bergmeister left a comment

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?

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.

4 participants