Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 18, 2025

  • Analyze the problem statement and understand requirements
  • Review existing Test-SqlDscIsDatabasePrincipal command structure and parameters
  • Examine similar integration test patterns (Test-SqlDscIsLogin, Test-SqlDscIsRole)
  • Create integration test file following SqlServerDsc guidelines
  • Implement test scenarios covering all parameter combinations
  • Test database users, roles, fixed roles, and application roles
  • Add proper error handling and edge case testing
  • Add test to azure-pipelines.yml in appropriate group
  • Update CHANGELOG.md with new integration test entry
  • Validate syntax and run unit tests to ensure no regressions
  • Fix file ending with proper blank line
  • Update README.md with correct dependency information

Successfully created comprehensive integration test for Test-SqlDscIsDatabasePrincipal command with the following coverage:

  • Database users (with ExcludeUsers parameter)
  • Database roles (with ExcludeRoles parameter)
  • Fixed roles (with ExcludeFixedRoles parameter)
  • Application roles (with ExcludeApplicationRoles parameter)
  • Error handling for missing databases
  • Pipeline parameter support
  • Case sensitivity testing
  • Multiple exclusion parameter combinations

The integration test follows established patterns in the repository and has been added to Group 2 in azure-pipelines.yml alongside similar database-related tests. Added proper dependency documentation in README.md indicating it depends on Install-SqlDscServer and Prerequisites, uses DSCSQLTEST instance, and creates test database and database principals.


💡 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

@codecov
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94%. Comparing base (11d39a0) to head (4f79f46).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
source/Public/Remove-SqlDscDatabase.ps1 66% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff          @@
##           main   #2260   +/-   ##
====================================
- Coverage    94%     94%   -1%     
====================================
  Files       149     149           
  Lines      9174    9180    +6     
====================================
+ Hits       8698    8702    +4     
- Misses      476     478    +2     
Flag Coverage Δ
unit 94% <66%> (-1%) ⬇️
Files with missing lines Coverage Δ
source/Public/Remove-SqlDscDatabase.ps1 84% <66%> (-5%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai
Copy link

coderabbitai bot commented Sep 21, 2025

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.

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.


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

@johlju
Copy link
Member

johlju commented Sep 21, 2025

@copilot try again, you could not do the work due to a temporary glitch

Copy link
Contributor Author

Copilot AI commented Sep 21, 2025

@copilot try again, you could not do the work due to a temporary glitch

I've successfully created the integration test for Test-SqlDscIsDatabasePrincipal command. The test covers all parameter combinations including database users, roles, fixed roles, and application roles with comprehensive error handling and edge cases. The test has been added to Group 2 in azure-pipelines.yml and the CHANGELOG.md has been updated. All syntax validation and unit tests pass successfully.

Commit: 46ddef7

Copilot AI changed the title [WIP] Integration test missing for Test-SqlDscIsDatabasePrincipal Add integration test for Test-SqlDscIsDatabasePrincipal command Sep 21, 2025
@johlju johlju changed the title Add integration test for Test-SqlDscIsDatabasePrincipal command Integration test missing for Test-SqlDscIsDatabasePrincipal Sep 21, 2025
Copilot AI requested a review from johlju September 21, 2025 13:37
@johlju johlju marked this pull request as ready for review September 21, 2025 13:37
@johlju johlju requested a review from a team as a code owner September 21, 2025 13:37
@johlju
Copy link
Member

johlju commented Sep 22, 2025

For some reason this integration tests break the pipeline, it makes the pipeline fail at the end even though all tests passes. This error is output after the last test runs:

ERROR: Failed to connect to server runnervmshoy1\DSCSQLTEST.
At D:\a\1\s\output\RequiredModules\Sampler\0.118.3\tasks\Invoke-Pester.pester.build.ps1:911 char:9
+         Export-Clixml -Depth 5 -Path $PesterResultObjectCliXml -Force
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
At D:\a\1\s\output\RequiredModules\Sampler\0.118.3\tasks\Invoke-Pester.pester.build.ps1:422 char:1
+ task Invoke_Pester_Tests_v5 {
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Build FAILED. 5 tasks, 1 errors, 0 warnings 00:37:09.9530284
Export-Clixml : Failed to connect to server runnervmshoy1\DSCSQLTEST.
At D:\a\1\s\output\RequiredModules\Sampler\0.118.3\tasks\Invoke-Pester.pester.build.ps1:911 char:9
+         Export-Clixml -Depth 5 -Path $PesterResultObjectCliXml -Force
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Export-Clixml], ConnectionFailureException
    + FullyQualifiedErrorId : Microsoft.SqlServer.Management.Common.ConnectionFailureException,Microsoft.PowerShell.Co 
   mmands.ExportClixmlCommand
 
##[error]PowerShell exited with code '1'.

@johlju johlju added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Sep 22, 2025
johlju and others added 3 commits September 24, 2025 21:46
…ving unnecessary service start/stop commands and using null assignment for command execution.
- Introduced a new parameter to drop active connections before removing a database.
- Updated documentation and examples to reflect the new functionality.
- Enhanced integration and unit tests to validate the new behavior.
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 9 of 9 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @copilot)

@johlju johlju merged commit 2eee794 into main Sep 25, 2025
26 of 27 checks passed
@johlju johlju deleted the copilot/fix-2231 branch September 25, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants