-
Notifications
You must be signed in to change notification settings - Fork 227
SqlRS: Validate Configuration.ServiceName for SQL Server 14+ #2340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR adds a validation check to SqlRS that ensures Configuration.ServiceName is not null or empty for SQL Server version 14 and higher. It includes a new error message string and corresponding unit tests to verify the validation behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
Co-authored-by: johlju <[email protected]>
Co-authored-by: johlju <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1 (1)
9-9: Optional: consider key‑naming alignment for localization.Guidelines prefer underscores for multi‑word keys; this file uses CamelCase (e.g., ReportingServicesNotFound). Keep as‑is for consistency now, but consider a future, sweeping rename (with find/replace + tests) to a uniform style across this resource.
Based on learnings
tests/Unit/DSC_SqlRS.Tests.ps1 (1)
746-755: Consider moving mock to BeforeAll for consistency.The mock is defined inside the
Itblock to override theBeforeAllmock for testing the empty string scenario. While this works, consider using a data-driven approach with-ForEachto keep all mocks inBeforeAll:Context 'When Configuration.ServiceName is null or empty for SQL Server version 14 and higher' -ForEach @( @{ ServiceNameValue = $null; TestCase = 'null' } @{ ServiceNameValue = ''; TestCase = 'empty' } ) { BeforeAll { # ... existing setup ... $mockGetCimInstance_ConfigurationSetting = { return @( ( New-Object -TypeName Microsoft.Management.Infrastructure.CimInstance -ArgumentList @( 'MSReportServer_ConfigurationSetting' "root/Microsoft/SQLServer/ReportServer/RS_SQL2016/v$TestCaseVersion/Admin" ) | Add-Member -MemberType NoteProperty -Name 'ServiceName' -Value $ServiceNameValue -PassThru -Force ) ) } Mock -CommandName Get-ReportingServicesData -MockWith { return @{ Configuration = (& $mockGetCimInstance_ConfigurationSetting)[0] ReportsApplicationName = 'ReportServerWebApp' SqlVersion = $TestCaseVersion } } } It "Should throw the correct error message when ServiceName is $TestCase" { # ... test implementation ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1(1 hunks)source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1(1 hunks)tests/Unit/DSC_SqlRS.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
source/DSCResources/**/en-US/DSC_*.strings.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
Each resource directory must contain an en-US localization folder with a strings file named DSC_.strings.psd1
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1: Additional localization folders must be named using Get-UICulture names (e.g., fr-FR) and contain DSC_.strings.psd1
In .strings.psd1 files, use underscores as word separators in localized string key names for multi-word keys
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Use 4 spaces for indentation (no tabs)
No spaces on empty lines
Try to limit lines to 120 characters
End files with exactly one blank line
Use line endings as defined by .gitattributes
Allow a maximum of two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
**/*.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
In module manifests, do not use NestedModules for shared commands without RootModule
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile 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
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Unit/DSC_SqlRS.Tests.ps1CHANGELOG.mdsource/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # 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:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)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:
@{}- 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...
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
tests/[Uu]nit/**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
tests/[Uu]nit/**/*.[Tt]ests.ps1: In unit tests, access localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
When mocking files in tests, use the $TestDrive variable for file system operations
All public commands must include parameter set validation tests
Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Use the provided Parameter Set Validation test template to assert command ParameterSetName and full parameter list string; for multiple parameter sets, supply multiple hashtables via -ForEach
Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[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 noop" first.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
**/*.{ps1,psm1}: Public PowerShell commands must follow the {Verb}-SqlDsc{Noun} naming format
Private PowerShell functions must follow the {Verb}-{Noun} naming format
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Functions must use PascalCase with approved Verb-Noun format
Parameters use PascalCase
Variables use camelCase
Keywords are lower-case
Classes use PascalCase
Include scope for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Newline before opening brace except for variable assignments
One newline after opening brace
Two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed
Arrays single-line format: @('one','two','three')
Multi-line arrays: one element per line with proper indentation
Do not use unary comma in return statements to force an array
Empty hashtable as @{}
Hashtable properties each on their own line with proper indentation
Hashtable property names use PascalCase
Single-line comments start with #, capitalized, on their own line
Multi-line comments use <# #>, brackets on their own lines, text indented
No commented-out code
Always add comment-based help to all functions and scripts
Comment-based help must include 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 in comment-based help
INPUTS: list each pipeline-accepted type with one-line description; repeat keyword per type
OUTPUTS: list each return type with one-line description; repeat keyword per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version), ≤2 short sentences
Av...
Files:
tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
**/*.{ps1,psm1,cs}
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
Prefer SMO over T-SQL for SQL Server interactions
Files:
tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
tests/Unit/**/*.Tests.ps1
📄 CodeRabbit inference engine (.github/instructions/SqlServerDsc-guidelines.instructions.md)
tests/Unit/**/*.Tests.ps1: Unit tests must not mock real SMO types; use SMO stub types from SMO.cs
Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Unit tests that reference SMO types must load stubs via Add-Type -Path "$PSScriptRoot/../Stubs/SMO.cs"
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: 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
Place executable test code only inside Describe blocks
Place assertions only in It blocks
Do not test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters in tests to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Invoke the tested entity from within It blocks
Keep result capture and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach instead
Avoid unnecessary remove/recreate cycles in tests
Have exactly one Describe block per file, and name it to match the tested entity
Context descriptions must start with 'When'
It descriptions must start with 'Should' and must not contain 'when'
Prefix variables used for mocks with 'mock'
Create a separate Context block for each scenario
Use nested Context blocks for complex scenarios
Define mocks in BeforeAll; use BeforeEach only when required
Place setup/teardown in the nearest appropriate BeforeAll, BeforeEach, AfterAll, or AfterEach to where it’s used
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use 'Should -Not -Throw'; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when the mock should return $null
Set $PSDefaultParameterValues entries for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in BeforeDiscovery (close to usage)
Use -ForEach only on Context and It blocks
Do not add param() inside Pester blocks when using -ForEach
Access test case properties directl...
Files:
tests/Unit/DSC_SqlRS.Tests.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
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- 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,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown documents
Use '1.' for all items in ordered lists (1/1/1 numbering style)
Disable MD013 for tables/code blocks exceeding 80 characters via an inline comment
Require empty lines before and after code blocks and headings (except before line 1)
Escape backslashes in file paths only, not inside code blocks
All fenced code blocks must specify a language identifier
Format parameter names as bold
Format values/literals as inline code
Format resource/module/product names as italic
Format commands, file names, and paths as inline code
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format
Describe notable changes briefly, with no more than 2 items per change type
Reference issues using the format issue #<issue_number>
No empty lines between list items in the same section
Skip adding an entry if the same change already exists in the Unreleased section
No duplicate sections or items in the Unreleased sectionAlways update the Unreleased section of CHANGELOG.md
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
source/DSCResources/**/*.psm1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
source/DSCResources/**/*.psm1: Every MOF DSC resource module must define the functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource
Export resource functions using the *-TargetResource naming pattern
Get-TargetResource must return a hashtable containing all resource properties
Test-TargetResource must return a boolean ($true/$false)
Set-TargetResource must not return anything (void)
Get-TargetResource parameters: include only those needed to retrieve actual current state values
Get-TargetResource: remove non-mandatory parameters that are never used
Set-TargetResource and Test-TargetResource must have identical parameters
For unused mandatory parameters in Set-TargetResource or Test-TargetResource, add a help comment: "Not used in <function_name>"
Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call
Get-TargetResource: Write-Verbose message should start with "Getting the current state of..."
Set-TargetResource: Write-Verbose message should start with "Setting the desired state of..."
Test-TargetResource: Write-Verbose message should start with "Determining the current state of..."
Use localized strings for all messages (e.g., Write-Verbose, Write-Error)
Import localized strings at the top of the module using Get-LocalizedData
Use try/catch blocks to handle exceptions in MOF-based resources
Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
⚙️ CodeRabbit configuration file
source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources GuidelinesRequired Functions
- Every DSC resource must define:
Get-TargetResource,Set-TargetResource,Test-TargetResource- Export using
*-TargetResourcepatternFunction Return Types
Get-TargetResource: Must return hashtable with all resource propertiesTest-TargetResource: Must return boolean ($true/$false)Set-TargetResource: Must not return anything (void)Parameter Guidelines
Get-TargetResource: Only include parameters needed to retrieve actual current state valuesGet-TargetResource: Remove non-mandatory parameters that are never usedSet-TargetResourceandTest-TargetResource: Must have identical parametersSet-TargetResourceandTest-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help commentRequired Elements
- Each function must include
Write-Verboseat least once
Get-TargetResource: Use verbose message starting with "Getting the current state of..."Set-TargetResource: Use verbose message starting with "Setting the desired state of..."Test-TargetResource: Use verbose message starting with "Determining the current state of..."- Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
- Import localized strings using
Get-LocalizedDataat module topError Handling
- Do not use
throwfor terminating errors- Use
try/catchblocks to handle exceptions- Throw localized exceptions using the appropriate
New-*Exceptioncmdlet:
New‑InvalidDataExceptionNew-ArgumentExceptionNew-InvalidOperationException- [
New-ObjectNotFoundException](https:/...
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
🧠 Learnings (30)
📚 Learning: 2025-10-04T21:33:23.022Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 85
File: source/en-US/UpdateServicesDsc.strings.psd1:1-2
Timestamp: 2025-10-04T21:33:23.022Z
Learning: In UpdateServicesDsc (and similar DSC modules), the module-level localization file (source/en-US/<ModuleName>.strings.psd1) can be empty when DSC resources have their own localization files in source/DSCResources/<ResourceName>/en-US/DSC_<ResourceName>.strings.psd1. Automated tests verify localization completeness for resources. Don't flag empty module-level localization files as issues when resources have separate localization.
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use localized strings for all messages (e.g., Write-Verbose, Write-Error)
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Define strings using ConvertFrom-StringData with entries like `KeyName = Message with {0} placeholder. (PREFIX0001)`
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1 : Additional localization folders must be named using Get-UICulture names (e.g., fr-FR) and contain DSC_<ResourceName>.strings.psd1
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/en-US/DSC_*.strings.psd1 : Each resource directory must contain an en-US localization folder with a strings file named DSC_<ResourceName>.strings.psd1
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1 : In .strings.psd1 files, use underscores as word separators in localized string key names for multi-word keys
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/**/*.ps1 : Use localized string keys instead of hardcoded strings in script output/messages
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-08-29T17:22:23.268Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-localization.instructions.md:0-0
Timestamp: 2025-08-29T17:22:23.268Z
Learning: Applies to source/en-US/*.strings.psd1 : Store class localization in source/en-US/{ResourceClassName}.strings.psd1
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.
Applied to files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-08-17T10:48:15.384Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Unit tests must set $env:SqlServerDscCI = $true in BeforeAll and remove it in AfterAll
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to tests/Integration/**/*.Tests.ps1 : Integration tests must use Connect-SqlDscDatabaseEngine with correct CI credentials to open DB sessions
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource and Test-TargetResource must have identical parameters
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-08-17T10:13:30.079Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.397Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.397Z
Learning: Applies to tests/Integration/**/*.Tests.ps1 : Integration tests must call Disconnect-SqlDscDatabaseEngine after Connect-SqlDscDatabaseEngine
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource must not return anything (void)
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-10-10T14:01:42.703Z
Learnt from: dan-hughes
Repo: dsccommunity/UpdateServicesDsc PR: 86
File: tests/Unit/MSFT_UpdateServicesServer.Tests.ps1:6-45
Timestamp: 2025-10-10T14:01:42.703Z
Learning: For script-based (MOF) DSC resources in UpdateServicesDsc, unit tests should use Initialize-TestEnvironment with variables $script:dscModuleName and $script:dscResourceName, and set -ResourceType 'Mof'. This differs from class-based resource tests which use the simpler $script:moduleName template without Initialize-TestEnvironment.
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to tests/Unit/**/*.Tests.ps1 : Unit tests must not mock real SMO types; use SMO stub types from SMO.cs
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-09-16T16:34:44.689Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md:0-0
Timestamp: 2025-09-16T16:34:44.689Z
Learning: Applies to tests/[Uu]nit/**/*.[Tt]ests.ps1 : Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Use DSCSQLTEST (DB Engine), SSRS (Reporting Services), and PBIRS (Power BI Report Server) instances in CI environment
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1CHANGELOG.md
📚 Learning: 2025-08-17T10:15:48.194Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-08-18T13:50:53.789Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 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/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-10-26T13:27:07.728Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-pester.instructions.md:0-0
Timestamp: 2025-10-26T13:27:07.728Z
Learning: Applies to **/*.[Tt]ests.ps1 : Omit -MockWith when the mock should return $null
Applied to files:
tests/Unit/DSC_SqlRS.Tests.ps1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Set-TargetResource: Write-Verbose message should start with "Setting the desired state of..."
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Export resource functions using the *-TargetResource naming pattern
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Get-TargetResource: remove non-mandatory parameters that are never used
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Test-TargetResource must return a boolean ($true/$false)
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : For unused mandatory parameters in Set-TargetResource or Test-TargetResource, add a help comment: "Not used in <function_name>"
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-10-26T13:26:13.396Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-10-26T13:26:13.396Z
Learning: Applies to **/*.cs : In Database Engine resources, add InstanceName, ServerName, and Credential to $this.ExcludeDscProperties
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
📚 Learning: 2025-09-12T13:20:57.155Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-09-12T13:20:57.155Z
Learning: Applies to source/DSCResources/**/*.psm1 : Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call
Applied to files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
⏰ 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). (24)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Power BI Report Server Integration PowerBI_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - Power BI Report Server Integration PowerBI_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2022_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2022_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2019_WIN2025)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2019_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server (Prepared Image) Commands - Prepared Image SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Resources - SQL Server Integration SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Integration Test Commands - SQL Server Commands SQL2017_WIN2022)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test Publish Code Coverage)
- GitHub Check: dsccommunity.SqlServerDsc (Quality Test and Unit Test HQRM)
- 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 (Build Package Module)
🔇 Additional comments (4)
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1 (2)
9-9: LGTM: localized message with placeholder and error ID.Message text, placeholder
{0}, and(DSC_SQLRS0002)follow existing patterns.
9-9: Verification confirms proper error ID and key usage throughout the codebase.The search results validate all concerns:
DSC_SQLRS0002appears exactly once (strings file, line 9) with no duplicate reuseServiceNameIsNullOrEmptykey is properly defined, used in resource code (DSC_SqlRS.psm1:302 viaNew-InvalidOperationException), and tested in two assertions (DSC_SqlRS.Tests.ps1:741, 766)- Tests verify the thrown error includes the expected localized message containing the embedded error ID
CHANGELOG.md (1)
130-131: Code changes verified and CHANGELOG entry correct.The validation logic is properly implemented in the SqlRS resource (lines 300–302), the localization string with error ID
DSC_SQLRS0002exists, and unit tests confirm the behavior for SQL Server version 14 and higher with null or emptyConfiguration.ServiceName. The CHANGELOG entry is concise and properly references issue #2342.source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1 (1)
298-305: LGTM! Validation logic correctly prevents downstream failures.The validation check for
Configuration.ServiceNameis well-placed and correctly implemented. It uses the appropriate error handling pattern (New-InvalidOperationException) with a localized message, preventing downstream failures when retrieving the service account or restarting the service.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2340 +/- ##
====================================
Coverage 94% 94%
====================================
Files 154 154
Lines 9318 9321 +3
====================================
+ Hits 8826 8829 +3
Misses 492 492
🚀 New features to boost your workflow:
|
johlju
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johlju reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @copilot)
PR #2326 changed service name retrieval to use
Configuration.ServiceNamedynamically for SQL Server 14+. If this property returns null or empty, it causes downstream failures when retrieving the service account or restarting the service.Changes
Configuration.ServiceNameinSet-TargetResourcefor SQL Server version ≥14, throwing localizedInvalidOperationExceptionwith error ID DSC_SQLRS0002ServiceNameIsNullOrEmptyerror string toDSC_SqlRS.strings.psd1Resolves #2342
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
This change is