New-SqlDscLogin: command to create SQL Server logins#2132
New-SqlDscLogin: command to create SQL Server logins#2132johlju merged 61 commits intodsccommunity:mainfrom
New-SqlDscLogin: command to create SQL Server logins#2132Conversation
WalkthroughAdds a new public PowerShell helper New-SqlDscLogin with unit and integration tests, localization strings, docs and CI wiring; updates Remove-SqlDscLogin integration tests to use the helper; extends SMO test stubs; replaces several New-InvalidArgumentException calls with New-ArgumentException; and updates editor/CI/guideline files plus .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Cmd as New-SqlDscLogin
participant Exists as Test-SqlDscIsLogin
participant SMO as SMO.Server/Login
User->>Cmd: Invoke with ServerObject, Name, parameter-set...
Cmd->>Exists: Test if login exists
Exists-->>Cmd: Exists? (true/false)
alt exists == true
Cmd-->>User: Throw terminating error (login exists)
else
Cmd->>User: ShouldProcess prompt (WhatIf/Confirm)
alt proceed
Cmd->>SMO: Create login (type, defaults, options, password)
opt -Disabled
Cmd->>SMO: Call Disable()
end
opt -PassThru
SMO-->>User: Return created Login object
end
else declined / WhatIf
Cmd-->>User: No change (WhatIf/declined)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)azure-pipelines.yml📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
**⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
📚 Learning: 2025-08-17T09:48:55.756ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (3)
✨ 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/Issue comments)Type 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: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
.gitignore(1 hunks).vscode/tasks.json(2 hunks)CHANGELOG.md(1 hunks)source/Public/New-SqlDscLogin.ps1(1 hunks)source/en-US/SqlServerDsc.strings.psd1(1 hunks)tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Prerequisites.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/README.md(4 hunks)tests/Unit/Public/New-SqlDscLogin.Tests.ps1(1 hunks)tests/Unit/Stubs/SMO.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
tests/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/**/*.ps1: All tests should use the Pester framework and use Pester v5.0 syntax.
Parameter validation should never be tested.
Test code should never be added outside of the Describe block.
There should only be one Pester Describe block per test file, and the name of the Describe block should be the same as the name of the public command, private function, or class-based resource being tested.
Each scenario or code path being tested should have its own Pester Context block that starts with the phrase 'When'.
Pester It block descriptions should start with the phrase 'Should'.
It blocks must always call the command or function being tested and result and outcomes should be kept in the same It block.
BeforeAll and BeforeEach blocks should never call the command or function being tested.
BeforeAll, BeforeEach, AfterAll and AfterEach blocks should be used inside the Context block as near as possible to the It block that will use the test data, test setup and teardown.
AfterAll block can be used to clean up any test data.
BeforeEach and AfterEach blocks should be used sparingly.
It is okay to duplicate code in BeforeAll and BeforeEach blocks that are used inside different Context blocks.
Use localized strings in the tests only when necessary.
Files that need to be mocked should be created in Pester's test drive using the $TestDrive variable.
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Integration tests for public commands should be placed in tests/Integration/Commands and named after the command with the suffix .Integration.Tests.ps1.
All integration tests must use the provided code block prior to the first Describe block to set up the integration test environment.
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.ps1: All PowerShell files should use UTF8 without BOM.
All PowerShell files must end with a new line.
Try to limit PowerShell code lines to 120 characters.
Use 4 spaces for indentation in PowerShell files, never use tabs.
Use '#' for single line comments in PowerShell code.
Use comment-blocks for multiline comments with the format <# ... #> where the multiline text is indented 4 spaces.
Use descriptive, clear, and full names for all variables, parameters, and function names. All names must be more than 2 characters. No abbreviations should be used.
Use camelCase for local variable names in PowerShell.
Use Write-Error for error messages.
Use Write-Warning for warning messages.
Use Write-Information for informational messages.
Never use Write-Host.
Never use backtick as line continuation in code.
Use splatting for commands to reduce line length.
PowerShell reserved keywords should be in all lower case.
Single quotes should always be used to delimit string literals wherever possible. Double quoted string literals may only be used when string literals contain ($) expressions that need to be evaluated.
Hashtables properties should always be in PascalCase and be on a separate line.
When comparing a value to $null, $null should be on the left side of the comparison.
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/Public/New-SqlDscLogin.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.md: Markdown files should wrap lines after a word when a line exceeds 80 characters.
Use 2 spaces for indentation in Markdown files.
Files:
tests/Integration/Commands/README.mdCHANGELOG.md
tests/Unit/{Classes,Public,Private}/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Unit tests should be added for all public commands, private functions and class-based resources. Unit tests for class-based resources should be placed in tests/Unit/Classes, for public commands in tests/Unit/Public, and for private functions in tests/Unit/Private. The unit tests should be named after the item being tested with the suffix .Tests.ps1.
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/Public/*.Tests.ps1: All public commands should always have a test to validate parameter sets using the provided template.
All public commands should also include tests to validate parameter properties.
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/**/*.ps1: Never test, mock or use Should -Invoke for Write-Verbose and Write-Debug.
Never use Should -Not -Throw in tests; instead, call the command directly.
All unit tests should use the provided code block prior to the Describe block to set up the test environment and load the correct module.
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
source/en-US/SqlServerDsc.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
For public commands and private functions, all localized strings should be added in source/en-US/SqlServerDsc.strings.psd1, and localized string key names should be prefixed with the function name using underscores.
Files:
source/en-US/SqlServerDsc.strings.psd1
source/en-US/*.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
For class-based resources, localized strings should be added in a separate file in source/en-US, named to exactly match the resource class name with the suffix .strings.psd1.
Files:
source/en-US/SqlServerDsc.strings.psd1
source/en-US/*.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Localized string key names should use underscores as word separators if the key name has more than one word.
Files:
source/en-US/SqlServerDsc.strings.psd1
source/Public/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/Public/*.ps1: Public PowerShell commands should each have their own script file named after the command (with .ps1 extension) and be placed in the folder source/Public.
All public command names must have the noun prefixed with 'SqlDsc', e.g. {Verb}-SqlDsc{Noun}.
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private,Classes}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private,Classes}/*.ps1: Comment-based help should always be before the function-statement for each public command and private function, and before the class-statement for each class-based resource. It should be in the format of a comment block and at least use the keywords: .SYNOPSIS, .DESCRIPTION, .PARAMETER, .EXAMPLE, and .NOTES.
Each comment-based help keyword should be indented with 4 spaces and each keyword's text should be indented 8 spaces.
The text for keyword .DESCRIPTION should be descriptive and must have a length greater than 40 characters. The .SYNOPSIS keyword text should be a short description.
A comment-based help must have at least one example, but preferably more examples to showcase all possible parameter sets and different parameter combinations.
Use PascalCase for function names and parameters in public commands, private functions, and class-based resources.
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private}/*.ps1: All message strings for Write-Debug, Write-Verbose, Write-Error, Write-Warning and other error messages in public commands and private functions should be localized using localized string keys.
All public command and private function names must follow the standard PowerShell Verb-Noun format.
All public command and private function names must use PowerShell approved verbs.
All public commands and private functions should always be advanced functions and have [CmdLetBinding()] attribute.
Public commands and private functions with no parameters should still have an empty parameter block param ().
Every parameter in public commands and private functions should include the [Parameter()] attribute.
A mandatory parameter in public commands and private functions should contain the decoration [Parameter(Mandatory = $true)], and non-mandatory parameters should not contain the Mandatory decoration.
Parameters attributes, datatype and its name should be on separate lines.
Parameters must be separated by a single, blank line.
Use Write-Verbose to output verbose output for actions a public command or private function does.
Use Write-Debug for debug output for processes within the script for user to see the decisions a command or private function does.
Files:
source/Public/New-SqlDscLogin.ps1
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
The Unreleased section in CHANGELOG.md should always be updated when making changes to the codebase, using the keepachangelog format and providing concrete release notes.
Files:
CHANGELOG.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : For public commands and private functions, all localized strings should be added in source/en-US/SqlServerDsc.strings.psd1, and localized string key names should be prefixed with the function name using underscores.
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to azure-pipelines.yml : Integration test script files for public commands must be added to a group within the 'Integration_Test_Commands_SqlServer' stage in ./azure-pipelines.yml.
Applied to files:
tests/Integration/Commands/README.md
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests for public commands should be placed in tests/Integration/Commands and named after the command with the suffix .Integration.Tests.ps1.
Applied to files:
tests/Integration/Commands/README.md
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : For public commands and private functions, all localized strings should be added in source/en-US/SqlServerDsc.strings.psd1, and localized string key names should be prefixed with the function name using underscores.
Applied to files:
source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : All integration tests must use the provided code block prior to the first Describe block to set up the integration test environment.
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
🪛 LanguageTool
tests/Integration/Commands/README.md
[grammar] ~43-~43: Use correct spacing
Context: ...cLogin | 2 | 1 (Install-SqlDscServer) | DSCSQLTEST ## Integration Tests ### Prerequisites´ ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~60-~60: Use correct spacing
Context: ...o other tests can use it for validation purposes. ## Dependencies ### SqlServer module Ther...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~121-~121: Use correct spacing
Context: ...lDscLogin integration tests for testing purposes. ### Image media (ISO) The environment varia...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (27)
.gitignore (1)
4-4: LGTM: ignoring coverage.xml is appropriatePrevents accidental commits of coverage artifacts produced by test runs.
.vscode/tasks.json (1)
44-44: LGTM: pass -Tasks build explicitlyThis clarifies intent and keeps build.ps1 invocation consistent between tasks.
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1 (1)
47-52: LGTM: adding SqlIntegrationTest user prerequisiteFollows existing pattern in this file and Pester v5 structure. Single quotes, indentation, and It descriptions comply with guidelines.
CHANGELOG.md (1)
26-32: LGTM: prerequisites/tests docs noted under ChangedClear and actionable; reflects the new SqlIntegrationTest user and test ordering updates.
source/en-US/SqlServerDsc.strings.psd1 (1)
231-238: LGTM! Localization strings follow the naming convention correctly.The new localization string keys for
New-SqlDscLoginare properly prefixed with the function name (Login_Add_) using underscores as word separators, which aligns with the established pattern used by other public commands in this file.tests/Integration/Commands/README.md (1)
112-112: LGTM! Documentation accurately describes the test setup.The new user entry for
.\SqlIntegrationTestis properly documented and aligns with its usage in the integration tests.tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1 (5)
1-31: LGTM! Integration test setup follows the required template.The test file correctly implements the required integration test setup code block before the first
Describeblock, including dependency resolution and module import.
32-32: LGTM! Test tags correctly target all SQL Server versions.The
Describeblock properly includes all supported SQL Server version tags for integration testing.
69-110: LGTM! Comprehensive SQL Server login test coverage.The test scenarios thoroughly cover SQL Server login creation including validation, custom database settings, and PassThru functionality with proper cleanup.
158-182: LGTM! Windows user login test with proper cleanup.The test correctly creates a Windows user login using the prerequisite user and includes proper cleanup in the finally block.
227-235: LGTM! WhatIf functionality test validates ShouldProcess support.The test correctly verifies that WhatIf prevents actual login creation while allowing the command to execute without errors.
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (8)
1-52: LGTM! Unit test setup follows the required template.The test file correctly implements the required unit test setup including dependency resolution, module loading, SMO stubs, and proper cleanup in
AfterAll.
53-53: LGTM! Proper use of the 'Public' tag for public command tests.The
Describeblock correctly uses the 'Public' tag as required for public command unit tests.
88-103: LGTM! Error handling test correctly validates existing login scenario.The test properly mocks an existing login condition and verifies the appropriate error is thrown with the expected message pattern.
104-114: LGTM! PassThru parameter test validates object return.The test correctly verifies that the PassThru parameter returns a non-null login object.
116-126: LGTM! Certificate-based login test covers the parameter set.The test validates certificate-based login creation with the required CertificateName parameter.
128-134: LGTM! Asymmetric key-based login test covers the parameter set.The test validates asymmetric key-based login creation with the required AsymmetricKeyName parameter.
196-206: LGTM! Comprehensive test for multiple SQL login options.The test effectively validates that the function can handle multiple options simultaneously including password policies, disabled state, and custom settings.
208-240: LGTM! Parameter filter tests provide precise mock verification.The additional context with parameter filters ensures that the mock assertions are specific to the exact invocation being tested, improving test precision.
source/Public/New-SqlDscLogin.ps1 (8)
1-118: LGTM! Comprehensive comment-based help with clear examples.The comment-based help follows the required format with proper indentation, includes multiple examples covering all parameter sets, and provides clear descriptions for all parameters.
119-123: LGTM! Function declaration follows DSC community guidelines.The function name follows the required
{Verb}-SqlDsc{Noun}pattern, uses an approved PowerShell verb, and includes proper attributes for OutputType and CmdletBinding.
124-201: LGTM! Parameter declarations follow best practices.All parameters follow the required format with proper attributes, datatypes, and names on separate lines. Mandatory parameters are correctly decorated, and parameter sets are well-organized.
213-226: LGTM! Proper error handling for existing logins.The function correctly checks for existing logins and throws a terminating error with a localized message, proper error ID, and appropriate error category.
240-268: LGTM! Well-structured switch statement for login types.The switch statement properly handles all login types and sets the appropriate LoginType enum values and additional properties for Certificate and AsymmetricKey logins.
280-300: LGTM! Correct implementation of SQL login options.The SQL login creation properly handles password expiration, policy enforcement, and login creation options using bitwise operations for combining flags.
307-311: LGTM! Proper implementation of the Disabled parameter.The function correctly calls the
Disable()method on the login object after creation when the Disabled parameter is specified.
315-318: LGTM! Correct implementation of PassThru parameter.The function properly returns the login object when PassThru is specified, following PowerShell conventions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2132 +/- ##
====================================
Coverage 94% 94%
====================================
Files 109 110 +1
Lines 8132 8171 +39
====================================
+ Hits 7693 7732 +39
Misses 439 439
🚀 New features to boost your workflow:
|
| { | ||
| { New-SqlDscLogin -ServerObject $script:serverObject -Name $disabledLoginName -SqlLogin -SecurePassword $script:testPassword -Disabled -Confirm:$false } | Should -Not -Throw | ||
|
|
||
| $loginObject = $script:serverObject.Logins[$disabledLoginName] |
There was a problem hiding this comment.
Add ned command Test-SqlDscIsLoginEnabled (that also uses Test-SqlDscIsLogin to verify exist).
johlju
force-pushed
the
f/command-new-sqldsclogin
branch
from
571fc83 to
c18db78
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
.vscode/tasks.json (1)
87-87: Re-add dependency restore to the test task to prevent missing modulesRemoving -AutoRestore means running tests won’t automatically bootstrap required dependencies. Add -ResolveDependency (or re-introduce -AutoRestore) so local runs don’t fail due to missing modules.
Apply:
- "args": ["-Tasks", "test"], + "args": ["-ResolveDependency", "-Tasks", "test"],tests/Unit/Stubs/SMO.cs (1)
481-484: Disable() now sets IsDisabled = true — matches expectationsThis aligns the stub with expected SMO-like behavior and unblocks tests asserting disabled state after creation.
tests/Integration/Commands/README.md (3)
43-43: Fix formatting issue before "## Integration Tests" heading.Remove the extra blank line to fix the formatting issue identified by the static analysis tool.
New-SqlDscLogin | 2 | 1 (Install-SqlDscServer) | DSCSQLTEST ## Integration Tests
56-60: Add missing period for consistency.Add a period at the end of the description for consistency with other sections.
Creates test logins on the DSCSQLTEST instance for use by other integration tests. The main test login `IntegrationTestSqlLogin` is left in place after -the test completes so other tests can use it for validation purposes +the test completes so other tests can use it for validation purposes.
121-121: Add missing period to the login description.Add a period at the end of the description for consistency with other entries.
-IntegrationTestSqlLogin | P@ssw0rd123! | - | SQL Server login created by New-SqlDscLogin integration tests for testing purposes +IntegrationTestSqlLogin | P@ssw0rd123! | - | SQL Server login created by New-SqlDscLogin integration tests for testing purposes.tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1 (1)
154-154: Update the error message pattern to match the localized string more precisely.The test should use a more specific pattern that matches the actual localized error message format.
- { New-SqlDscLogin -ServerObject $script:serverObject -Name $script:testSqlLoginName -SqlLogin -SecurePassword $script:testPassword -Confirm:$false } | Should -Throw -ExpectedMessage "*The login '$script:testSqlLoginName' already exists on the instance '$($script:serverObject.InstanceName)'*" + { New-SqlDscLogin -ServerObject $script:serverObject -Name $script:testSqlLoginName -SqlLogin -SecurePassword $script:testPassword -Confirm:$false } | Should -Throw -ExpectedMessage "*The login '$script:testSqlLoginName' already exists on the instance*"tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (1)
69-77: Consider verifying the Create method invocation.While the test verifies that
Test-SqlDscIsLoginis called, it could be enhanced to verify that theCreatemethod on the login object is actually invoked.source/Public/New-SqlDscLogin.ps1 (1)
210-211: Consider using a more descriptive variable name.The variable name could be more descriptive to clarify its purpose.
# Determine login type from parameter set -$loginType = $PSCmdlet.ParameterSetName +$loginTypeParameterSet = $PSCmdlet.ParameterSetNameAlso update all references to
$loginTypeto$loginTypeParameterSetin lines 234, 240, and 280.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
.gitignore(1 hunks).vscode/tasks.json(2 hunks)CHANGELOG.md(1 hunks)source/Public/New-SqlDscLogin.ps1(1 hunks)source/en-US/SqlServerDsc.strings.psd1(1 hunks)tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Prerequisites.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/README.md(4 hunks)tests/Unit/Public/New-SqlDscLogin.Tests.ps1(1 hunks)tests/Unit/Stubs/SMO.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
tests/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/**/*.ps1: All tests should use the Pester framework and use Pester v5.0 syntax.
Parameter validation should never be tested.
Test code should never be added outside of the Describe block.
There should only be one Pester Describe block per test file, and the name of the Describe block should be the same as the name of the public command, private function, or class-based resource being tested.
Each scenario or code path being tested should have its own Pester Context block that starts with the phrase 'When'.
Pester It block descriptions should start with the phrase 'Should'.
It blocks must always call the command or function being tested and result and outcomes should be kept in the same It block.
BeforeAll and BeforeEach blocks should never call the command or function being tested.
BeforeAll, BeforeEach, AfterAll and AfterEach blocks should be used inside the Context block as near as possible to the It block that will use the test data, test setup and teardown.
AfterAll block can be used to clean up any test data.
BeforeEach and AfterEach blocks should be used sparingly.
It is okay to duplicate code in BeforeAll and BeforeEach blocks that are used inside different Context blocks.
Use localized strings in the tests only when necessary.
Files that need to be mocked should be created in Pester's test drive using the $TestDrive variable.
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Integration tests for public commands should be placed in tests/Integration/Commands and named after the command with the suffix .Integration.Tests.ps1.
All integration tests must use the provided code block prior to the first Describe block to set up the integration test environment.
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.ps1: All PowerShell files should use UTF8 without BOM.
All PowerShell files must end with a new line.
Try to limit PowerShell code lines to 120 characters.
Use 4 spaces for indentation in PowerShell files, never use tabs.
Use '#' for single line comments in PowerShell code.
Use comment-blocks for multiline comments with the format <# ... #> where the multiline text is indented 4 spaces.
Use descriptive, clear, and full names for all variables, parameters, and function names. All names must be more than 2 characters. No abbreviations should be used.
Use camelCase for local variable names in PowerShell.
Use Write-Error for error messages.
Use Write-Warning for warning messages.
Use Write-Information for informational messages.
Never use Write-Host.
Never use backtick as line continuation in code.
Use splatting for commands to reduce line length.
PowerShell reserved keywords should be in all lower case.
Single quotes should always be used to delimit string literals wherever possible. Double quoted string literals may only be used when string literals contain ($) expressions that need to be evaluated.
Hashtables properties should always be in PascalCase and be on a separate line.
When comparing a value to $null, $null should be on the left side of the comparison.
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/Public/New-SqlDscLogin.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.md: Markdown files should wrap lines after a word when a line exceeds 80 characters.
Use 2 spaces for indentation in Markdown files.
Files:
tests/Integration/Commands/README.mdCHANGELOG.md
source/en-US/SqlServerDsc.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
For public commands and private functions, all localized strings should be added in source/en-US/SqlServerDsc.strings.psd1, and localized string key names should be prefixed with the function name using underscores.
Files:
source/en-US/SqlServerDsc.strings.psd1
source/en-US/*.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
For class-based resources, localized strings should be added in a separate file in source/en-US, named to exactly match the resource class name with the suffix .strings.psd1.
Files:
source/en-US/SqlServerDsc.strings.psd1
source/en-US/*.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Localized string key names should use underscores as word separators if the key name has more than one word.
Files:
source/en-US/SqlServerDsc.strings.psd1
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
The Unreleased section in CHANGELOG.md should always be updated when making changes to the codebase, using the keepachangelog format and providing concrete release notes.
Files:
CHANGELOG.md
tests/Unit/{Classes,Public,Private}/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Unit tests should be added for all public commands, private functions and class-based resources. Unit tests for class-based resources should be placed in tests/Unit/Classes, for public commands in tests/Unit/Public, and for private functions in tests/Unit/Private. The unit tests should be named after the item being tested with the suffix .Tests.ps1.
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/Public/*.Tests.ps1: All public commands should always have a test to validate parameter sets using the provided template.
All public commands should also include tests to validate parameter properties.
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/**/*.ps1: Never test, mock or use Should -Invoke for Write-Verbose and Write-Debug.
Never use Should -Not -Throw in tests; instead, call the command directly.
All unit tests should use the provided code block prior to the Describe block to set up the test environment and load the correct module.
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
source/Public/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/Public/*.ps1: Public PowerShell commands should each have their own script file named after the command (with .ps1 extension) and be placed in the folder source/Public.
All public command names must have the noun prefixed with 'SqlDsc', e.g. {Verb}-SqlDsc{Noun}.
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private,Classes}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private,Classes}/*.ps1: Comment-based help should always be before the function-statement for each public command and private function, and before the class-statement for each class-based resource. It should be in the format of a comment block and at least use the keywords: .SYNOPSIS, .DESCRIPTION, .PARAMETER, .EXAMPLE, and .NOTES.
Each comment-based help keyword should be indented with 4 spaces and each keyword's text should be indented 8 spaces.
The text for keyword .DESCRIPTION should be descriptive and must have a length greater than 40 characters. The .SYNOPSIS keyword text should be a short description.
A comment-based help must have at least one example, but preferably more examples to showcase all possible parameter sets and different parameter combinations.
Use PascalCase for function names and parameters in public commands, private functions, and class-based resources.
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private}/*.ps1: All message strings for Write-Debug, Write-Verbose, Write-Error, Write-Warning and other error messages in public commands and private functions should be localized using localized string keys.
All public command and private function names must follow the standard PowerShell Verb-Noun format.
All public command and private function names must use PowerShell approved verbs.
All public commands and private functions should always be advanced functions and have [CmdLetBinding()] attribute.
Public commands and private functions with no parameters should still have an empty parameter block param ().
Every parameter in public commands and private functions should include the [Parameter()] attribute.
A mandatory parameter in public commands and private functions should contain the decoration [Parameter(Mandatory = $true)], and non-mandatory parameters should not contain the Mandatory decoration.
Parameters attributes, datatype and its name should be on separate lines.
Parameters must be separated by a single, blank line.
Use Write-Verbose to output verbose output for actions a public command or private function does.
Use Write-Debug for debug output for processes within the script for user to see the decisions a command or private function does.
Files:
source/Public/New-SqlDscLogin.ps1
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : For public commands and private functions, all localized strings should be added in source/en-US/SqlServerDsc.strings.psd1, and localized string key names should be prefixed with the function name using underscores.
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to azure-pipelines.yml : Integration test script files for public commands must be added to a group within the 'Integration_Test_Commands_SqlServer' stage in ./azure-pipelines.yml.
Applied to files:
tests/Integration/Commands/README.mdtests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : For public commands and private functions, all localized strings should be added in source/en-US/SqlServerDsc.strings.psd1, and localized string key names should be prefixed with the function name using underscores.
Applied to files:
source/en-US/SqlServerDsc.strings.psd1CHANGELOG.md
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to source/{Public,Private}/*.ps1 : All message strings for Write-Debug, Write-Verbose, Write-Error, Write-Warning and other error messages in public commands and private functions should be localized using localized string keys.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-08-03T09:50:27.001Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-03T09:50:27.001Z
Learning: Applies to tests/Unit/{Classes,Public,Private}/*.Tests.ps1 : Unit tests should be added for all public commands, private functions and class-based resources. Unit tests for class-based resources should be placed in tests/Unit/Classes, for public commands in tests/Unit/Public, and for private functions in tests/Unit/Private. The unit tests should be named after the item being tested with the suffix .Tests.ps1.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
🪛 LanguageTool
tests/Integration/Commands/README.md
[grammar] ~43-~43: Use correct spacing
Context: ... | 1 (Install-SqlDscServer) | DSCSQLTEST ## Integration Tests ### Prerequisites´ ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~59-~59: There might be a mistake here.
Context: ...tionTestSqlLogin` is left in place after the test completes so other tests can us...
(QB_NEW_EN)
[grammar] ~60-~60: Use correct spacing
Context: ...ests can use it for validation purposes. ## Dependencies ### SqlServer module Ther...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~121-~121: Use correct spacing
Context: ... integration tests for testing purposes. ### Image media (ISO) The environment varia...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (20)
.gitignore (1)
4-4: LGTM: ignore coverage.xmlIgnoring coverage.xml is harmless and helps keep the repo clean from local test artifacts.
CHANGELOG.md (1)
30-34: Good, concise entry for New-SqlDscLoginThe “Added” section accurately documents the new public command and capabilities.
tests/Unit/Stubs/SMO.cs (2)
486-489: Enable() resets IsDisabled — good fidelity for testsSymmetric to Disable(); makes state transitions explicit and test-friendly.
491-494: Certificate/AsymmetricKey/Language fields added — useful for New-SqlDscLogin testsThese properties provide the hooks needed to validate certificate/key-backed logins and language.
source/en-US/SqlServerDsc.strings.psd1 (1)
237-245: Localized strings for New-SqlDscLogin are present and follow existing naming patternsShouldProcess description/warning/caption and creation messages look consistent with module conventions (e.g., Audit_* and Login_* patterns).
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1 (1)
47-52: LGTM! The test correctly creates the SqlIntegrationTest user.The test properly creates the local Windows user needed for the
New-SqlDscLoginintegration tests with appropriate metadata and validates creation success.tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1 (6)
1-31: LGTM! Test setup follows the proper integration test structure.The integration test correctly sets up the test environment using DscResource.Test and properly imports the SqlServerDsc module.
32-44: LGTM! Proper test resource initialization.The test properly starts the SQL Server service and sets up test login names for different authentication types.
74-90: LGTM! Comprehensive SQL Server login creation tests.The tests thoroughly validate SQL Server login creation, including verification of login existence, type, and default database settings.
159-182: LGTM! Windows user login tests are well structured.The test properly creates and validates Windows user login, including cleanup in the finally block.
184-204: LGTM! Windows group login tests follow best practices.The test properly validates Windows group login creation with appropriate cleanup.
206-235: LGTM! Force and WhatIf parameter tests are comprehensive.The tests properly validate the Force parameter bypasses confirmation and WhatIf prevents actual login creation.
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (3)
1-52: LGTM! Unit test setup follows the standard template.The test setup correctly loads the module, mocks, and SMO stubs as required for unit testing.
116-126: LGTM! Certificate and asymmetric key login tests are well structured.The tests properly validate certificate-based and asymmetric key-based login creation.
196-206: LGTM! Comprehensive test for multiple SQL login options.The test thoroughly validates that multiple login options can be combined successfully.
source/Public/New-SqlDscLogin.ps1 (5)
1-118: LGTM! Comprehensive comment-based help documentation.The comment-based help is well-structured with clear descriptions, multiple examples covering different scenarios, and proper documentation of all parameters.
119-201: LGTM! Well-designed function signature with proper parameter sets.The function correctly implements five distinct parameter sets for different login types with appropriate mandatory parameters and validation attributes.
240-268: LGTM! Login type configuration is properly implemented.The switch statement correctly sets the appropriate login type and associated properties for each authentication method.
279-305: LGTM! SQL Server login creation with password options is well implemented.The code properly handles SQL Server login specific properties including password policies and creation options, while correctly creating Windows/Certificate/AsymmetricKey logins without passwords.
307-319: LGTM! Post-creation operations are properly handled.The function correctly disables the login if requested and returns the login object when PassThru is specified.
johlju
force-pushed
the
f/command-new-sqldsclogin
branch
from
c18db78 to
ca62b47
Compare
…gin for creating test logins
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (10)
tests/Unit/Stubs/SMO.cs (1)
487-495: Disable()/Enable() now properly toggle IsDisabled — good fixThis addresses the earlier review to reflect state changes in the stub, improving fidelity for unit tests asserting post-action state.
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1 (4)
83-85: Replace direct SMO calls with Get-SqlDscLogin command.The tests are directly accessing
$script:serverObject.Loginsto verify login properties, which should be replaced with theGet-SqlDscLogincommand as suggested in past review comments. This approach is more maintainable and consistent with the existing command usage pattern.For example, lines 83-85:
- $loginObject = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testSqlLoginName + $loginObject = Get-SqlDscLogin -ServerObject $script:serverObject -Name $script:testSqlLoginNameThis pattern appears to already be correctly used in the code. Good!
Also applies to: 88-90, 170-172, 192-194
154-154: Update ExpectedMessage pattern to match localized string more precisely.The test uses a wildcarded pattern that correctly matches the localized string. This is an improvement from the previous generic pattern.
107-107: Consider using Remove-SqlDscLogin for cleanup.The test is directly calling
Drop()method on the login object. As suggested in past review comments, consider usingRemove-SqlDscLogincommand when it becomes available for more consistent cleanup operations.Would you like me to open an issue to track the creation of
Remove-SqlDscLogincommand and updating these integration tests to use it?Also applies to: 128-128, 140-140
140-141: Add Test-SqlDscIsLoginEnabled for comprehensive verification.As suggested in past review comments, consider adding
Test-SqlDscIsLoginEnabledcommand to verify the enabled/disabled state of logins rather than directly accessing theIsDisabledproperty.Would you like me to open an issue to track the creation of
Test-SqlDscIsLoginEnabledcommand for more comprehensive login state verification?tests/Integration/Commands/README.md (3)
43-43: Fix formatting issue with extra blank line.There's an extra blank line before the "## Integration Tests" heading that breaks the formatting.
Remove the extra blank line between line 43 and the heading:
New-SqlDscLogin | 2 | 1 (Install-SqlDscServer) | DSCSQLTEST - ## Integration Tests
60-60: Add missing period for punctuation consistency.The description for New-SqlDscLogin is missing a terminal period.
-the test completes so other tests can use it for validation purposes. +the test completes so other tests can use it for validation purposes.
121-121: Add missing period in login description.The description for IntegrationTestSqlLogin is missing a terminal period.
-IntegrationTestSqlLogin | P@ssw0rd123! | - | SQL Server login created by New-SqlDscLogin integration tests for testing purposes. +IntegrationTestSqlLogin | P@ssw0rd123! | - | SQL Server login created by New-SqlDscLogin integration tests for testing purposes.tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (1)
75-77: Consider adding verification for the Create method invocation.While the test verifies that
Test-SqlDscIsLoginis called, it doesn't verify that theCreatemethod on the login object is actually invoked. This would provide more comprehensive test coverage.Consider adding a mock or assertion to verify the Create method is called on the login object. This could be done by mocking the New-Object cmdlet to return a mock login object with a verifiable Create method.
source/Public/New-SqlDscLogin.ps1 (1)
210-211: Consider using a more descriptive variable name.The variable name
$loginTypecould be more descriptive to clarify its purpose as it represents the parameter set name.- # Determine login type from parameter set - $loginType = $PSCmdlet.ParameterSetName + # Determine login type from parameter set + $loginTypeParameterSet = $PSCmdlet.ParameterSetNameAnd update all references to
$loginTypethroughout the function to use$loginTypeParameterSet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.gitignore(1 hunks)CHANGELOG.md(1 hunks)source/Public/New-SqlDscLogin.ps1(1 hunks)source/en-US/SqlServerDsc.strings.psd1(1 hunks)tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Prerequisites.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/README.md(4 hunks)tests/Unit/Public/New-SqlDscLogin.Tests.ps1(1 hunks)tests/Unit/Stubs/SMO.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (19)
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Always update the Unreleased section in CHANGELOG.md using Keep a Changelog format with concrete release notes for changes (new/modified commands, functions, resources)
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe main changes as concise release notes
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Markdown: wrap lines at word boundaries when exceeding 80 characters; use 2 spaces for indentation
Files:
CHANGELOG.mdtests/Integration/Commands/README.md
⚙️ CodeRabbit Configuration File
**/*.md: # Markdown Style Guidelines
- Wrap lines at 80 characters
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Surround fenced code blocks with blank lines
Files:
CHANGELOG.mdtests/Integration/Commands/README.md
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.mdtests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Unit/Stubs/SMO.cstests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/en-US/SqlServerDsc.strings.psd1tests/Integration/Commands/README.mdsource/Public/New-SqlDscLogin.ps1
tests/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/**/*.ps1: All tests must use Pester and Pester v5.0 syntax
Do not test parameter validation
Test code must not exist outside of a Describe block
Each test file must contain exactly one Describe block named after the subject (public command, private function, or resource)
Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Use BeforeAll/BeforeEach/AfterAll/AfterEach inside Context, placed near the It blocks using them; duplication across contexts is acceptable; AfterAll can clean up; use BeforeEach/AfterEach sparingly
Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Files that need to be mocked in tests must be created in Pester's $TestDrive
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named .Integration.Tests.ps1
In CI, integration tests must use Get-ComputerName when the computer name is needed
Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.ps1: PowerShell files must be UTF-8 without BOM and end with a newline
Try to limit lines to 120 characters
Use 4 spaces for indentation; never use tabs
Use '#' for single-line comments
Use <# ... #> for multiline comments; indent the multiline text 4 spaces
Use descriptive, clear, full names for variables, parameters, and function names; names must be more than 2 characters; no abbreviations
Use camelCase for local variable names
Never use Write-Host
Never use backtick (`) for line continuation
Use splatting to reduce line length
PowerShell reserved keywords should be lowercase
Use single quotes for string literals whenever possible; use double quotes only when interpolation is needed
Hashtable properties should be PascalCase and each on a separate line
When comparing to $null, place $null on the left side of the comparison
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/Public/New-SqlDscLogin.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit Configuration File
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: List each return type (one per line) with a 1‑line description. Must match both [OutputType()] and actual ...
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/en-US/SqlServerDsc.strings.psd1source/Public/New-SqlDscLogin.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit Configuration File
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- One
Describeblock per file matching the tested entity name- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test
Write-Verbose,Write-Debug, or parameter binding behavior- Pass all mandatory parameters to avoid prompts
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEachItdescriptions start with 'Should'Contextdescriptions start with 'When'- Mock variables prefix: 'mock'
- Prefer
-BeTrue/-BeFalseover-Be $true/-Be $false- No
Should -Not -Throw- invoke commands directlyFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachallowed onContextandItblocks- Keep scope close to usage context
Best Practices
- Assign unused return objects to
$null- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Cover all scenarios and code paths
- Use
BeforeEachandAfterEachsparingly
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
⚙️ CodeRabbit Configuration File
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).Required Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' } } BeforeAll { $script:dscModuleName = 'SqlServerDsc' Import-Module -Name $script:dscModuleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
tests/Unit/**/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/**/*.Tests.ps1: Never test, mock, or use Should -Invoke for Write-Verbose or Write-Debug in unit tests
Do not use Should -Not -Throw; invoke the command directly
Include the standard unit test setup block (BeforeDiscovery/BeforeAll/AfterAll with module loading and PSDefaultParameterValues) before the Describe block
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/Public/*.Tests.ps1: Unit tests for public commands must be placed in tests/Unit/Public and named .Tests.ps1
Public command unit tests must include parameter set validation using the provided template/pattern
Public command unit tests must include tests to validate parameter properties (e.g., Mandatory, ValueFromPipeline)
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
source/en-US/SqlServerDsc.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Files:
source/en-US/SqlServerDsc.strings.psd1
source/en-US/*.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Each class-based resource must have a strings file in source/en-US named exactly .strings.psd1; keys use underscores as word separator
Files:
source/en-US/SqlServerDsc.strings.psd1
tests/Integration/Commands/README.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Use the environment details in tests/Integration/Commands/README.md when writing integration tests
Files:
tests/Integration/Commands/README.md
source/Public/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/Public/*.ps1: Each public PowerShell command must be in its own script file named exactly as the command and located in source/Public
Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Files:
source/Public/New-SqlDscLogin.ps1
source/Public/*-SqlDsc*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
All public command names must have the noun prefixed with 'SqlDsc' (Verb-SqlDscNoun)
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private,Classes}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private,Classes}/*.ps1: Comment-based help must precede each function/class with a comment block using at least .SYNOPSIS, .DESCRIPTION, .PARAMETER, .EXAMPLE, .NOTES
In comment-based help, each keyword must be indented 4 spaces and its text indented 8 spaces
Comment-based help .DESCRIPTION text must be descriptive and > 40 characters; .SYNOPSIS should be short; include at least one example
Use PascalCase for function names and parameters in public commands, private functions, and class-based resources
Files:
source/Public/New-SqlDscLogin.ps1
source/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Prefer SQL Server Management Objects (SMO) over T-SQL for interacting with SQL Server; only use T-SQL when SMO cannot achieve the functionality
Files:
source/Public/New-SqlDscLogin.ps1
⚙️ CodeRabbit Configuration File
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/SqlServerDsc.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming
- Format:
FunctionName_Description(underscore separators)- Example:
Get_SqlDscDatabase_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private}/*.ps1: Public command and private function names must follow Verb-Noun format and use PowerShell approved verbs
Public commands and private functions must be advanced functions and include [CmdletBinding()]
Even with no parameters, public commands and private functions must include an empty parameter block: param ()
Every parameter in public commands and private functions must include a [Parameter()] attribute
Only mandatory parameters should have [Parameter(Mandatory = $true)]; non-mandatory parameters must not set Mandatory
For parameters, place attributes, datatype, and name on separate lines; separate parameters by a single blank line
Use Write-Verbose for action messages; Write-Debug for internal decision messages; Write-Error for errors; Write-Warning for warnings; Write-Information for informational messages
Files:
source/Public/New-SqlDscLogin.ps1
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Public/*.ps1 : Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Applied to files:
CHANGELOG.mdtests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Public/*.ps1 : Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
CHANGELOG.mdsource/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Private/*.ps1 : Private functions must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
CHANGELOG.mdsource/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T10:48:15.373Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/suffix.ps1:24-24
Timestamp: 2025-08-17T10:48:15.373Z
Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.
Applied to files:
CHANGELOG.mdtests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to tests/**/*.ps1 : Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Applied to files:
tests/Integration/Commands/Prerequisites.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.052Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.052Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Unit/Stubs/SMO.cstests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/README.md
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Integration/Commands/README.md
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Classes/*.ps1 : A DSC resource should inherit SqlResourceBase if it needs to connect to a SQL Server Database Engine
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.175Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.175Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to tests/**/*.ps1 : Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Applied to files:
source/en-US/SqlServerDsc.strings.psd1
🪛 LanguageTool
tests/Integration/Commands/README.md
[grammar] ~43-~43: Use correct spacing
Context: ... | 1 (Install-SqlDscServer) | DSCSQLTEST ## Integration Tests ### Prerequisites´ ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~59-~59: There might be a mistake here.
Context: ...tionTestSqlLogin` is left in place after the test completes so other tests can us...
(QB_NEW_EN)
[grammar] ~60-~60: Use correct spacing
Context: ...ests can use it for validation purposes. ## Dependencies ### SqlServer module Ther...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~121-~121: Use correct spacing
Context: ... integration tests for testing purposes. ### Image media (ISO) The environment varia...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (12)
.gitignore (1)
4-4: Add coverage.xml ignore — LGTMThe addition is minimal and safe; no further changes needed here.
tests/Unit/Stubs/SMO.cs (1)
497-500: Stub surface extended for New-SqlDscLogin scenarios — looks consistentExposing Certificate, AsymmetricKey, and Language as simple fields is sufficient for current tests. No functional concerns.
source/en-US/SqlServerDsc.strings.psd1 (1)
244-252: Verified New-SqlDscLogin uses all localized strings; no hard-coded text remainsAll
ShouldProcesscalls (line 232) andWrite-Verbosemessages (lines 234, 313) consume the new$script:localizedData.Login_Add_*keys. No literal strings are passed to anyWrite-*cmdlets. Approving these changes.CHANGELOG.md (2)
40-44: Changelog entry for New-SqlDscLogin — clear and compliantEntry follows Keep a Changelog style and is wrapped sensibly. No edits needed.
51-57: Prerequisites and integration docs bullets — LGTMAccurately reflect added test scaffolding for the new command.
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1 (2)
72-72: LGTM! Secure password handling.The test correctly creates a SecureString for password testing, following PowerShell security best practices.
227-235: LGTM! Good coverage of ShouldProcess functionality.The test properly validates WhatIf behavior by ensuring the login is not created when WhatIf is specified.
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (2)
30-31: LGTM! Proper test environment setup.The test correctly sets the CI environment variable and imports the module, following the test guidelines.
218-226: Good use of parameter filters for precise mock verification.The tests correctly use parameter filters to ensure mock invocations are verified with specific parameters, providing precise test coverage.
Also applies to: 229-233, 235-239
source/Public/New-SqlDscLogin.ps1 (3)
6-7: LGTM! Clear and comprehensive documentation.The comment-based help is well-structured with detailed descriptions, multiple examples covering different scenarios, and proper documentation of all parameters.
214-226: LGTM! Proper error handling for existing logins.The function correctly checks for existing logins and throws a terminating error with appropriate localized message and error category.
282-283: LGTM! Correct handling of switch parameters.The code properly uses
.IsPresentto check the state of switch parameters for password settings.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (2)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (1)
75-77: Test description doesn’t match assertionsThe test claims to verify Create() is called, but it doesn’t assert that. Either add an assertion or rename the test.
Option A (preferred): assert invocation via a seam you can mock (e.g., a private helper that constructs and creates the login) and use Should -Invoke on that helper.
Option B (rename):
- It 'Should call Create method on login object' { - New-SqlDscLogin -ServerObject $script:mockServerObject -Name 'TestLogin' -SqlLogin -SecurePassword $script:mockSecurePassword -Confirm:$false - } + It 'Should create the login' { + New-SqlDscLogin -ServerObject $script:mockServerObject -Name 'TestLogin' -SqlLogin -SecurePassword $script:mockSecurePassword -Confirm:$false + }If you want, I can help introduce a small factory wrapper to make Create() verifiable with Should -Invoke.
source/Public/New-SqlDscLogin.ps1 (1)
210-266: Improve variable naming clarity for parameter set usageRename $loginType to $loginTypeParameterSet for self-documenting intent and to avoid confusion with SMO LoginType.
Apply this diff:
- # Determine login type from parameter set - $loginType = $PSCmdlet.ParameterSetName + # Determine login type from parameter set + $loginTypeParameterSet = $PSCmdlet.ParameterSetName @@ - $verboseDescriptionMessage = $script:localizedData.Login_Add_ShouldProcessVerboseDescription -f $Name, $loginType, $ServerObject.InstanceName + $verboseDescriptionMessage = $script:localizedData.Login_Add_ShouldProcessVerboseDescription -f $Name, $loginTypeParameterSet, $ServerObject.InstanceName @@ - switch ($loginType) + switch ($loginTypeParameterSet) @@ - if ($loginType -eq 'SqlLogin') + if ($loginTypeParameterSet -eq 'SqlLogin')Also applies to: 277-299
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/instructions/dsc-community-style-guidelines-markdown.instructions.md(1 hunks)source/Public/New-SqlDscLogin.ps1(1 hunks)source/en-US/SqlServerDsc.strings.psd1(1 hunks)tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1(1 hunks)tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1(3 hunks)tests/Unit/Public/New-SqlDscLogin.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
tests/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/**/*.ps1: All tests must use Pester and Pester v5.0 syntax
Do not test parameter validation
Test code must not exist outside of a Describe block
Each test file must contain exactly one Describe block named after the subject (public command, private function, or resource)
Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Use BeforeAll/BeforeEach/AfterAll/AfterEach inside Context, placed near the It blocks using them; duplication across contexts is acceptable; AfterAll can clean up; use BeforeEach/AfterEach sparingly
Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Files that need to be mocked in tests must be created in Pester's $TestDrive
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
tests/Integration/Commands/*.Integration.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Integration/Commands/*.Integration.Tests.ps1: Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named .Integration.Tests.ps1
In CI, integration tests must use Get-ComputerName when the computer name is needed
Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.ps1: PowerShell files must be UTF-8 without BOM and end with a newline
Try to limit lines to 120 characters
Use 4 spaces for indentation; never use tabs
Use '#' for single-line comments
Use <# ... #> for multiline comments; indent the multiline text 4 spaces
Use descriptive, clear, full names for variables, parameters, and function names; names must be more than 2 characters; no abbreviations
Use camelCase for local variable names
Never use Write-Host
Never use backtick (`) for line continuation
Use splatting to reduce line length
PowerShell reserved keywords should be lowercase
Use single quotes for string literals whenever possible; use double quotes only when interpolation is needed
Hashtable properties should be PascalCase and each on a separate line
When comparing to $null, place $null on the left side of the comparison
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1source/Public/New-SqlDscLogin.ps1
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1source/en-US/SqlServerDsc.strings.psd1source/Public/New-SqlDscLogin.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit Configuration File
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: List each return type (one per line) with a 1‑line description. Must match both [OutputType()] and actual ...
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1source/en-US/SqlServerDsc.strings.psd1source/Public/New-SqlDscLogin.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit Configuration File
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- One
Describeblock per file matching the tested entity name- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test
Write-Verbose,Write-Debug, or parameter binding behavior- Pass all mandatory parameters to avoid prompts
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEachItdescriptions start with 'Should'Contextdescriptions start with 'When'- Mock variables prefix: 'mock'
- Prefer
-BeTrue/-BeFalseover-Be $true/-Be $false- No
Should -Not -Throw- invoke commands directlyFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachallowed onContextandItblocks- Keep scope close to usage context
Best Practices
- Assign unused return objects to
$null- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Cover all scenarios and code paths
- Use
BeforeEachandAfterEachsparingly
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
⚙️ CodeRabbit Configuration File
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).Required Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' } } BeforeAll { $script:dscModuleName = 'SqlServerDsc' Import-Module -Name $script:dscModuleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
tests/Unit/**/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/**/*.Tests.ps1: Never test, mock, or use Should -Invoke for Write-Verbose or Write-Debug in unit tests
Do not use Should -Not -Throw; invoke the command directly
Include the standard unit test setup block (BeforeDiscovery/BeforeAll/AfterAll with module loading and PSDefaultParameterValues) before the Describe block
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/Public/*.Tests.ps1: Unit tests for public commands must be placed in tests/Unit/Public and named .Tests.ps1
Public command unit tests must include parameter set validation using the provided template/pattern
Public command unit tests must include tests to validate parameter properties (e.g., Mandatory, ValueFromPipeline)
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
source/en-US/SqlServerDsc.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Files:
source/en-US/SqlServerDsc.strings.psd1
source/en-US/*.strings.psd1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Each class-based resource must have a strings file in source/en-US named exactly .strings.psd1; keys use underscores as word separator
Files:
source/en-US/SqlServerDsc.strings.psd1
source/Public/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/Public/*.ps1: Each public PowerShell command must be in its own script file named exactly as the command and located in source/Public
Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Files:
source/Public/New-SqlDscLogin.ps1
source/Public/*-SqlDsc*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
All public command names must have the noun prefixed with 'SqlDsc' (Verb-SqlDscNoun)
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private,Classes}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private,Classes}/*.ps1: Comment-based help must precede each function/class with a comment block using at least .SYNOPSIS, .DESCRIPTION, .PARAMETER, .EXAMPLE, .NOTES
In comment-based help, each keyword must be indented 4 spaces and its text indented 8 spaces
Comment-based help .DESCRIPTION text must be descriptive and > 40 characters; .SYNOPSIS should be short; include at least one example
Use PascalCase for function names and parameters in public commands, private functions, and class-based resources
Files:
source/Public/New-SqlDscLogin.ps1
source/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Prefer SQL Server Management Objects (SMO) over T-SQL for interacting with SQL Server; only use T-SQL when SMO cannot achieve the functionality
Files:
source/Public/New-SqlDscLogin.ps1
⚙️ CodeRabbit Configuration File
source/**/*.ps1: # Localization GuidelinesRequirements
- Localize all Write-Debug, Write-Verbose, Write-Error, Write-Warning and $PSCmdlet.ThrowTerminatingError() messages
- Use localized string keys, not hardcoded strings
- Assume
$script:localizedDatais availableString Files
- Commands/functions:
source/en-US/SqlServerDsc.strings.psd1- Class resources:
source/en-US/{ResourceClassName}.strings.psd1Key Naming
- Format:
FunctionName_Description(underscore separators)- Example:
Get_SqlDscDatabase_ConnectingToDatabaseString Format
ConvertFrom-StringData @' KeyName = Message with {0} placeholder. (PREFIX0001) '@String IDs
- Format:
(PREFIX####)- PREFIX: First letter of each word in class or function name (SqlSetup → SS, Get-SqlDscDatabase → GSDD)
- Number: Sequential from 0001
Usage
Write-Verbose -Message ($script:localizedData.KeyName -f $value1)
Files:
source/Public/New-SqlDscLogin.ps1
source/{Public,Private}/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
source/{Public,Private}/*.ps1: Public command and private function names must follow Verb-Noun format and use PowerShell approved verbs
Public commands and private functions must be advanced functions and include [CmdletBinding()]
Even with no parameters, public commands and private functions must include an empty parameter block: param ()
Every parameter in public commands and private functions must include a [Parameter()] attribute
Only mandatory parameters should have [Parameter(Mandatory = $true)]; non-mandatory parameters must not set Mandatory
For parameters, place attributes, datatype, and name on separate lines; separate parameters by a single blank line
Use Write-Verbose for action messages; Write-Debug for internal decision messages; Write-Error for errors; Write-Warning for warnings; Write-Information for informational messages
Files:
source/Public/New-SqlDscLogin.ps1
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.052Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.052Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1tests/Unit/Public/New-SqlDscLogin.Tests.ps1tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:48:15.373Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/suffix.ps1:24-24
Timestamp: 2025-08-17T10:48:15.373Z
Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Classes/*.ps1 : A DSC resource should inherit SqlResourceBase if it needs to connect to a SQL Server Database Engine
Applied to files:
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.175Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.175Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to **/*.md : Markdown: wrap lines at word boundaries when exceeding 80 characters; use 2 spaces for indentation
Applied to files:
.github/instructions/dsc-community-style-guidelines-markdown.instructions.md
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Public/*.ps1 : Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/Private/*.ps1 : Private functions must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
source/en-US/SqlServerDsc.strings.psd1
📚 Learning: 2025-08-17T09:48:55.739Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to tests/**/*.ps1 : Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Applied to files:
source/en-US/SqlServerDsc.strings.psd1
🧬 Code Graph Analysis (5)
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1 (1)
tests/Unit/Stubs/SMO.cs (1)
Login(354-484)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (1)
tests/Unit/Stubs/SMO.cs (4)
Login(354-484)Create(439-479)LoginType(29-39)ServerConnection(730-736)
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1 (1)
tests/Unit/Stubs/SMO.cs (6)
Login(354-484)Create(439-479)LoginCreateOptions(18-23)ChangePassword(385-419)LoginType(29-39)Login(376-379)
source/en-US/SqlServerDsc.strings.psd1 (1)
tests/Unit/Stubs/SMO.cs (1)
Login(354-484)
source/Public/New-SqlDscLogin.ps1 (1)
tests/Unit/Stubs/SMO.cs (7)
Login(354-484)Create(439-479)Create(421-430)Login(371-374)ServerConnection(730-736)Login(376-379)LoginType(29-39)
🪛 LanguageTool
.github/instructions/dsc-community-style-guidelines-markdown.instructions.md
[grammar] ~8-~8: There might be a mistake here.
Context: ...Markdown Style Guidelines - Wrap lines after word when over 80 characters - Use 2 sp...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...lines after word when over 80 characters - Use 2 spaces for indentation - Use '1.' ...
(QB_NEW_EN_OTHER)
⏰ 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). (2)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (8)
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1 (2)
60-63: Good: switched to New-SqlDscLogin for setupUsing the public command with -Force aligns with integration test guidelines and avoids prompts.
119-122: Good: consistent creation path via New-SqlDscLoginThis improves readability and reuses the public surface instead of ad-hoc T-SQL.
source/en-US/SqlServerDsc.strings.psd1 (1)
245-251: Localization keys consumption verifiedAll five new localization keys are used in New-SqlDscLogin.ps1:
- Login_Add_LoginAlreadyExists at line 216
- Login_Add_ShouldProcessVerboseDescription at line 228
- Login_Add_ShouldProcessVerboseWarning at line 229
- Login_Add_ShouldProcessCaption at line 230
- Login_Add_LoginCreated at line 311
No further changes needed—keys are correctly consumed.
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (1)
97-101: OK to assert throws; consider keeping message generic in unit testsUsing a generic “already exists” pattern is acceptable here. No change requested.
source/Public/New-SqlDscLogin.ps1 (4)
268-276: LGTM: Default database and optional language are applied correctlyConditionally applying Language only when provided avoids unintended changes; DefaultDatabase set explicitly.
311-316: LGTM: Verbose message and PassThru behavior are appropriateLocalized verbose output and returning the Login object only when requested match module conventions.
130-133: Strengthen parameter validation (null/empty) per guidelinesAdd [ValidateNotNullOrEmpty()] (and [ValidateNotNull()] for SecureString) to catch bad input early and keep behavior consistent with other commands.
Apply this diff:
[Parameter(Mandatory = $true, ValueFromPipeline = $true)] + [ValidateNotNull()] [Microsoft.SqlServer.Management.Smo.Server] $ServerObject, @@ [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] [System.String] $Name, @@ [Parameter(ParameterSetName = 'SqlLogin', Mandatory = $true)] + [ValidateNotNull()] [System.Security.SecureString] $SecurePassword, @@ [Parameter(ParameterSetName = 'Certificate', Mandatory = $true)] + [ValidateNotNullOrEmpty()] [System.String] $CertificateName, @@ [Parameter(ParameterSetName = 'AsymmetricKey', Mandatory = $true)] + [ValidateNotNullOrEmpty()] [System.String] $AsymmetricKeyName, @@ [Parameter()] + [ValidateNotNullOrEmpty()] [System.String] $DefaultDatabase = 'master', @@ [Parameter()] + [ValidateNotNullOrEmpty()] [System.String] $DefaultLanguage,Also applies to: 154-157, 158-161, 162-165, 166-169, 170-173, 126-129
⛔ Skipped due to learnings
Learnt from: johlju PR: dsccommunity/SqlServerDsc#2136 File: source/Public/Remove-SqlDscLogin.ps1:59-77 Timestamp: 2025-08-17T10:59:03.922Z Learning: In PowerShell, mandatory parameters with [Parameter(Mandatory = $true)] automatically guard against $null values and don't require additional [ValidateNotNull()] or [ValidateNotNullOrEmpty()] attributes, as these validations are redundant with the mandatory parameter behavior.
205-208: Fix Force/Confirm logic: $Confirm is not a valid variableUse $PSBoundParameters.ContainsKey('Confirm') when honoring -Force; $Confirm isn’t bound as a variable.
Apply this diff:
- if ($Force.IsPresent -and -not $Confirm) + if ($Force.IsPresent -and -not $PSBoundParameters.ContainsKey('Confirm')) { $ConfirmPreference = 'None' }⛔ Skipped due to learnings
Learnt from: johlju PR: dsccommunity/SqlServerDsc#2136 File: source/Public/Remove-SqlDscLogin.ps1:83-86 Timestamp: 2025-08-17T10:03:59.955Z Learning: In PowerShell functions with SupportsShouldProcess, the $Confirm automatic variable defaults to $false when no -Confirm parameter is passed, and only becomes $true when -Confirm or -Confirm:$true is explicitly passed. The logic `if ($Force.IsPresent -and -not $Confirm)` correctly handles both cases where no -Confirm is passed and where -Confirm:$false is explicitly passed.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
azure-pipelines.yml (1)
277-291: Add missing New-SqlDscLogin integration test to the pipelineThe file
tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1exists but isn’t referenced inazure-pipelines.yml, so its tests won’t run.• Update
azure-pipelines.yml(around lines 277–291) in Group 2 to include the new test before the existing login assertions:./build.ps1 -Tasks test -CodeCoverageThreshold 0 -PesterTag $(TEST_CONFIGURATION) -PesterPath @( # Run the integration tests in a specific group order. # Group 0 'tests/Integration/Commands/Prerequisites.Integration.Tests.ps1' # Group 1 'tests/Integration/Commands/Install-SqlDscServer.Integration.Tests.ps1' 'tests/Integration/Commands/Connect-SqlDscDatabaseEngine.Integration.Tests.ps1' # Group 2 + 'tests/Integration/Commands/New-SqlDscLogin.Integration.Tests.ps1' 'tests/Integration/Commands/Assert-SqlDscLogin.Integration.Tests.ps1' 'tests/Integration/Commands/Get-SqlDscLogin.Integration.Tests.ps1' 'tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1' # Group 9 'tests/Integration/Commands/Uninstall-SqlDscServer.Integration.Tests.ps1' )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.vscode/settings.json(1 hunks)azure-pipelines.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
azure-pipelines.yml
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Files:
azure-pipelines.yml
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
azure-pipelines.yml
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.739Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
⏰ 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). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
…ncy by consolidating BeforeEach and AfterEach blocks into It statements, ensuring proper login creation and verification.
…usage, improving readability and clarity.
…alidations and improving command metadata checks for better test coverage and reliability.
…improved readability and maintainability.
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r18, 2 of 2 files at r19, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @johlju)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (3)
265-271: Prefer parameter-set-specific attribute over __AllParameterSetsWhen both a specific ParameterSetName and __AllParameterSets exist, Select-Object -First 1 can pick the wrong one, masking issues. First look for the specific ParameterSetName, then fall back to __AllParameterSets.
Apply this diff:
- $parameterAttribute = $parameter.Attributes | - Where-Object -FilterScript { - $_ -is [System.Management.Automation.ParameterAttribute] -and - ($_.ParameterSetName -eq $ParameterSetName -or $_.ParameterSetName -eq '__AllParameterSets') - } | - Select-Object -First 1 + $parameterAttributes = $parameter.Attributes | + Where-Object -FilterScript { + $_ -is [System.Management.Automation.ParameterAttribute] -and + ($_.ParameterSetName -eq $ParameterSetName -or $_.ParameterSetName -eq '__AllParameterSets') + } + + $parameterAttribute = $parameterAttributes | + Where-Object -FilterScript { $_.ParameterSetName -eq $ParameterSetName } | + Select-Object -First 1 + + if (-not $parameterAttribute) + { + $parameterAttribute = $parameterAttributes | + Where-Object -FilterScript { $_.ParameterSetName -eq '__AllParameterSets' } | + Select-Object -First 1 + }
393-403: Add WhatIf coverage to assert ShouldProcess is honoredA -WhatIf scenario ensures Create() is not invoked and prevents regressions in ShouldProcess wiring.
Proposed test block to add near this context:
Context 'When running with WhatIf' { BeforeAll { $script:mockSecurePassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force } It 'Should not create a SQL login when WhatIf is used' { $result = New-SqlDscLogin -ServerObject $script:mockServerObject -Name 'WhatIfLogin' -SqlLogin -SecurePassword $script:mockSecurePassword -PassThru -WhatIf $result | Should -Not -BeNullOrEmpty $result.MockCreateCalled | Should -BeFalse Should -Invoke -CommandName Test-SqlDscIsLogin -Exactly -Times 1 -Scope It } }I can send a follow-up PR commit if you want this added.
506-538: Add negative path for password policy failure to improve error-surface coverageThe SMO stub throws a nested FailedOperationException when the password equals 'pw'. Assert that the module surfaces a terminating error with a meaningful message.
Proposed test to append in this context:
It 'Should throw a terminating error on password policy failure' { $weakPw = ConvertTo-SecureString -String 'pw' -AsPlainText -Force { New-SqlDscLogin -ServerObject $mockServerObject -Name 'WeakPwLogin' -SqlLogin -SecurePassword $weakPw } | Should -Throw -ExpectedMessage '*password*policy*' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
tests/**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/**/*.ps1: All tests must use Pester and Pester v5.0 syntax
Do not test parameter validation
Test code must not exist outside of a Describe block
Each test file must contain exactly one Describe block named after the subject (public command, private function, or resource)
Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Use BeforeAll/BeforeEach/AfterAll/AfterEach inside Context, placed near the It blocks using them; duplication across contexts is acceptable; AfterAll can clean up; use BeforeEach/AfterEach sparingly
Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Files that need to be mocked in tests must be created in Pester's $TestDrive
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/**/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/**/*.Tests.ps1: Never test, mock, or use Should -Invoke for Write-Verbose or Write-Debug in unit tests
Do not use Should -Not -Throw; invoke the command directly
Include the standard unit test setup block (BeforeDiscovery/BeforeAll/AfterAll with module loading and PSDefaultParameterValues) before the Describe block
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
tests/Unit/Public/*.Tests.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
tests/Unit/Public/*.Tests.ps1: Unit tests for public commands must be placed in tests/Unit/Public and named .Tests.ps1
Public command unit tests must include parameter set validation using the provided template/pattern
Public command unit tests must include tests to validate parameter properties (e.g., Mandatory, ValueFromPipeline)
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
**/*.ps1
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.ps1: PowerShell files must be UTF-8 without BOM and end with a newline
Try to limit lines to 120 characters
Use 4 spaces for indentation; never use tabs
Use '#' for single-line comments
Use <# ... #> for multiline comments; indent the multiline text 4 spaces
Use descriptive, clear, full names for variables, parameters, and function names; names must be more than 2 characters; no abbreviations
Use camelCase for local variable names
Never use Write-Host
Never use backtick (`) for line continuation
Use splatting to reduce line length
PowerShell reserved keywords should be lowercase
Use single quotes for string literals whenever possible; use double quotes only when interpolation is needed
Hashtable properties should be PascalCase and each on a separate line
When comparing to $null, place $null on the left side of the comparison
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit Configuration File
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: List each return type (one per line) with a 1‑line description. Must match both [OutputType()] and actual ...
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit Configuration File
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- One
Describeblock per file matching the tested entity name- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test
Write-Verbose,Write-Debug, or parameter binding behavior- Pass all mandatory parameters to avoid prompts
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEachItdescriptions start with 'Should'Contextdescriptions start with 'When'- Mock variables prefix: 'mock'
- Prefer
-BeTrue/-BeFalseover-Be $true/-Be $false- No
Should -Not -Throw- invoke commands directlyFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachallowed onContextandItblocks- Keep scope close to usage context
Best Practices
- Assign unused return objects to
$null- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Cover all scenarios and code paths
- Use
BeforeEachandAfterEachsparingly
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
📚 Learning: 2025-08-18T13:50:53.762Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.762Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Public command unit tests must include parameter set validation using the provided template/pattern
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Public command unit tests must include tests to validate parameter properties (e.g., Mandatory, ValueFromPipeline)
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Do not use Should -Not -Throw; invoke the command directly
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-18T10:44:38.958Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:254-257
Timestamp: 2025-08-18T10:44:38.958Z
Learning: When implementing Force and Confirm parameter interaction in PowerShell functions with SupportsShouldProcess, use the pattern `if ($Force.IsPresent -and -not $Confirm)` to suppress confirmations. This ensures that explicit `-Confirm:$true` is respected even when `-Force` is present, preserving user intent when they explicitly request confirmation alongside Force.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T10:03:59.993Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:83-86
Timestamp: 2025-08-17T10:03:59.993Z
Learning: In PowerShell functions with SupportsShouldProcess, the $Confirm automatic variable defaults to $false when no -Confirm parameter is passed, and only becomes $true when -Confirm or -Confirm:$true is explicitly passed. The logic `if ($Force.IsPresent -and -not $Confirm)` correctly handles both cases where no -Confirm is passed and where -Confirm:$false is explicitly passed.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Do not test parameter validation
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-16T13:22:15.230Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2134
File: tests/Unit/Public/Get-SqlDscLogin.Tests.ps1:78-93
Timestamp: 2025-08-16T13:22:15.230Z
Learning: In PowerShell unit tests for parameter validation in SqlServerDsc, accessing parameter attributes directly like `$cmd.Parameters['ParameterName'].Attributes.Mandatory` works correctly because PowerShell automatically iterates through the array and returns the property values. Additional filtering to ParameterAttribute is not necessary when the parameter structure is known and controlled.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
🧬 Code Graph Analysis (1)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (1)
tests/Unit/Stubs/SMO.cs (6)
Login(354-484)LoginType(29-39)Create(439-479)LoginCreateOptions(18-23)Create(421-430)ServerConnection(730-736)
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (3)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (3)
27-39: Great test harness bootstrap and module loadThe standard unit test setup is solid: dependency resolution with DscResource.Test, -ErrorAction 'Stop' on Import-Module, SMO stubs loaded, and PSDefaultParameterValues set for Mock/Should.
51-106: Parameter set and command metadata validation are comprehensiveGood coverage for parameter sets and CmdletBinding metadata (SupportsShouldProcess, ConfirmImpact, DefaultParameterSetName). This aligns well with repo guidelines.
289-391: Creation path tests are meaningful and verify Create() is invokedThe PassThru checks and MockCreateCalled assertions on Windows, certificate, and asymmetric key scenarios meaningfully validate creation paths, not just absence of exceptions.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1 (1)
77-86: Clean up the created login in the WhatIf scenario to avoid test residueThis It leaves the login behind. Add explicit cleanup so the environment is left clean even when this test runs in isolation or on reruns.
It 'Should handle WhatIf parameter correctly' { # Ensure the login exists for this scenario (don't rely on other Its) $null = $script:serverObject | New-SqlDscLogin -Name $script:testLoginName -SqlLogin -SecurePassword $script:testLoginPassword -Force # Use WhatIf - login should not be removed $script:serverObject | Remove-SqlDscLogin -Name $script:testLoginName -WhatIf # Verify the login still exists $loginExists = Test-SqlDscIsLogin -ServerObject $script:serverObject -Name $script:testLoginName $loginExists | Should -BeTrue + + # Cleanup to leave environment in a known state + $script:serverObject | Remove-SqlDscLogin -Name $script:testLoginName -Force }Optional: wrap the body in try/finally and perform cleanup in finally for robustness.
♻️ Duplicate comments (1)
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1 (1)
149-156: Nice: local SecureString scope matches earlier guidanceUsing a local variable inside the It block avoids scope bleed and aligns with previous feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit Configuration File
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: List each return type (one per line) with a 1‑line description. Must match both [OutputType()] and actual ...
Files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit Configuration File
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- One
Describeblock per file matching the tested entity name- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test
Write-Verbose,Write-Debug, or parameter binding behavior- Pass all mandatory parameters to avoid prompts
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEachItdescriptions start with 'Should'Contextdescriptions start with 'When'- Mock variables prefix: 'mock'
- Prefer
-BeTrue/-BeFalseover-Be $true/-Be $false- No
Should -Not -Throw- invoke commands directlyFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachallowed onContextandItblocks- Keep scope close to usage context
Best Practices
- Assign unused return objects to
$null- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Cover all scenarios and code paths
- Use
BeforeEachandAfterEachsparingly
Files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1
⚙️ CodeRabbit Configuration File
tests/[iI]ntegration/**/*.[iI]ntegration.[tT]ests.ps1: # Integration Tests GuidelinesRequirements
- Location Commands:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1- Location Resources:
tests/Integration/Resources/{ResourceName}.Integration.Tests.ps1- No mocking - real environment only
- Cover all scenarios and code paths
- Use
Get-ComputerNamefor computer names in CI- Avoid
ExpectedMessageforShould -Throwassertions- Only run integration tests in CI unless explicitly instructed.
- Call commands with
-Forceparameter where applicable (avoids prompting).Required Setup Block
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.' } } BeforeAll { $script:dscModuleName = 'SqlServerDsc' Import-Module -Name $script:dscModuleName -Force -ErrorAction 'Stop' }
Files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Applied to files:
tests/Integration/Commands/Remove-SqlDscLogin.Integration.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). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (1)
tests/Integration/Commands/Remove-SqlDscLogin.Integration.Tests.ps1 (1)
89-93: LGTM: Good defensive setup for the negative testEnsuring non-existence upfront makes the Should -Throw assertion reliable and idempotent.
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r20, all commit messages.
Reviewable status:
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (4)
265-272: Prefer specific ParameterSetName attribute before falling back to __AllParameterSetsCurrent selection may grab __AllParameterSets first and mask per-set differences. Select the specific set first, then fall back.
- $parameterAttribute = $parameter.Attributes | - Where-Object -FilterScript { - $_ -is [System.Management.Automation.ParameterAttribute] -and - ($_.ParameterSetName -eq $ParameterSetName -or $_.ParameterSetName -eq '__AllParameterSets') - } | - Select-Object -First 1 + $parameterAttributes = $parameter.Attributes | + Where-Object -FilterScript { + $_ -is [System.Management.Automation.ParameterAttribute] -and + ($_.ParameterSetName -eq $ParameterSetName -or $_.ParameterSetName -eq '__AllParameterSets') + } + + $parameterAttribute = $parameterAttributes | + Where-Object -FilterScript { $_.ParameterSetName -eq $ParameterSetName } | + Select-Object -First 1 + + if (-not $parameterAttribute) + { + $parameterAttribute = $parameterAttributes | + Where-Object -FilterScript { $_.ParameterSetName -eq '__AllParameterSets' } | + Select-Object -First 1 + }
463-471: Compare LoginType consistently (avoid string-vs-enum)LoginType is an enum in the stub; comparing to 'SqlLogin' is type-mismatched. Compare to the enum or call ToString().
- $login.LoginType | Should -Be 'SqlLogin' + $login.LoginType.ToString() | Should -Be 'SqlLogin'Alternatively:
$login.LoginType | Should -Be ([Microsoft.SqlServer.Management.Smo.LoginType]::SqlLogin)
487-493: Same enum comparison issue in hashed-login scenarioAlign with the change above to prevent brittle comparisons.
- $login.LoginType | Should -Be 'SqlLogin' + $login.LoginType.ToString() | Should -Be 'SqlLogin'Alternatively:
$login.LoginType | Should -Be ([Microsoft.SqlServer.Management.Smo.LoginType]::SqlLogin)
289-539: Add WhatIf coverage to assert ShouldProcess is honored (no Create when -WhatIf is used)A -WhatIf scenario guards the ShouldProcess wiring. Recommend adding a context that verifies no Create occurred while the path executes normally.
Add this new context near the other creation tests:
Context 'When running with WhatIf' { BeforeAll { $script:mockSecurePassword = ConvertTo-SecureString -String 'P@ssw0rd1' -AsPlainText -Force } It 'Should not create a SQL login when WhatIf is used' { $result = New-SqlDscLogin -ServerObject $script:mockServerObject -Name 'WhatIfLogin' -SqlLogin -SecurePassword $script:mockSecurePassword -PassThru -WhatIf $result | Should -Not -BeNullOrEmpty $result.MockCreateCalled | Should -BeFalse Should -Invoke -CommandName Test-SqlDscIsLogin -Exactly -Times 1 -Scope It } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
**/*.ps?(m|d)1
⚙️ CodeRabbit Configuration File
**/*.ps?(m|d)1: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Multi-line: each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description.
- OUTPUTS: List each return type (one per line) with a 1‑line description. Must match both [OutputType()] and actual ...
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
**/*.[Tt]ests.ps1
⚙️ CodeRabbit Configuration File
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- One
Describeblock per file matching the tested entity name- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test
Write-Verbose,Write-Debug, or parameter binding behavior- Pass all mandatory parameters to avoid prompts
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEachItdescriptions start with 'Should'Contextdescriptions start with 'When'- Mock variables prefix: 'mock'
- Prefer
-BeTrue/-BeFalseover-Be $true/-Be $false- No
Should -Not -Throw- invoke commands directlyFile Organization
- Class resources:
tests/Unit/Classes/{Name}.Tests.ps1- Public commands:
tests/Unit/Public/{Name}.Tests.ps1- Private functions:
tests/Unit/Private/{Name}.Tests.ps1Data-Driven Tests
- Define variables in separate
BeforeDiscoveryfor-ForEach(close to usage)-ForEachallowed onContextandItblocks- Keep scope close to usage context
Best Practices
- Assign unused return objects to
$null- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Cover all scenarios and code paths
- Use
BeforeEachandAfterEachsparingly
Files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Public command unit tests must include parameter set validation using the provided template/pattern
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Public command unit tests must include tests to validate parameter properties (e.g., Mandatory, ValueFromPipeline)
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Do not use Should -Not -Throw; invoke the command directly
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-18T10:44:38.990Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: source/Public/New-SqlDscLogin.ps1:254-257
Timestamp: 2025-08-18T10:44:38.990Z
Learning: When implementing Force and Confirm parameter interaction in PowerShell functions with SupportsShouldProcess, use the pattern `if ($Force.IsPresent -and -not $Confirm)` to suppress confirmations. This ensures that explicit `-Confirm:$true` is respected even when `-Force` is present, preserving user intent when they explicitly request confirmation alongside Force.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T10:03:59.993Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:83-86
Timestamp: 2025-08-17T10:03:59.993Z
Learning: In PowerShell functions with SupportsShouldProcess, the $Confirm automatic variable defaults to $false when no -Confirm parameter is passed, and only becomes $true when -Confirm or -Confirm:$true is explicitly passed. The logic `if ($Force.IsPresent -and -not $Confirm)` correctly handles both cases where no -Confirm is passed and where -Confirm:$false is explicitly passed.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Use localized strings in tests only when necessary, retrieving values from $script:localizedData inside InModuleScope
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Do not test parameter validation
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Each scenario/path should be in Context blocks starting with 'When'; It block descriptions should start with 'Should' and must call the subject; results and assertions stay within the same It; BeforeAll/BeforeEach must not call the subject
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-16T13:22:15.230Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2134
File: tests/Unit/Public/Get-SqlDscLogin.Tests.ps1:78-93
Timestamp: 2025-08-16T13:22:15.230Z
Learning: In PowerShell unit tests for parameter validation in SqlServerDsc, accessing parameter attributes directly like `$cmd.Parameters['ParameterName'].Attributes.Mandatory` works correctly because PowerShell automatically iterates through the array and returns the property values. Additional filtering to ParameterAttribute is not necessary when the parameter structure is known and controlled.
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Use BeforeAll/BeforeEach/AfterAll/AfterEach inside Context, placed near the It blocks using them; duplication across contexts is acceptable; AfterAll can clean up; use BeforeEach/AfterEach sparingly
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Test code must not exist outside of a Describe block
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Include the standard unit test setup block (BeforeDiscovery/BeforeAll/AfterAll with module loading and PSDefaultParameterValues) before the Describe block
Applied to files:
tests/Unit/Public/New-SqlDscLogin.Tests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/**/*.ps1 : Each test file must contain exactly one Describe block named after the subject (public command, private function, or resource)
Applied to files:
tests/Unit/Public/New-SqlDscLogin.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). (4)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Determine DSC Resource Test Requirements)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Unit)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (2)
tests/Unit/Public/New-SqlDscLogin.Tests.ps1 (2)
5-25: Solid bootstrap of DscResource.Test with fail-fast importGood defensive fallback to build script and -ErrorAction Stop on import. This follows the repo pattern and improves reliability.
27-39: Good module load and SMO stub wiringImport with -ErrorAction Stop and explicit Add-Type for SMO stubs are spot on for deterministic tests.
…on tests run correctly
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
azure-pipelines.yml (1)
95-99: Normalize PR target branch by stripping refs/heads/ and using $(defaultBranch) fallback.SYSTEM_PULLREQUEST_TARGETBRANCH often includes a refs/heads/ prefix. Prefixing origin/ without stripping can produce invalid refs (e.g., origin/refs/heads/main). Use $(defaultBranch) as the fallback, not a hardcoded name.
Apply:
- if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { - $targetBranch = "origin/$env:SYSTEM_PULLREQUEST_TARGETBRANCH" - } else { - $targetBranch = "origin/$(defaultBranch)" - } + if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { + $targetBranchName = $env:SYSTEM_PULLREQUEST_TARGETBRANCH -replace '^refs/heads/', '' + $targetBranch = "origin/$targetBranchName" + } else { + $targetBranch = "origin/$(defaultBranch)" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
CHANGELOG.md(2 hunks)azure-pipelines.yml(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
CHANGELOG.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Always update the Unreleased section in CHANGELOG.md using Keep a Changelog format with concrete release notes for changes (new/modified commands, functions, resources)
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe main changes as concise release notes
- Reference issues using format #<issue_number>
- No empty lines between list items in same section
- Do not add item if there are already an existing item for the same change
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Markdown: wrap lines at word boundaries when exceeding 80 characters; use 2 spaces for indentation
Files:
CHANGELOG.md
⚙️ CodeRabbit Configuration File
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Surround fenced code blocks with blank lines
Files:
CHANGELOG.md
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.mdazure-pipelines.yml
azure-pipelines.yml
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Files:
azure-pipelines.yml
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Applied to files:
CHANGELOG.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Public/*.ps1 : Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
CHANGELOG.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Private/*.ps1 : Private functions must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
CHANGELOG.md
📚 Learning: 2025-08-17T10:48:15.384Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/suffix.ps1:24-24
Timestamp: 2025-08-17T10:48:15.384Z
Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
CHANGELOG.mdazure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named <Command>.Integration.Tests.ps1
Applied to files:
CHANGELOG.mdazure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Unit tests for public commands must be placed in tests/Unit/Public and named <Command>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Classes/*.Tests.ps1 : Unit tests for class-based resources must be placed in tests/Unit/Classes and named <Resource>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Private/*.Tests.ps1 : Unit tests for private functions must be placed in tests/Unit/Private and named <Function>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : In CI, integration tests must use Get-ComputerName when the computer name is needed
Applied to files:
azure-pipelines.yml
🧬 Code Graph Analysis (1)
CHANGELOG.md (1)
tests/Unit/Stubs/SMO.cs (1)
Login(354-484)
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (3)
azure-pipelines.yml (1)
285-289: Good: New-SqlDscLogin integration test wired into Group 2.The new command’s test is correctly added alongside related login commands in the SQL Server Commands stage.
CHANGELOG.md (2)
12-14: Changelog fix entry reads clearly and matches the pipeline change.The description concisely explains the boolean condition correction for DSC resource integration test gating.
43-49: Changelog entry for New-SqlDscLogin looks complete and aligned with module conventions.Covers purpose, supported login types, and parameter set constraints. Nicely scoped.
… use lowercase comparison
…for consistent comparison in pipeline conditions
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
azure-pipelines.yml (1)
95-99: Normalize PR target branch: strip refs/heads/ before prefixing with origin/SYSTEM_PULLREQUEST_TARGETBRANCH often includes a refs/heads/ prefix. Without stripping, the computed ref becomes origin/refs/heads/, which may not match what Test-ShouldRunDscResourceIntegrationTests.ps1 expects. Also prefer $(defaultBranch) for fallback (you already do).
Apply this diff:
- if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { - $targetBranch = "origin/$env:SYSTEM_PULLREQUEST_TARGETBRANCH" - } else { - $targetBranch = "origin/$(defaultBranch)" - } + if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { + $targetBranchName = $env:SYSTEM_PULLREQUEST_TARGETBRANCH -replace '^refs/heads/', '' + $targetBranch = "origin/$targetBranchName" + } else { + $targetBranch = "origin/$(defaultBranch)" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
azure-pipelines.yml(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
azure-pipelines.yml
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Files:
azure-pipelines.yml
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
azure-pipelines.yml
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named <Command>.Integration.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Unit tests for public commands must be placed in tests/Unit/Public and named <Command>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Classes/*.Tests.ps1 : Unit tests for class-based resources must be placed in tests/Unit/Classes and named <Resource>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Private/*.Tests.ps1 : Unit tests for private functions must be placed in tests/Unit/Private and named <Function>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : In CI, integration tests must use Get-ComputerName when the computer name is needed
Applied to files:
azure-pipelines.yml
⏰ 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). (2)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (2)
azure-pipelines.yml (2)
107-109: LGTM: helpful debug echo before exporting output variablePrinting the computed ShouldRunDscResourceIntegrationTests value aids troubleshooting and does not affect behavior.
311-311: New-SqlDscLogin integration tests correctly addedAll verification steps passed—no further changes required.
…te conditions for consistency
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
azure-pipelines.yml (2)
95-99: Strip refs/heads/ from PR target branch before prefixing with origin/SYSTEM_PULLREQUEST_TARGETBRANCH often includes refs/heads/*. Using it verbatim yields origin/refs/heads/, which most git commands don’t accept. Strip the prefix before composing targetBranch. Also keep the fallback to $(defaultBranch).
- if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { - $targetBranch = "origin/$env:SYSTEM_PULLREQUEST_TARGETBRANCH" - } else { - $targetBranch = "origin/$(defaultBranch)" - } + if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { + $targetBranchName = $env:SYSTEM_PULLREQUEST_TARGETBRANCH -replace '^refs/heads/', '' + $targetBranch = "origin/$targetBranchName" + } else { + $targetBranch = "origin/$(defaultBranch)" + }
120-142: Gate the debug job on System.Debug to reduce noiseThis always-on diagnostic job adds noise and time to normal runs. Gate it on System.Debug so it only runs when troubleshooting.
- job: Debug_DSC_Resource_Test_Flag displayName: 'Debug: DSC Resource Test Flag' dependsOn: Determine_DSC_Resource_Test_Requirements + condition: and(succeeded(), eq(lower(variables['System.Debug']), 'true')) pool: vmImage: 'windows-latest'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
azure-pipelines.yml(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
azure-pipelines.yml
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Files:
azure-pipelines.yml
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
azure-pipelines.yml
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named <Command>.Integration.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Unit tests for public commands must be placed in tests/Unit/Public and named <Command>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Classes/*.Tests.ps1 : Unit tests for class-based resources must be placed in tests/Unit/Classes and named <Resource>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Private/*.Tests.ps1 : Unit tests for private functions must be placed in tests/Unit/Private and named <Function>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : In CI, integration tests must use Get-ComputerName when the computer name is needed
Applied to files:
azure-pipelines.yml
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (2)
azure-pipelines.yml (2)
107-118: Good: normalize ShouldRun to lowercase and export both long and short outputsLowercasing once at the source avoids brittle case handling in YAML expressions and across stages. The dual outputs (long + short alias) improve readability.
317-319: LGTM: added New-SqlDscLogin integration tests to Commands groupCorrect path and placement (Group 2) alongside related login commands. This satisfies the guideline to wire new public command integration tests into CI.
…ceIntegrationTests and simplify output variable handling
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
azure-pipelines.yml (2)
95-98: Normalize SYSTEM_PULLREQUEST_TARGETBRANCH (strip refs/heads/) and keep defaultBranch fallbackCurrent logic can yield origin/refs/heads/, which breaks git diff. Strip the prefix before composing the remote ref. This mirrors earlier guidance.
- if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { - $targetBranch = "origin/$env:SYSTEM_PULLREQUEST_TARGETBRANCH" - } else { - $targetBranch = "origin/$(defaultBranch)" - } + if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { + $targetBranchName = $env:SYSTEM_PULLREQUEST_TARGETBRANCH -replace '^refs/heads/', '' + $targetBranch = "origin/$targetBranchName" + } else { + $targetBranch = "origin/$(defaultBranch)" + }
118-137: Gate the debug job on System.Debug and fix misleading commentRun the debug job only when diagnostics are requested; also the comment about “both lowercase” is inaccurate and may mislead future edits.
- job: Debug_DSC_Resource_Test_Flag displayName: 'Debug: DSC Resource Test Flag' dependsOn: Determine_DSC_Resource_Test_Requirements + condition: and(succeeded(), eq(lower(variables['System.Debug']), 'true')) pool: vmImage: 'windows-latest' variables: - # Use the correct output format: step_name.variable_name (both lowercase) + # Use the correct output format: step_name.variable_name (match actual casing) ShouldRun: $[ dependencies.Determine_DSC_Resource_Test_Requirements.outputs['determineDscResourceTests.ShouldRunDscResourceIntegrationTests'] ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.build/Test-ShouldRunDscResourceIntegrationTests.ps1(2 hunks)azure-pipelines.yml(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
azure-pipelines.yml
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Files:
azure-pipelines.yml
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
azure-pipelines.yml
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/{Public,Private,Classes}/*.ps1 : Use PascalCase for function names and parameters in public commands, private functions, and class-based resources
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: tests/Unit/Public/Remove-SqlDscLogin.Tests.ps1:36-39
Timestamp: 2025-08-17T10:15:48.194Z
Learning: Public command unit tests guideline: Never use InModuleScope unless accessing localized strings from $script:localizedData. PSDefaultParameterValues for InModuleScope should be kept in public command tests to support localized string retrieval when necessary.
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Public/*.ps1 : Each public PowerShell command must be in its own script file named exactly as the command and located in source/Public
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/{Public,Private}/*.ps1 : Public commands and private functions must be advanced functions and include [CmdletBinding()]
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/{Public,Private}/*.ps1 : Public command and private function names must follow Verb-Noun format and use PowerShell approved verbs
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Public/*.ps1 : Public commands must localize Write-Debug/Verbose/Error/Warning and other messages using keys from source/en-US/SqlServerDsc.strings.psd1; keys prefixed with function name using underscores; data available as $script:localizedData
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/Private/*.ps1 : Each private function must be in its own script file named exactly as the function and located in source/Private (even if used by a single public command)
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named <Command>.Integration.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Unit tests for public commands must be placed in tests/Unit/Public and named <Command>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Classes/*.Tests.ps1 : Unit tests for class-based resources must be placed in tests/Unit/Classes and named <Resource>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Private/*.Tests.ps1 : Unit tests for private functions must be placed in tests/Unit/Private and named <Function>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : In CI, integration tests must use Get-ComputerName when the computer name is needed
Applied to files:
azure-pipelines.yml
⏰ 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). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (1)
azure-pipelines.yml (1)
314-314: New-SqlDscLogin integration test added in the correct group/orderGood placement alongside related login commands. This aligns with the guidelines for Commands integration tests.
… to use ShouldRunDscResourceIntegrationTests output variable
…ntegration_Test_Resources_SqlServer stage
There was a problem hiding this comment.
Actionable comments posted: 1> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limits.
🔭 Outside diff range comments (1)
azure-pipelines.yml (1)
444-451: Fix stage condition: incorrect dependency scope and mismatched parentheses
- At stage-level, use stageDependencies and include the job name when accessing outputs. dependencies here won’t resolve across stages.
- There’s one extra closing parenthesis, which will break the expression.
Apply this diff:
condition: | - and( - succeeded(), - eq(dependencies.Quality_Test_and_Unit_Test.outputs['Determine_DSC_Resource_Test_Requirements.determineDscResourceTests.ShouldRunDscResourceIntegrationTests'], 'true')) - ) + and( + succeeded(), + eq(stageDependencies.Quality_Test_and_Unit_Test.Determine_DSC_Resource_Test_Requirements.outputs['determineDscResourceTests.ShouldRunDscResourceIntegrationTests'], 'true') + )
♻️ Duplicate comments (1)
azure-pipelines.yml (1)
95-99: Normalize PR target branch (strip refs/heads) and use defaultBranch fallbackAs previously noted, SYSTEM_PULLREQUEST_TARGETBRANCH often includes a refs/heads/ prefix. Without stripping it, BaseBranch comparisons may fail.
Apply this diff:
- if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { - $targetBranch = "origin/$env:SYSTEM_PULLREQUEST_TARGETBRANCH" - } else { - $targetBranch = "origin/$(defaultBranch)" - } + if ($env:SYSTEM_PULLREQUEST_TARGETBRANCH) { + $targetBranchName = $env:SYSTEM_PULLREQUEST_TARGETBRANCH -replace '^refs/heads/', '' + $targetBranch = "origin/$targetBranchName" + } else { + $targetBranch = "origin/$(defaultBranch)" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
azure-pipelines.yml(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
azure-pipelines.yml
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Files:
azure-pipelines.yml
**
⚙️ CodeRabbit Configuration File
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow
- Run project scripts in PowerShell from repository root
- Build after source changes:
.\build.ps1 -Tasks build- Test workflow: Build →
Invoke-Pester -Path @('<test paths>') -Output Detailed- New session required after class changes
File Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
azure-pipelines.yml
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to source/en-US/SqlServerDsc.strings.psd1 : All localized strings for public commands and private functions must be added to source/en-US/SqlServerDsc.strings.psd1 following existing key patterns
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2136
File: source/Public/Remove-SqlDscLogin.ps1:104-108
Timestamp: 2025-08-17T10:13:30.079Z
Learning: In SqlServerDsc unit tests, SMO object stubs (like Login objects) should have properly mocked Parent properties with correct Server stub types and valid InstanceName values, rather than handling null Parent scenarios in production code.
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
Learnt from: johlju
PR: dsccommunity/SqlServerDsc#2132
File: tests/Unit/Public/New-SqlDscLogin.Tests.ps1:0-0
Timestamp: 2025-08-18T13:50:53.789Z
Learning: In SqlServerDsc unit tests, SMO stub objects can be used to verify method calls like Create() on Login objects by adding mock verifications with Should -Invoke, providing more robust testing than just checking for no exceptions.
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to azure-pipelines.yml : Add each public command integration test script to a group within the Integration_Test_Commands_SqlServer stage in ./azure-pipelines.yml, choosing the appropriate group
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Integration tests must exist for all public commands, not mock any commands, be placed at tests/Integration/Commands, and be named <Command>.Integration.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Public/*.Tests.ps1 : Unit tests for public commands must be placed in tests/Unit/Public and named <Command>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : Include the standard integration test setup block (BeforeDiscovery/BeforeAll with module loading) before the first Describe block
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Classes/*.Tests.ps1 : Unit tests for class-based resources must be placed in tests/Unit/Classes and named <Resource>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Unit/Private/*.Tests.ps1 : Unit tests for private functions must be placed in tests/Unit/Private and named <Function>.Tests.ps1
Applied to files:
azure-pipelines.yml
📚 Learning: 2025-08-17T09:48:55.756Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T09:48:55.756Z
Learning: Applies to tests/Integration/Commands/*.Integration.Tests.ps1 : In CI, integration tests must use Get-ComputerName when the computer name is needed
Applied to files:
azure-pipelines.yml
⏰ 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). (2)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (2)
azure-pipelines.yml (2)
107-116: Good: output normalization for gating is robustLowercasing ShouldRunDscResourceIntegrationTests before exporting avoids brittle case issues in conditions. No changes needed here.
293-293: Correct: Added New-SqlDscLogin integration test to Commands groupThe new public command integration test is properly added under Integration_Test_Commands_SqlServer (Group 2), matching the project guidelines and the retrieved learning.
…multiple integration test stages
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r21, 1 of 2 files at r25, 1 of 1 files at r27, all commit messages.
Reviewable status:
1 participant
Pull Request (PR) description
New-SqlDscLoginEngine instance.
logins, certificate-based logins, and asymmetric key-based logins.
This Pull Request (PR) fixes the following issues
None.
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is