Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 30, 2025

  • Remove testResults.xml and add to .gitignore
  • Add Force parameter to New-SqlDscAgentOperator with proper implementation
  • Use ::new() instead of New-Object for ErrorRecord creation throughout all Agent Operator files
  • Remove property change checks in Set-SqlDscAgentOperator - always set user-specified properties
  • Improve ShouldProcess description in Set-SqlDscAgentOperator to show bound parameters and values in clean format
  • Update localization strings to support new parameter format
  • Remove orphaned localization strings for removed property checks
  • Update unit tests for New-SqlDscAgentOperator parameter validation (Force parameter)
  • Update unit tests for Set-SqlDscAgentOperator parameter validation (all new parameters)
  • Fix New-Object mocking strategy for better testability
  • Fix remaining unit test issues (error handling test and mock invocation verification)
  • Add Enable-SqlDscAgentOperator and Disable-SqlDscAgentOperator commands
  • Add comprehensive integration tests for all Agent Operator commands
  • Update PowerShell style guidelines with ::new() recommendation
  • Fix exception handling in all Agent Operator commands to follow PowerShell guidelines
  • Fix unit tests to work correctly with new exception handling patterns
  • Update parameter set validation tests to include Force parameter
  • Update PowerShell guidelines to clarify exception handling with localized message
  • NEW: Rename Assert-SqlDscAgentOperatorExists to Assert-SqlDscAgentOperator
  • NEW: Remove ErrorMessage and ErrorId parameters from Assert-SqlDscAgentOperator
  • NEW: Implement generic localized message pattern (AgentOperator_NotFound)
  • NEW: Modify Assert-SqlDscAgentOperator to not return anything when operator exists
  • NEW: Create Get-AgentOperatorObject private function for operator retrieval
  • NEW: Update all Agent Operator commands to use Get-AgentOperatorObject
  • NEW: Remove orphaned localization strings for individual command error messages
  • NEW: Add comprehensive unit tests for both refactored functions
  • NEW: Ensure all files end with one blank line as per guidelines

💡 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 Reviewable

Copilot AI changed the title [WIP] Based on this file @dsccommunity/SqlServerDsc/files/source/DSCResources/DSC_SqlAgentOperator/DSC_SqlAgentOperator.psm1 I would like to create new public commands that does the creation, testing, removing and setting of the object/task it handles. The c... Add SQL Agent Operator management commands Aug 30, 2025
Copilot AI requested a review from johlju August 30, 2025 12:19
@codecov
Copy link

codecov bot commented Aug 30, 2025

Codecov Report

