Enhance performance of DSC resource integration tests#2143
Enhance performance of DSC resource integration tests#2143johlju merged 6 commits intodsccommunity:mainfrom
Conversation
WalkthroughUpdates to docs, CI workflows, and build scripts: the test-gate script now matches changed files against a configurable SourcePath; README and CHANGELOG reflect Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CI as CI Workflow
participant Script as Test-ShouldRunDscResourceIntegrationTests.ps1
participant Repo as Repository
Dev->>CI: Push PR / changes
CI->>Script: Invoke test gate (passes configured SourcePath)
Script->>Repo: Retrieve list of changed files
Note over Script: Build prefix regex from SourcePath\n(normalize separators, escape regex, trim trailing slash)
Script->>Script: Match changed files against SourcePath prefix
alt Any match under SourcePath
Script-->>CI: ShouldRun = true
CI->>CI: Run DSC integration tests
else No match under SourcePath
Script-->>CI: ShouldRun = false
CI->>CI: Skip DSC integration tests
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
✨ 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 (
|
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @johlju)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.build/Test-ShouldRunDscResourceIntegrationTests.ps1 (1)
462-507: SourcePath-awareness is only applied to the early check; downstream regexes still hard-code 'source/'With a non-default SourcePath (e.g., src/), changes under that path won’t be detected by the DSCResources/Classes/Public/Private checks, causing false negatives and skipping tests incorrectly.
Use the already computed $sourcePrefix to construct the subsequent patterns:
- $sourcePrefix = '^' + [regex]::Escape((($SourcePath -replace '\\','/') -replace '/+$','')) + '/' - $changedSourceFiles = $changedFiles | Where-Object -FilterScript { $_ -match $sourcePrefix } + $sourcePrefix = '^' + [regex]::Escape((($SourcePath -replace '\\','/') -replace '/+$','')) + '/' + $sourceDscPattern = "$($sourcePrefix)(?:DSCResources|Classes)/" + $publicPattern = "$($sourcePrefix)Public/(.+)\.ps1$" + $privatePattern = "$($sourcePrefix)Private/(.+)\.ps1$" + $changedSourceFiles = $changedFiles | Where-Object -FilterScript { $_ -match $sourcePrefix } @@ - Write-Host "No changes detected under the source folder. DSC resource integration tests can be skipped." + Write-Host "No changes detected under the configured source path '$SourcePath'. DSC resource integration tests can be skipped." @@ - $changedDscResources = $changedFiles | Where-Object -FilterScript { $_ -match '^source/DSCResources/' -or $_ -match '^source/Classes/' } + $changedDscResources = $changedFiles | Where-Object -FilterScript { $_ -match $sourceDscPattern } @@ - $changedPublicCommands = $changedFiles | Where-Object -FilterScript { $_ -match '^source/Public/(.+)\.ps1$' } | + $changedPublicCommands = $changedFiles | Where-Object -FilterScript { $_ -match $publicPattern } | @@ - $changedPrivateFunctions = $changedFiles | Where-Object -FilterScript { $_ -match '^source/Private/(.+)\.ps1$' } | + $changedPrivateFunctions = $changedFiles | Where-Object -FilterScript { $_ -match $privatePattern } |Follow-up: The comment block for Test-ShouldRunDscResourceIntegrationTests still says “under the source/ folder”; consider updating it to “under the configured SourcePath” for accuracy.
📜 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)
.build/README.md(2 hunks).build/Test-ShouldRunDscResourceIntegrationTests.ps1(1 hunks).github/workflows/code-analysis-built-module.yml(1 hunks).github/workflows/code-analysis.yml(1 hunks).github/workflows/copilot-setup-steps.yml(1 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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 (except tables/code blocks)
- 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
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
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.md
🧠 Learnings (3)
📚 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/Classes/*.ps1 : Each DSC class-based resource must be in its own script file named exactly as the resource class and located in source/Classes
Applied to files:
.build/README.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 CHANGELOG.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)
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/Classes/*.ps1 : Derived DSC resource classes must override Get, Test, Set, GetCurrentState, Modify, AssertProperties, NormalizeProperties following the provided pattern
Applied to files:
.build/Test-ShouldRunDscResourceIntegrationTests.ps1
🪛 LanguageTool
.build/README.md
[grammar] ~21-~21: Use correct spacing
Context: ...checks if any files under the configured source path (-SourcePath, default `sou...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~22-~22: There might be a mistake here.
Context: ...cePath, default source/`) have changed - If no source changes, skips integration ...
(QB_NEW_EN_OTHER)
🪛 YAMLlint (1.37.1)
.github/workflows/code-analysis.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[warning] 16-16: too few spaces before comment: expected 2
(comments)
[warning] 17-17: too few spaces before comment: expected 2
(comments)
[warning] 18-18: missing starting space in comment
(comments)
.github/workflows/code-analysis-built-module.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[warning] 16-16: too few spaces before comment: expected 2
(comments)
[warning] 17-17: too few spaces before comment: expected 2
(comments)
[warning] 18-18: missing starting space in comment
(comments)
.github/workflows/copilot-setup-steps.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
⏰ 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 (5)
.build/README.md (1)
21-22: Tighten spacing in bullet to avoid linter nagsMinor whitespace/wording clean-up; no content change.
-1. **Early Source Check**: First checks if any files under the configured - source path (`-SourcePath`, default `source/`) have changed +1. **Early Source Check**: First checks if any files under the configured + source path (`-SourcePath`, default `source/`) have changedOptional: explicitly reference “configured SourcePath” elsewhere for consistency with the diagram.
Likely an incorrect or invalid review comment.
.github/workflows/copilot-setup-steps.yml (4)
86-91: Good: apt keyring usage and signed-by are modernized.Using /etc/apt/keyrings with signed-by is the current best practice and avoids deprecated apt-key.
103-117: Non-blocking: OMI install resilience looks good.You correctly probe availability and continue gracefully if omi is absent or fails to install, which is important given distro/repo variance.
172-187: GitVersion usage flow is sound.Idempotent update/install and verification steps are correct, and earlier full-history fetch unblocks accurate versioning.
229-266: Nice module import diagnostics.Post-build verification and helpful output increase developer ergonomics in Copilot sessions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
.github/workflows/copilot-setup-steps.yml (5)
1-1: Fix CRLF line endings to satisfy YAML lintingThe workflow has CRLF line endings. Convert to LF to resolve “wrong new line character: expected \n”.
If you want, I can add a .gitattributes rule to enforce LF globally:
* text eol=lf
6-13: Quote the top-level YAML key 'on'Unquoted “on” may be flagged by YAML 1.1 linters as a truthy token. Quote it to silence warnings.
Apply this diff:
-on: +'on': workflow_dispatch: pull_request: paths: - '.github/workflows/copilot-setup-steps.yml' push: paths: - '.github/workflows/copilot-setup-steps.yml'
127-131: Guard LD_LIBRARY_PATH for strict mode compatibilityWith set -u enabled earlier in this step, expanding an unset LD_LIBRARY_PATH will error. Provide a default.
Apply this diff:
- echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> "$GITHUB_ENV" + echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH:-}" >> "$GITHUB_ENV"
88-93: Avoid hard-coding amd64; detect arch and codename dynamicallySupport future/alternate runners by deriving arch and distro codename instead of hard-coding amd64.
Apply this diff:
- sudo install -m 0755 -d /etc/apt/keyrings - curl -fsSL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg >/dev/null - sudo chmod a+r /etc/apt/keyrings/microsoft.gpg - DISTRO_CODENAME="$(lsb_release -cs)" - echo "deb [arch=amd64 signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/microsoft-ubuntu-${DISTRO_CODENAME}-prod ${DISTRO_CODENAME} main" | sudo tee "/etc/apt/sources.list.d/microsoft-${DISTRO_CODENAME}-prod.list" >/dev/null + sudo install -m 0755 -d /etc/apt/keyrings + curl -fsSL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg >/dev/null + sudo chmod a+r /etc/apt/keyrings/microsoft.gpg + ARCH="$(dpkg --print-architecture)" + # Prefer /etc/os-release (present by default); fall back to lsb_release. + DISTRO_CODENAME="$( + . /etc/os-release 2>/dev/null && printf '%s' "${VERSION_CODENAME:-}" || true + )" + if [ -z "$DISTRO_CODENAME" ]; then DISTRO_CODENAME="$(lsb_release -cs)"; fi + echo "deb [arch=${ARCH} signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/microsoft-ubuntu-${DISTRO_CODENAME}-prod ${DISTRO_CODENAME} main" | sudo tee "/etc/apt/sources.list.d/microsoft-${DISTRO_CODENAME}-prod.list" >/dev/null
88-98: Optional hardening: verify Microsoft repo key fingerprintAdd a fingerprint check after dearmoring the key to reduce supply-chain risk from key substitution.
Insert after creating /etc/apt/keyrings/microsoft.gpg:
# Verify the Microsoft GPG key fingerprint (update if Microsoft rotates keys) EXPECTED_FPR="BC528686B50D79E339D3721CEB3E94ADBE1229CF" if ! gpg --show-keys --with-colons /etc/apt/keyrings/microsoft.gpg \ | awk -F: '$1=="fpr"{print $10}' | grep -qx "$EXPECTED_FPR"; then echo "::error::Unexpected Microsoft GPG key fingerprint"; exit 1 fi
📜 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)
.github/workflows/copilot-setup-steps.yml(1 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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 (except tables/code blocks)
- 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
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
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.md
🪛 YAMLlint (1.37.1)
.github/workflows/copilot-setup-steps.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 6-6: truthy value should be one of [false, true]
(truthy)
⏰ 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)
.github/workflows/copilot-setup-steps.yml (1)
71-79: Good: strict Bash error handling is enabledset -Eeuo pipefail improves reliability of the install step and prevents silent pipeline failures.
… source change check and update README with detailed flow diagram
johlju
force-pushed
the
fix/improver-build-script-2
branch
from
2b80926 to
738a3a2
Compare
…arkdown, JSON, and source control files
…ing OMI installation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.build/Test-ShouldRunDscResourceIntegrationTests.ps1 (4)
477-491: Critical: remaining hard-coded '^source/' breaks custom SourcePathSubsequent matchers still hard-code
^source/..., defeating the purpose of the configurable SourcePath. Use the computed$sourcePrefixfor all downstream patterns.Apply:
- $changedDscResources = $changedFiles | Where-Object -FilterScript { $_ -match '^source/DSCResources/' -or $_ -match '^source/Classes/' } + $changedDscResources = $changedFiles | Where-Object -FilterScript { + $_ -match ($sourcePrefix + 'DSCResources/') -or $_ -match ($sourcePrefix + 'Classes/') + } @@ - $changedPublicCommands = $changedFiles | Where-Object -FilterScript { $_ -match '^source/Public/(.+)\.ps1$' } | + $changedPublicCommands = $changedFiles | Where-Object -FilterScript { $_ -match ($sourcePrefix + 'Public/(.+)\.ps1$') } | ForEach-Object -Process { [System.IO.Path]::GetFileNameWithoutExtension((Split-Path -Path $_ -Leaf)) }
505-506: Finish SourcePath migration for Private functionsComplete the pattern update for private functions to honor SourcePath.
- $changedPrivateFunctions = $changedFiles | Where-Object -FilterScript { $_ -match '^source/Private/(.+)\.ps1$' } | + $changedPrivateFunctions = $changedFiles | Where-Object -FilterScript { $_ -match ($sourcePrefix + 'Private/(.+)\.ps1$') } | ForEach-Object -Process { [System.IO.Path]::GetFileNameWithoutExtension((Split-Path -Path $_ -Leaf)) }
432-435: Script-level SourcePath isn’t wired through when executed directlyThe function supports
-SourcePath, but:
- The script’s param block does not expose
-SourcePath.- The bottom-of-file invocation doesn’t forward it.
As-is, SourcePath is only configurable when importing the function, not when running the script directly. Align with docs by exposing and passing
-SourcePath.Add to the script param block (top of file):
[Parameter()] [System.String] $SourcePath = 'source'Pass it through in the direct-execution path:
$shouldRun = Test-ShouldRunDscResourceIntegrationTests ` -BaseBranch $BaseBranch ` -CurrentBranch $CurrentBranch ` -UseMergeBase:$UseMergeBase ` -SourcePath $SourcePathOptionally, update the README Parameters table to include
SourcePathand add a usage example showing a custom value.
467-470: Nit: message should reflect SourcePathMessage still says “source folder.” Consider referencing the actual
-SourcePathvalue for clarity.- Write-Host "No changes detected under the source folder. DSC resource integration tests can be skipped." + Write-Host "No changes detected under SourcePath '$SourcePath'. DSC resource integration tests can be skipped.".github/workflows/copilot-setup-steps.yml (1)
1-279: Please address remaining workflow normalization issues
Quote the
onkeys consistently across all workflows:
• .github/workflows/copilot-setup-steps.yml
• .github/workflows/code-analysis.yml
• .github/workflows/code-analysis-built-module.yml
• .github/workflows/stale.yml
Example diff for each:- on: + 'on':Convert CRLF line endings to LF in
.github/workflows/stale.yml.Use a guarded expansion when appending
LD_LIBRARY_PATHin.github/workflows/copilot-setup-steps.yml:- echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> "$GITHUB_ENV" + echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH:-}" >> "$GITHUB_ENV"
♻️ Duplicate comments (9)
.github/workflows/code-analysis-built-module.yml (2)
3-7: Quote the top-level 'on' key and tighten bracket lists to satisfy YAMLlintYAMLlint flags the unquoted
onkey and spaces inside bracketed lists. No behavior change; formatting only.Apply:
-on: +'on': push: - branches: [ main ] + branches: [main] pull_request: - branches: [ main ] + branches: [main]
16-18: Normalize inline comment spacingEnsure two spaces before inline comments and a space after the hash in commented keys.
- contents: read # for actions/checkout to fetch code + contents: read # for actions/checkout to fetch code - security-events: write # for github/codeql-action/upload-sarif to upload SARIF results + security-events: write # for github/codeql-action/upload-sarif to upload SARIF results - #actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status + # actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run statusCHANGELOG.md (1)
83-84: LGTM: Wording now matches SourcePath-aware logicThe entry correctly refers to “configured SourcePath” and removes “early optimization” phrasing. Matches the implementation and docs.
.github/workflows/code-analysis.yml (2)
3-7: Mirror YAML hygiene from the built-module workflowQuote
onand tighten bracket lists. Formatting-only change.-on: +'on': push: - branches: [ main ] + branches: [main] pull_request: - branches: [ main ] + branches: [main]
16-18: Normalize inline comment spacingTwo spaces before inline comments; add leading space after
#in commented keys.- contents: read # for actions/checkout to fetch code + contents: read # for actions/checkout to fetch code - security-events: write # for github/codeql-action/upload-sarif to upload SARIF results + security-events: write # for github/codeql-action/upload-sarif to upload SARIF results - #actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status + # actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status.github/workflows/copilot-setup-steps.yml (4)
8-15: Quote the YAML key 'on' to appease YAML 1.1 lintersUnquoted “on” is flagged as truthy by YAMLlint. Quote the key.
Apply:
-on: +'on': workflow_dispatch: pull_request: paths: - '.github/workflows/copilot-setup-steps.yml' push: paths: - '.github/workflows/copilot-setup-steps.yml'
69-76: Enable bash strict mode to surface failures earlyWithout
set -Eeuo pipefail, pipeline errors (e.g., curl|gpg|tee) may be masked and the step may continue in a bad state.Apply:
run: | - echo "Installing Linux dependencies for PowerShell DSC..." + set -Eeuo pipefail + IFS=$'\n\t' + echo "Installing Linux dependencies for PowerShell DSC..."
86-91: Make repo setup robust: verify GPG key, detect arch/codename dynamically
- Hard-coded
arch=amd64breaks on non-amd64 runners.- No key fingerprint verification (supply-chain hardening).
- Prefer /etc/os-release for codename, then fallback to lsb_release.
Apply:
- sudo install -m 0755 -d /etc/apt/keyrings - curl -fsSL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg >/dev/null - sudo chmod a+r /etc/apt/keyrings/microsoft.gpg - DISTRO_CODENAME="$(lsb_release -cs)" - echo "deb [arch=amd64 signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/microsoft-ubuntu-${DISTRO_CODENAME}-prod ${DISTRO_CODENAME} main" | sudo tee "/etc/apt/sources.list.d/microsoft-${DISTRO_CODENAME}-prod.list" >/dev/null + sudo install -m 0755 -d /etc/apt/keyrings + curl -fsSL https://packages.microsoft.com/keys/microsoft.asc \ + | gpg --dearmor \ + | sudo tee /etc/apt/keyrings/microsoft.gpg >/dev/null + sudo chmod a+r /etc/apt/keyrings/microsoft.gpg + # Verify Microsoft key fingerprint (replace with the current official fingerprint if it changes) + EXPECTED_FPR="BC528686B50D79E339D3721CEB3E94ADBE1229CF" + if ! gpg --batch --show-keys --with-colons /etc/apt/keyrings/microsoft.gpg \ + | awk -F: '$1=="fpr"{print $10}' | grep -qx "$EXPECTED_FPR"; then + echo "::error::Unexpected Microsoft GPG key fingerprint"; exit 1 + fi + ARCH="$(dpkg --print-architecture)" + DISTRO_CODENAME="$( + . /etc/os-release 2>/dev/null && printf '%s' "${VERSION_CODENAME:-}" || true + )" + if [ -z "$DISTRO_CODENAME" ]; then DISTRO_CODENAME="$(lsb_release -cs)"; fi + echo "deb [arch=${ARCH} signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/microsoft-ubuntu-${DISTRO_CODENAME}-prod ${DISTRO_CODENAME} main" \ + | sudo tee "/etc/apt/sources.list.d/microsoft-${DISTRO_CODENAME}-prod.list" >/dev/null
137-141: Guard LD_LIBRARY_PATH expansion; consider arch-aware lib path
${LD_LIBRARY_PATH}will error if unset once strict mode is enabled.- The hard-coded
/usr/lib/x86_64-linux-gnuwill be wrong on non-amd64 runners.Apply minimal guard:
- echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> "$GITHUB_ENV" + echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH:-}" >> "$GITHUB_ENV"Optional arch-aware variant:
ARCH="$(dpkg --print-architecture)" case "$ARCH" in amd64) SYS_LIB="/usr/lib/x86_64-linux-gnu" ;; arm64) SYS_LIB="/usr/lib/aarch64-linux-gnu" ;; armhf) SYS_LIB="/usr/lib/arm-linux-gnueabihf" ;; *) SYS_LIB="/usr/lib" ;; esac echo "LD_LIBRARY_PATH=/opt/omi/lib:${SYS_LIB}:${LD_LIBRARY_PATH:-}" >> "$GITHUB_ENV"
📜 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 (7)
.build/README.md(2 hunks).build/Test-ShouldRunDscResourceIntegrationTests.ps1(1 hunks).gitattributes(1 hunks).github/workflows/code-analysis-built-module.yml(1 hunks).github/workflows/code-analysis.yml(1 hunks).github/workflows/copilot-setup-steps.yml(1 hunks)CHANGELOG.md(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:
CHANGELOG.md
**/*.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)
- Surround fenced code blocks with blank lines
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters
Files:
CHANGELOG.md
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
🧠 Learnings (2)
📚 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/Classes/*.ps1 : Derived DSC resource classes must override Get, Test, Set, GetCurrentState, Modify, AssertProperties, NormalizeProperties following the provided pattern
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/Classes/*.ps1 : Each DSC class-based resource must be in its own script file named exactly as the resource class and located in source/Classes
Applied to files:
.build/README.md
🪛 YAMLlint (1.37.1)
.github/workflows/code-analysis.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[warning] 16-16: too few spaces before comment: expected 2
(comments)
[warning] 17-17: too few spaces before comment: expected 2
(comments)
[warning] 18-18: missing starting space in comment
(comments)
.github/workflows/code-analysis-built-module.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[warning] 16-16: too few spaces before comment: expected 2
(comments)
[warning] 17-17: too few spaces before comment: expected 2
(comments)
[warning] 18-18: missing starting space in comment
(comments)
.github/workflows/copilot-setup-steps.yml
[warning] 8-8: truthy value should be one of [false, true]
(truthy)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
🪛 LanguageTool
.build/README.md
[grammar] ~21-~21: Use correct spacing
Context: ...checks if any files under the configured source path (-SourcePath, default `sou...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~22-~22: There might be a mistake here.
Context: ...cePath, default source/`) have changed - If no source changes, skips integration ...
(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). (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 (4)
.github/workflows/code-analysis.yml (1)
1-1: LF line endings are already normalized—no changes needed.No tracked files were modified by
git add --renormalize ., indicating that.github/workflows/code-analysis.ymlalready uses LF endings..build/README.md (1)
46-50: Diagram label consistency“Under SourcePath?” is good; no change required.
.build/Test-ShouldRunDscResourceIntegrationTests.ps1 (1)
462-465: Good: robust, SourcePath-aware prefix constructionEscapes metacharacters, normalizes separators, and trims trailing slashes before anchoring. This is the right foundation.
.gitattributes (1)
4-16: Good targeted EOL overrides; YAML/MD/JSON now normalized to LFThe per-pattern LF rules will override the repo-wide CRLF default for the specified file types, which should resolve workflow YAML linter errors and improve cross-platform diffs for Markdown/JSON. No further action needed here.
johlju
force-pushed
the
fix/improver-build-script-2
branch
from
4714609 to
9ce94f7
Compare
… errors during OMI installation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
.github/workflows/copilot-setup-steps.yml (8)
8-15: Quote YAML key 'on' to satisfy YAMLlint (YAML 1.1 truthy pitfall).Quoting avoids the YAML 1.1 truthy ambiguity warnings from linters. No behavioral change for GitHub Actions.
-on: +'on': workflow_dispatch: pull_request: paths: - '.github/workflows/copilot-setup-steps.yml' push: paths: - '.github/workflows/copilot-setup-steps.yml'
69-76: Enable bash strict mode to fail fast and surface pipeline errors.Without
-e/-o pipefail, partial failures (e.g., curl|gpg|tee) may pass silently.run: | + set -Eeuo pipefail echo "Installing Linux dependencies for PowerShell DSC..."
82-101: Silence grep file-missing errors explicitly with -s (clearer intent).Globs may not match;
grep -savoids noisy errors. Behavior unchanged.- if ! grep -q "packages.microsoft.com" /etc/apt/sources.list /etc/apt/sources.list.d/* 2>/dev/null; then + if ! grep -qs "packages.microsoft.com" /etc/apt/sources.list /etc/apt/sources.list.d/*; then
86-91: Detect ARCH and codename dynamically; avoid hard-coding amd64 and brittle lsb_release.Future runners or non-amd64 archs will break; prefer dynamic detection and os-release fallback.
- DISTRO_CODENAME="$(lsb_release -cs)" - echo "deb [arch=amd64 signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/microsoft-ubuntu-${DISTRO_CODENAME}-prod ${DISTRO_CODENAME} main" | sudo tee "/etc/apt/sources.list.d/microsoft-${DISTRO_CODENAME}-prod.list" >/dev/null + ARCH="$(dpkg --print-architecture)" + # Prefer /etc/os-release; fall back to lsb_release if needed. + DISTRO_CODENAME="$( + . /etc/os-release 2>/dev/null && printf '%s' "${VERSION_CODENAME:-}" || true + )" + if [ -z "$DISTRO_CODENAME" ]; then DISTRO_CODENAME="$(lsb_release -cs)"; fi + echo "deb [arch=${ARCH} signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/microsoft-ubuntu-${DISTRO_CODENAME}-prod ${DISTRO_CODENAME} main" | sudo tee "/etc/apt/sources.list.d/microsoft-${DISTRO_CODENAME}-prod.list" >/dev/null
86-96: Optional defense-in-depth: verify Microsoft GPG key fingerprint before trusting.Prevents key substitution attacks with negligible overhead.
sudo install -m 0755 -d /etc/apt/keyrings curl -fsSL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg >/dev/null sudo chmod a+r /etc/apt/keyrings/microsoft.gpg + # Verify the downloaded key fingerprint (replace with the official fingerprint) + EXPECTED_FPR="BC528686B50D79E339D3721CEB3E94ADBE1229CF" + if ! gpg --show-keys --with-colons /etc/apt/keyrings/microsoft.gpg \ + | awk -F: '$1=="fpr"{print $10}' | grep -qx "$EXPECTED_FPR"; then + echo "::error::Unexpected Microsoft GPG key fingerprint"; exit 1 + fi
145-148: Guard LD_LIBRARY_PATH for unset var; consider multi-arch libdir discovery.Using
${LD_LIBRARY_PATH:-}is strict-mode-safe. Also avoid hardcodingx86_64-linux-gnu.- echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH}" >> "$GITHUB_ENV" + echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH:-}" >> "$GITHUB_ENV"Additionally (outside this hunk), consider computing the multiarch libdir:
# Near the top of this step (before exporting LD_LIBRARY_PATH) LIBDIR="$(dpkg-architecture -qDEB_HOST_MULTIARCH 2>/dev/null || echo x86_64-linux-gnu)" echo "LD_LIBRARY_PATH=/opt/omi/lib:/usr/lib/${LIBDIR}:${LD_LIBRARY_PATH:-}" >> "$GITHUB_ENV"
107-107: Trim trailing whitespace to satisfy YAMLlint.These lines still carry trailing spaces per linter. Remove trailing spaces.
Also applies to: 114-114, 121-121
1-3: Minor style: unify scalar quoting (optional).Quotes here are unnecessary; either unquote these or quote consistently across workflows.
-name: 'Copilot Setup Steps' +name: Copilot Setup Steps env: - MODULE_NAME: 'SqlServerDsc' + MODULE_NAME: SqlServerDsc
📜 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)
.github/workflows/copilot-setup-steps.yml(1 hunks)
⏰ 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)
.github/workflows/copilot-setup-steps.yml (1)
111-121: Good improvement: trap guarantees hostname restoration on failures.Robust cleanup even on early exits. Nicely handled.
johlju
left a comment
johlju
left a comment
There was a problem hiding this comment.
@johlju reviewed 1 of 3 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status:
1 participant
Pull Request (PR) description
This Pull Request (PR) fixes the following issues
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