Performance: Avoid Use of Array Addition #162
Conversation
WalkthroughThis change set primarily introduces suppression of the 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule across multiple PowerShell test scripts, with an additional suppression for 'PSAvoidUsingConvertToSecureStringWithPlainText' in one script. Several scripts refactor array construction and collection logic for test cases, replacing array concatenation with strongly-typed lists or direct foreach expressions. The Azure pipeline YAML is updated to remove a patching step and standardize quoting and formatting. Changes
Sequence Diagram(s)Not applicable—changes are limited to code style, suppression attributes, and collection construction refactoring without affecting control flow or introducing new features. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
source/Tests/QA/RelativePathLength.common.v5.Tests.ps1 (1)
86-86: Fix variable reference bug.The variable
$fullPathHardLimitis referenced but should be$relativePathHardLimitbased on the variable defined on line 82.- $RelativePath.Length | Should -Not -BeGreaterThan $relativePathHardLimit -Because ('for the module to be able to be downloaded and used in Azure Automation the max lengths of the relative paths of the modules files much not be greater than {0} characters including path separators' -f $fullPathHardLimit) + $RelativePath.Length | Should -Not -BeGreaterThan $relativePathHardLimit -Because ('for the module to be able to be downloaded and used in Azure Automation the max lengths of the relative paths of the modules files much not be greater than {0} characters including path separators' -f $relativePathHardLimit)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.md(1 hunks)azure-pipelines.yml(6 hunks)source/Tests/QA/Changelog.common.v5.Tests.ps1(2 hunks)source/Tests/QA/ExampleFiles.common.v5.Tests.ps1(2 hunks)source/Tests/QA/FileFormatting.common.v5.Tests.ps1(3 hunks)source/Tests/QA/MarkdownLinks.common.v5.Tests.ps1(3 hunks)source/Tests/QA/ModuleManifest.common.v5.Tests.ps1(5 hunks)source/Tests/QA/ModuleScriptFiles.common.v5.Tests.ps1(3 hunks)source/Tests/QA/PSSAResource.common.v5.Tests.ps1(2 hunks)source/Tests/QA/PublishExampleFiles.common.v5.Tests.ps1(4 hunks)source/Tests/QA/RelativePathLength.common.v5.Tests.ps1(2 hunks)source/Tests/QA/ResourceSchema.common.v5.Tests.ps1(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...anAssignments' ScriptAnalyzer rule. - Suppress 'PSAvoidUsingConvertToSecureStringWithP...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: dsccommunity.DscResource.Test (Test Publish Code Coverage)
- GitHub Check: dsccommunity.DscResource.Test (Test Test Windows (WPS))
- GitHub Check: dsccommunity.DscResource.Test (Test Test Windows (PS))
- GitHub Check: dsccommunity.DscResource.Test (Test Test Linux)
- GitHub Check: dsccommunity.DscResource.Test (Test Test macOS)
- GitHub Check: dsccommunity.DscResource.Test (Test Integration)
- GitHub Check: dsccommunity.DscResource.Test (Build Package Module)
🔇 Additional comments (31)
azure-pipelines.yml (4)
4-4: LGTM! Improved indentation consistency.The indentation fixes in the trigger section improve YAML readability and consistency.
Also applies to: 7-7
10-10: LGTM! Consistent quote usage.Converting to single quotes for tag patterns improves consistency across the YAML configuration.
Also applies to: 12-12
75-75: LGTM! Consistent quote usage in PowerShell arguments.The change from double quotes to single quotes in PowerShell task arguments improves consistency throughout the pipeline configuration.
Also applies to: 107-107, 139-139, 171-171
223-226: LGTM! Improved dependency list formatting.The increased indentation for the dependency list enhances readability while maintaining the same semantic meaning.
source/Tests/QA/ResourceSchema.common.v5.Tests.ps1 (1)
15-15: LGTM! Appropriate static analysis suppression.The suppression of
PSUseDeclaredVarsMoreThanAssignmentsis appropriate here, as this test script likely has variables that are used by the Pester framework in ways that static analysis cannot detect.source/Tests/QA/Changelog.common.v5.Tests.ps1 (2)
15-15: LGTM! Consistent static analysis suppression.The suppression attribute follows the same pattern applied across other test files in this PR to reduce static analysis noise.
57-57: LGTM! Consistent quote usage.The change from double quotes to single quotes in the Write-Debug statement improves consistency with PowerShell best practices.
source/Tests/QA/ExampleFiles.common.v5.Tests.ps1 (2)
14-15: LGTM! Appropriate static analysis suppressions.Both suppression attributes are justified:
PSUseDeclaredVarsMoreThanAssignmentsfor variables used by Pester frameworkPSAvoidUsingConvertToSecureStringWithPlainTextfor test scenarios where mock credentials are needed
67-73: Excellent performance improvement!Replacing the array concatenation loop with a
foreachexpression eliminates the performance penalty of PowerShell's immutable array+=operations. This approach builds the collection efficiently in a single operation rather than creating new arrays on each iteration.source/Tests/QA/FileFormatting.common.v5.Tests.ps1 (3)
18-18: LGTM! Consistent static analysis suppression.The suppression attribute follows the same pattern applied across other test files to reduce static analysis noise.
67-67: Excellent performance improvement with strongly-typed collections!Replacing PowerShell arrays with
System.Collections.Generic.List[System.Object]and usingAddRangemethods provides significant performance benefits:
- Avoids the overhead of PowerShell's immutable array operations
AddRangeis more efficient than repeated array concatenation- Strongly-typed collections offer better performance characteristics
This change directly addresses the PR objective of avoiding array addition operations.
Also applies to: 69-69, 73-73
87-94: Outstanding performance optimization!The refactor from array concatenation loop to a
foreachexpression that directly outputs hashtables eliminates the performance penalty of PowerShell's+=operator on arrays. This approach:
- Builds the collection in a single efficient operation
- Avoids creating new arrays on each iteration
- Results in cleaner, more readable code
Perfect alignment with the PR's performance improvement objectives.
source/Tests/QA/MarkdownLinks.common.v5.Tests.ps1 (4)
18-18: LGTM: Valid suppression for Pester test context.The suppression attribute correctly addresses the static analysis warning for variables that appear unused but are actually utilized by Pester's test framework.
86-90: Excellent performance optimization!Replacing array concatenation (
+=) withSystem.Collections.Generic.List[System.Object]andAddRangeeliminates the overhead of array copying that occurs with each+=operation. This is a significant performance improvement when processing multiple markdown files.
96-103: LGTM: Cleaner collection construction.The direct
foreachexpression that outputs hashtables is more idiomatic PowerShell and eliminates the overhead of intermediate array operations. This improves both performance and readability.
126-126: LGTM: Streamlined pipeline operation.Specifying properties directly in
Format-Tableeliminates the unnecessarySelect-Objectpipeline stage, resulting in cleaner and slightly more efficient code.source/Tests/QA/PSSAResource.common.v5.Tests.ps1 (2)
94-101: LGTM: Consistent performance optimization.The direct
foreachexpression eliminates intermediate array operations and aligns with the performance improvements applied consistently across the test suite.
149-149: LGTM: PowerShell best practice.Using single quotes for string literals that don't require variable expansion follows PowerShell best practices and maintains consistency with the codebase.
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1 (3)
15-15: LGTM: Consistent suppression pattern.The suppression attribute follows the same pattern applied across all test files in this PR to address static analysis warnings in Pester test contexts.
60-65: LGTM: Performance improvement applied consistently.The direct
foreachexpression eliminates intermediate array operations, following the same optimization pattern used throughout this PR.
117-119: Good practice: Documenting future improvements.The TODO comments properly document potential future enhancements to the test suite, which helps with maintenance and evolution of the codebase.
source/Tests/QA/RelativePathLength.common.v5.Tests.ps1 (2)
14-14: LGTM: Consistent suppression application.The suppression attribute follows the established pattern for addressing static analysis warnings in Pester test files.
61-67: LGTM: Effective performance optimization.Replacing the
ForEach-Objectpipeline with a directforeachexpression eliminates pipeline overhead and improves performance when processing module files.source/Tests/QA/ModuleScriptFiles.common.v5.Tests.ps1 (4)
18-18: LGTM! Valid suppression for test script variables.The suppression attribute appropriately addresses PSScriptAnalyzer warnings about variables that are assigned in discovery blocks but used in test contexts, which is a common pattern in Pester test files.
67-67: Excellent performance improvement!Converting from an array to
System.Collections.Generic.List[System.Object]eliminates the overhead of array recreation during addition operations, which is a significant performance benefit when building collections dynamically.
69-69: Well-implemented array addition replacement.Using
AddRangewith the array subexpression operator@()efficiently adds multiple items while ensuring correct behavior even when the pipeline returns a single item. This eliminates the performance penalty of array recreation.Also applies to: 73-73
86-93: Perfect refactoring that achieves both PR objectives!This change eliminates array addition operations by leveraging PowerShell's natural collection output from
foreachstatements, while also inlining the collection building. The code is now more performant and idiomatic.source/Tests/QA/PublishExampleFiles.common.v5.Tests.ps1 (4)
15-15: LGTM! Consistent suppression pattern.The suppression attribute matches the pattern used across other test files in this PR and appropriately addresses PSScriptAnalyzer warnings for test script variables.
68-74: Excellent consistency with performance improvements.This refactoring follows the same pattern as other files in the PR, eliminating array addition operations by using PowerShell's natural collection output from
foreachstatements.
93-112: Well-executed pipeline to foreach refactoring.The conversion from pipeline operations to explicit
foreachwith inline hashtable creation improves performance while preserving all error handling logic. The try-catch structure ensures robust handling of files with missing metadata.
127-127: Good simplification of variable assignment.Removing the unnecessary
Select-Objectoperation and directly assigning the result ofTest-ScriptFileInfoeliminates pipeline overhead while maintaining the same functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CHANGELOG.md (1)
18-52: Reduce repetition and fix hyphenation for clarity.Reword bullets to avoid repeated “Suppress …” openings and hyphenate “single-use”. Keeps content intact while improving readability.
-### Changed -- `Changelog.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. -- `ExampleFiles.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Suppress 'PSAvoidUsingConvertToSecureStringWithPlainText' ScriptAnalyzer rule. - - Remove array addition. -- `FileFormatting.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. -- `MarkdownLinks.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. -- `ModuleScriptFiles.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. -- `PSSAResource.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. -- `PublishExampleFiles.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. -- `RelativePathLength.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. -- `ResourceSchema.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. +### Changed +- `Changelog.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. +- `ExampleFiles.common.v5.Tests` + - Added suppressions for 'PSUseDeclaredVarsMoreThanAssignments' and 'PSAvoidUsingConvertToSecureStringWithPlainText'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `FileFormatting.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `MarkdownLinks.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `ModuleScriptFiles.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `PSSAResource.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `PublishExampleFiles.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `RelativePathLength.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array addition with foreach/list-based construction. + - Inlined single-use variables. +- `ResourceSchema.common.v5.Tests` + - Added suppression for 'PSUseDeclaredVarsMoreThanAssignments'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)source/DscResource.Test.psm1(1 hunks)source/Tests/QA/ExampleFiles.common.v4.Tests.ps1(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~10-~10: Use correct spacing
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Fixed - Add ability to filter out files in `Exam...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~12-~12: Use correct spacing
Context: ... files in ExampleFiles.common.v4 #120. ### Removed - Remove ModuleBuilder patch. ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~16-~16: Use correct spacing
Context: ...). ### Removed - Remove ModuleBuilder patch. ### Changed - Changelog.common.v5.Tests ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~21-~21: Use articles correctly
Context: ...nged - Changelog.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~21-~21: There might be a problem here.
Context: ...VarsMoreThanAssignments' ScriptAnalyzer rule. - ExampleFiles.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...anAssignments' ScriptAnalyzer rule. - Suppress 'PSAvoidUsingConvertToSecureStringWithP...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~25-~25: There might be a problem here.
Context: ...' ScriptAnalyzer rule. - Remove array addition. - FileFormatting.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~29-~29: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `MarkdownLinks.common.v5...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~29-~29: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - MarkdownLinks.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~33-~33: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `ModuleScriptFiles.commo...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~33-~33: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - ModuleScriptFiles.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~37-~37: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `PSSAResource.common.v5....
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~37-~37: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - PSSAResource.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~41-~41: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `PublishExampleFiles.com...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~41-~41: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - PublishExampleFiles.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~45-~45: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `RelativePathLength.comm...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~45-~45: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - RelativePathLength.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~49-~49: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `ResourceSchema.common.v...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~49-~49: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - ResourceSchema.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~51-~51: Use correct spacing
Context: ...VarsMoreThanAssignments' ScriptAnalyzer rule. ## [0.17.2] - 2025-02-28 ### Fixed - Now ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (1)
source/Tests/QA/ExampleFiles.common.v4.Tests.ps1 (1)
31-31: Add –File and deterministic sorting to the pipelineTo ensure the test only returns files (not directories) and runs in a stable order, update line 31 as follows and please verify that
WhereSourceFileNotExcludedgracefully handles a null/empty$ExcludeSourceFile:- $examples = Get-ChildItem -Path $examplesPath -Filter '*.ps1' -Recurse | WhereSourceFileNotExcluded -ExcludeSourceFile $ExcludeSourceFile + $examples = Get-ChildItem -Path $examplesPath -Filter '*.ps1' -Recurse -File | + WhereSourceFileNotExcluded -ExcludeSourceFile $ExcludeSourceFile | + Sort-Object -Property FullName• Use
-Fileto exclude directories up front.
• Pipe toSort-Object FullNamefor a deterministic ordering in your tests.
• Manually confirm thatWhereSourceFileNotExcludedaccepts an empty or$null-ExcludeSourceFile; if not, wrap the call in anif ($ExcludeSourceFile) { … }check.
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Reviewed 11 of 12 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dan-hughes)
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1 line 117 at r2 (raw file):
} #TODO: Add checks for public functions and aliases.
Create an issue for this and remove comment?
Code quote:
#TODO: Add checks for public functions and aliases.source/Tests/QA/ModuleManifest.common.v5.Tests.ps1 line 119 at r2 (raw file):
#TODO: Add checks for public functions and aliases. #TODO: Should cmdlets be exported be set to '*'?
Suggested in PR #165, we can remove comment.
Code quote:
#TODO: Should cmdlets be exported be set to '*'?
johlju
added
waiting for code fix
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
CHANGELOG.md (1)
24-56: Reduce repetition, fix hyphenation, and align tone with Keep a Changelog styleReword sub-bullets to past-tense “Added/Removed,” hyphenate “single-use,” and avoid repeated “Suppress …” phrasing. This also addresses the LanguageTool flags and improves readability.
- - `Changelog.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. + - `Changelog.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. - - `ExampleFiles.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Suppress 'PSAvoidUsingConvertToSecureStringWithPlainText' ScriptAnalyzer rule. - - Remove array addition. + - `ExampleFiles.common.v5.Tests` + - Added suppressions for ScriptAnalyzer rules 'PSUseDeclaredVarsMoreThanAssignments' and 'PSAvoidUsingConvertToSecureStringWithPlainText'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. - - Remove single use variable. + - Inlined single-use variables. - - `FileFormatting.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. + - `FileFormatting.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. + - Inlined single-use variables. - - `MarkdownLinks.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. + - `MarkdownLinks.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. + - Inlined single-use variables. - - `ModuleScriptFiles.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. + - `ModuleScriptFiles.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. + - Inlined single-use variables. - - `PSSAResource.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. + - `PSSAResource.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. + - Inlined single-use variables. - - `PublishExampleFiles.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. + - `PublishExampleFiles.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. + - Inlined single-use variables. - - `RelativePathLength.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. - - Remove array addition. - - Remove single use variable. + - `RelativePathLength.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'. + - Replaced array additions (+=) with strongly-typed lists or pipeline output for performance. + - Inlined single-use variables. - - `ResourceSchema.common.v5.Tests` - - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ScriptAnalyzer rule. + - `ResourceSchema.common.v5.Tests` + - Added suppression for ScriptAnalyzer rule 'PSUseDeclaredVarsMoreThanAssignments'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~25-~25: Use articles correctly
Context: ...nged - Changelog.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~25-~25: There might be a problem here.
Context: ...VarsMoreThanAssignments' ScriptAnalyzer rule. - ExampleFiles.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...anAssignments' ScriptAnalyzer rule. - Suppress 'PSAvoidUsingConvertToSecureStringWithP...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~29-~29: There might be a problem here.
Context: ...' ScriptAnalyzer rule. - Remove array addition. - FileFormatting.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~33-~33: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `MarkdownLinks.common.v5...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~33-~33: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - MarkdownLinks.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~37-~37: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `ModuleScriptFiles.commo...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~37-~37: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - ModuleScriptFiles.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~41-~41: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `PSSAResource.common.v5....
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~41-~41: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - PSSAResource.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~45-~45: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `PublishExampleFiles.com...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~45-~45: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - PublishExampleFiles.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~49-~49: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `RelativePathLength.comm...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~49-~49: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - RelativePathLength.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~53-~53: Use hyphens correctly
Context: .... - Remove array addition. - Remove single use variable. - `ResourceSchema.common.v...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~53-~53: There might be a problem here.
Context: ...e array addition. - Remove single use variable. - ResourceSchema.common.v5.Tests - Suppress 'PSUseDeclaredVarsMoreThanAssignments' ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~55-~55: Use correct spacing
Context: ...VarsMoreThanAssignments' ScriptAnalyzer rule. ## [0.17.2] - 2025-02-28 ### Fixed - Now ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #162 +/- ##
===================================
Coverage 80% 80%
===================================
Files 42 42
Lines 554 554
===================================
Hits 447 447
Misses 107 107 🚀 New features to boost your workflow:
|
dan-hughes
left a comment
dan-hughes
left a comment
There was a problem hiding this comment.
Reviewable status: 10 of 14 files reviewed, 4 unresolved discussions (waiting on @johlju)
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1 line 117 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Create an issue for this and remove comment?
Done.
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1 line 119 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Suggested in PR #165, we can remove comment.
Done.
|
@johlju, issue raised and TODOs removed. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-09T19:29:36.323Z
Learnt from: johlju
PR: dsccommunity/DscResource.Test#167
File: source/Private/Test-FileContainsClassResource.ps1:44-56
Timestamp: 2025-08-09T19:29:36.323Z
Learning: In the DscResource.Test repository, DSC resource attributes are consistently written as `[DscResource(...)]` rather than using variations like `[DscResourceAttribute()]` or fully qualified names like `[Microsoft.PowerShell.DesiredStateConfiguration.DscResource()]`. The Test-FileContainsClassResource function should focus on detecting the standard `[DscResource(...)]` pattern that is actually used in the codebase.
Applied to files:
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: dsccommunity.DscResource.Test (Build Package Module)
🔇 Additional comments (2)
source/Tests/QA/ModuleManifest.common.v5.Tests.ps1 (2)
91-91: LGTM: Style consistencySwitch to single quotes in the It name is consistent with the repo’s style updates. No issues.
107-107: LGTM: Per-test description consistencySingle-quoted It string aligns with the style changes. The assertion remains clear and correct.
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dan-hughes)
johlju
removed
the
waiting for code fix
2 participants
Pull Request (PR) description
Remove use of addition operation on arrays.
Also single use variables have been inlined.
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed and how that affects users (if applicable), and
reference the issue being resolved (if applicable).
This change is