❌ Patch coverage is 94.24658% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 94%. Comparing base (8313dbe) to head (cf61df7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
source/Public/Set-SqlDscConfigurationOption.ps1 89% 9 Missing ⚠️
source/Public/Test-SqlDscConfigurationOption.ps1 86% 9 Missing ⚠️
source/Public/Get-SqlDscConfigurationOption.ps1 96% 1 Missing ⚠️
source/Public/New-SqlDscAgentAlert.ps1 75% 1 Missing ⚠️
source/Public/New-SqlDscDatabase.ps1 50% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##           main   #2154    +/-   ##
=====================================
  Coverage    94%     94%            
=====================================
  Files       132     145    +13     
  Lines      8740    9091   +351     
=====================================
+ Hits       8284    8617   +333     
- Misses      456     474    +18     
Flag Coverage Δ
unit 94% <94%> (+<1%) ⬆️
Files with missing lines Coverage Δ
source/Classes/020.SqlRSSetup.ps1 91% <100%> (ø)
...rivate/ConvertTo-FormattedParameterDescription.ps1 100% <100%> (ø)
source/Private/Get-AgentOperatorObject.ps1 100% <100%> (ø)
source/Private/Get-CommandParameter.ps1 100% <100%> (ø)
source/Public/Assert-SqlDscAgentOperator.ps1 100% <100%> (ø)
source/Public/Disable-SqlDscAgentOperator.ps1 100% <100%> (ø)
source/Public/Enable-SqlDscAgentOperator.ps1 100% <100%> (ø)
source/Public/Get-SqlDscAgentOperator.ps1 100% <100%> (ø)
source/Public/New-SqlDscAgentOperator.ps1 100% <100%> (ø)
source/Public/New-SqlDscRole.ps1 100% <100%> (ø)
... and 10 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot We should also add commands Enable-SqlDscAgentOperator and Disable-SqlDscAgentOperator that sets the Enabled propert on the Operator object.

Copy link
Contributor Author

Copilot AI commented Aug 30, 2025

@copilot We should also add commands Enable-SqlDscAgentOperator and Disable-SqlDscAgentOperator that sets the Enabled propert on the Operator object.

Added both Enable-SqlDscAgentOperator and Disable-SqlDscAgentOperator commands that set the Enabled property on the Operator object. Both commands follow the same pattern as the existing Enable/Disable login commands with ServerObject and OperatorObject parameter sets, ShouldProcess support, Force parameter, and Refresh option. Commit e8c3673

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • aka.ms
    • Triggering command: pwsh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Add SQL Agent Operator management commands Add comprehensive SQL Agent Operator management commands with Enable/Disable functionality Aug 30, 2025
Copilot AI requested a review from johlju August 30, 2025 17:15
@coderabbitai
Copy link

coderabbitai bot commented Sep 6, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds a comprehensive Agent Operator command suite and configuration option commands, new private helpers and SMO test stubs, extensive unit and integration tests, localization entries, CI/test grouping changes, and style-guideline and small syntactic refinements; several cmdlets adopt ShouldProcess/ConfirmImpact, Force, and terminating-error semantics.

Changes

Cohort / File(s) Summary
Guidelines & Changelog
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md, CHANGELOG.md, tests/Integration/Commands/README.md
Style guidance updates (ConfirmImpact, ConfirmPreference, error-handling wording, use ::new() for .NET types, trailing-whitespace rule) and changelog/docs updated with new cmdlets, behaviours, and test run-order/persistent operator notes.
CI / Ignore
azure-pipelines.yml, .gitignore
Adds many new integration tests to pipeline groups and ordering; adds testResults.xml to .gitignore.
Localization
source/en-US/SqlServerDsc.strings.psd1
Adds localization keys for Set/Get configuration option flows and full Agent Operator surface (prompts, success/failure, errors); some keys duplicated in file.
Public cmdlets — Agent Operator
source/Public/Get-SqlDscAgentOperator.ps1, New-SqlDscAgentOperator.ps1, Set-SqlDscAgentOperator.ps1, Remove-SqlDscAgentOperator.ps1, Enable-SqlDscAgentOperator.ps1, Disable-SqlDscAgentOperator.ps1, Test-SqlDscIsAgentOperator.ps1, Assert-SqlDscAgentOperator.ps1
New exported cmdlets implementing Operator CRUD and control (Get/New/Set/Remove/Enable/Disable/Test/Assert) with dual parameter sets (ByName/ByObject or ServerObject/OperatorObject), ShouldProcess support, ConfirmImpact, Force handling, explicit terminating-error/ErrorRecord usage, and localized messaging.
Public cmdlets — Config / Alerts / DB
source/Public/Set-SqlDscConfigurationOption.ps1, Get-SqlDscConfigurationOption.ps1, source/Public/New-SqlDscAgentAlert.ps1, source/Public/New-SqlDscDatabase.ps1, source/Public/New-SqlDscRole.ps1, source/Public/Set-SqlDscAgentAlert.ps1
Adds Set-SqlDscConfigurationOption (ShouldProcess, validation, tab-completion), changes Get-SqlDscConfigurationOption to Metadata (default) vs -Raw modes, New-SqlDscAgentAlert adds -Force and ConfirmImpact, and several commands now throw terminating errors with explicit ErrorRecords.
Private helpers
source/Private/Get-AgentOperatorObject.ps1, source/Private/Get-CommandParameter.ps1, source/Private/ConvertTo-FormattedParameterDescription.ps1
New private utilities: retrieve SMO Operator objects, compute settable command parameters (exclusions), and format bound-parameter descriptions (masking sensitive values); include CmdletBinding/OutputType metadata.
Source minor edits
source/Classes/020.SqlRSSetup.ps1, source/Public/New-SqlDscDatabase.ps1
Small syntactic change to Where-Object usage; creation flows changed to ThrowTerminatingError with explicit ErrorRecord in database/role creation error paths.
SMO test stubs
tests/Unit/Stubs/SMO.cs
Expanded SMO test stubs: Operator, OperatorCollection, WeekDays enum, Configuration/ConfigPropertyCollection, JobServer operator counters and wiring to support operator lifecycle testing.
Unit tests (private & public)
tests/Unit/Private/*.Tests.ps1, tests/Unit/Public/*.Tests.ps1, tests/Unit/DSC_SqlMaxDop.Tests.ps1
Adds/updates extensive unit tests covering new helpers and public cmdlets (Agent Operator, ConfigurationOption, AgentAlert), parameter-set assertions, behaviour, error paths, WhatIf/Force coverage, and SMO stub usage.
Integration tests
tests/Integration/Commands/*.Integration.Tests.ps1, tests/Integration/Resources/DSC_SqlRSSetup.Integration.Tests.ps1
Adds many integration suites for Get/Set/New/Remove/Enable/Disable/Test/Assert Agent Operator and ConfigurationOption cmdlets, adjusts discovery bootstrap stream redirections, and adds Uninstall-SqlDscServer test inclusion.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant Cmdlet
    participant Helper as Get-AgentOperatorObject / Get-CommandParameter / ConvertTo-FormattedParameterDescription
    participant SMO as SMO Server / JobServer / Operator

    rect rgba(200,230,255,0.20)
    User->>Cmdlet: Invoke (e.g. New-/Set-/Remove-/Enable-/Disable-)
    Note right of Cmdlet: ParameterSet selection, Validate bound params
    end

    Cmdlet->>Cmdlet: Build ShouldProcess message
    alt Force supplied
        Cmdlet->>Cmdlet: Suppress confirmation (ConfirmPreference)
    else ShouldProcess check
        Cmdlet->>User: Prompt / WhatIf/Confirm
    end

    Cmdlet->>Helper: Resolve target / settable params
    Helper->>SMO: Query JobServer.Operators or Configuration (optional Refresh)
    SMO-->>Helper: Operator / ConfigProperty or null
    Helper-->>Cmdlet: Return target info

    alt Approved & target found
        Cmdlet->>SMO: Call Create()/Alter()/Drop()/Change ConfigValue
        SMO-->>Cmdlet: Success or throw
        opt Exception
            Cmdlet->>Cmdlet: ThrowTerminatingError(ErrorRecord with localized message/id/category/target)
        end
    else Target missing
        Cmdlet->>Cmdlet: Write-Error or ThrowTerminatingError (per command)
    end

    Cmdlet-->>User: Return result (object/boolean/none) or error
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~180 minutes

Rationale: Large, heterogeneous changes touching public API surface, new cmdlets with ShouldProcess/Force/terminating-error semantics, many new private helpers, expanded SMO stubs, extensive unit and integration tests, localization additions, and CI updates — requires careful API/behavioral, tests, and localization review.

Possibly related PRs

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 39.29% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Add SQL Agent Operator management commands” accurately highlights the significant addition of operator management cmdlets in this pull request but does not mention other key enhancements such as configuration and database management commands that are also introduced.
Description Check ✅ Passed The pull request description enumerates detailed additions and modifications that directly correspond to the updated files and implemented features, demonstrating a clear and comprehensive alignment with the changes made in this branch.

Comment @coderabbitai help to get the list of available commands and usage tips.

@johlju
Copy link
Member

johlju commented Sep 6, 2025

@copilot the instructions has changed since this last was worked on, make sure the code and tests follow all instructions.

Copy link
Contributor Author

Copilot AI commented Sep 6, 2025

@copilot the instructions has changed since this last was worked on, make sure the code and tests follow all instructions.

Updated all Agent Operator source and test files to comply with current DSC community guidelines. Key changes include:

Test Setup Fixes:

  • Changed $script:dscModuleName to $script:moduleName
  • Added missing $PSDefaultParameterValues['InModuleScope:ModuleName']
  • Set $env:SqlServerDscCI = $true BEFORE module import to prevent SMO type conflicts
  • Added proper cleanup of all PSDefaultParameterValues

File Format Fixes:

  • Added missing blank lines at end of all source files
  • Ensured all test files end with blank lines

Validation:

  • All Agent Operator unit tests now pass successfully
  • Build process completes without errors
  • Files follow exact required setup pattern from instructions

Commit 1113ccf

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot add to PowerShell instructions in the File Rules section that "no line shall have trailing whitespace": https://github.com/dsccommunity/SqlServerDsc/blob/main/.github/instructions/dsc-community-style-guidelines-powershell.instructions.md#file-rules

Copy link
Contributor Author

Copilot AI commented Sep 7, 2025

@copilot add to PowerShell instructions in the File Rules section that "no line shall have trailing whitespace": https://github.com/dsccommunity/SqlServerDsc/blob/main/.github/instructions/dsc-communi...

Added "No line shall have trailing whitespace" rule to the PowerShell style guidelines File Rules section. Commit fad08ed

Copilot AI requested a review from johlju September 7, 2025 06:30
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@johlju reviewed 2 of 10 files at r9, 7 of 39 files at r17, 7 of 34 files at r18, 2 of 16 files at r19, 6 of 15 files at r20, 7 of 15 files at r21, 4 of 11 files at r22, 1 of 1 files at r23, 1 of 5 files at r24, 2 of 5 files at r25, 30 of 30 files at r26, all commit messages.
Reviewable status: all files reviewed, 60 unresolved discussions (waiting on @copilot)

@johlju johlju merged commit 48e61cb into main Sep 12, 2025
44 of 45 checks passed
@johlju johlju deleted the copilot/fix-2e4f4524-bcfc-418f-abb1-7eb2a7482232 branch September 12, 2025 05:01
@johlju johlju removed the needs review The pull request needs a code review. label Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants