From 9765c9f4fd7dda9dcf7ab6177ead51ec4b670f43 Mon Sep 17 00:00:00 2001 From: Chrissy LeMaire Date: Fri, 25 Oct 2024 12:25:52 +0200 Subject: [PATCH] update support commands --- .aider/prompts/conventions.md | 285 ++++++++++++----------------- private/testing/Get-TestConfig.ps1 | 2 + 2 files changed, 120 insertions(+), 167 deletions(-) diff --git a/.aider/prompts/conventions.md b/.aider/prompts/conventions.md index 63261612f3..a3b9acb916 100644 --- a/.aider/prompts/conventions.md +++ b/.aider/prompts/conventions.md @@ -1,32 +1,39 @@ -# Tasks +# Pester v5 Test Standards -1. **Restructure Test Code:** - - Move all test code into appropriate blocks: `It`, `BeforeAll`, `BeforeEach`, `AfterAll`, or `AfterEach`. - - Place any file setup code into the appropriate blocks at the beginning of each test file. +## Core Requirements +```powershell +#Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0"} +param($ModuleName = "dbatools") +$global:TestConfig = Get-TestConfig +``` +These three lines must start every test file. -2. **Write Clear Test Hierarchies** -- Each `Describe` block should represent a unit of functionality -- Each `Context` block should represent a specific scenario or state -- All test code should be inside `It` blocks -- Avoid loose code in `Describe` or `Context` blocks +## Test Structure + +### Describe Blocks +- Name your Describe blocks with static command names +- Include appropriate tags (`-Tag "UnitTests"` or `-Tag "IntegrationTests"`) +- One command per Describe block -Example: ```powershell -# ❌ Avoid this -Describe "Get-DbaDatabase" { - $results = Get-DbaDatabase # Loose code! +Describe "Get-DbaDatabase" -Tag "UnitTests" { + # tests here +} +``` - Context "Basic tests" { - $databases = $results # More loose code! +### Context Blocks +- Describe specific scenarios or states +- Use clear, descriptive names that explain the test scenario +- Example: "When getting all databases", "When database is offline" - It "Returns results" { - $databases | Should -Not -BeNullOrEmpty - } - } -} +### Test Code Placement +- All setup code goes in `BeforeAll` or `BeforeEach` blocks +- All cleanup code goes in `AfterAll` or `AfterEach` blocks +- All test assertions go in `It` blocks +- No loose code in `Describe` or `Context` blocks -# ✅ Do this instead -Describe "Get-DbaDatabase" { +```powershell +Describe "Get-DbaDatabase" -Tag "IntegrationTests" { Context "When getting all databases" { BeforeAll { $results = Get-DbaDatabase @@ -39,172 +46,116 @@ Describe "Get-DbaDatabase" { } ``` -3. **Refactor Skip Conditions:** - - Move skip logic outside of `BeforeAll` blocks. - - Use global read-only variables for skip conditions where appropriate. - - Ensure that `-Skip` parameters evaluate to `$true` or `$false`, not a string. - -4. **Update `TestCases`:** - - Define `TestCases` in a way that is compatible with Pester v5's discovery phase. - -5. **Update Assertion Syntax:** - - Replace assertions like `Should Be` with `Should -Be`. - - Update other assertion operators as needed (e.g., `Should Throw` to `Should -Throw`). - -6. **Modify `InModuleScope` Usage:** - - Remove `InModuleScope` from around `Describe` and `It` blocks. - - Use the `-ModuleName` parameter on `Mock` commands where possible. - -7. **Update `Invoke-Pester` Calls:** - - Modify `Invoke-Pester` parameters to align with Pester v5's simple or advanced interface. - - **Do not use the Legacy parameter set**, as it is deprecated and may not work correctly. +## TestCases +Use the `-ForEach` parameter in `It` blocks for multiple test cases: -8. **Adjust Mocking Syntax:** - - Update any mock definitions to Pester v5 syntax. - -9. **Remove Parameter Testing Using `knownparameters`:** - - Identify any existing "Validate parameters" contexts that use `knownparameters` sections - - Remove the entire "Validate parameters" context and replace it with the Pester v5 approach using `Should -HaveParameter`, as shown in the example Pester v5 test script. - -10. **Use TestCases Whenever Possible:** - - Look for opportunities to use TestCases in the test code. - - Convert existing tests to use TestCases when applicable. - - Define TestCases using the `ForEach` parameter in the `It` block, as shown in the example below. +```powershell +It "Should calculate correctly" -ForEach @( + @{ Input = 1; Expected = 2 } + @{ Input = 2; Expected = 4 } + @{ Input = 3; Expected = 6 } +) { + $result = Get-Double -Number $Input + $result | Should -Be $Expected +} +``` -## Instructions +## Style Guidelines +- Use double quotes for strings (we're a SQL Server module) +- Array declarations should be on multiple lines: +```powershell +$array = @( + "Item1", + "Item2", + "Item3" +) +``` +- Skip conditions must evaluate to `$true` or `$false`, not strings +- Use `$global:` instead of `$script:` for test configuration variables when required for Pester v5 scoping +- Avoid script blocks in Where-Object when possible: -- **Variable Scoping:** - - Replace all `$script:` variable scopes with `$global:` where required for Pester v5 scoping. +```powershell +# Good - direct property comparison +$master = $databases | Where-Object Name -eq "master" +$systemDbs = $databases | Where-Object Name -in "master", "model", "msdb", "tempdb" -- **Comments and Debugging Notes:** - - Leave comments like `#$TestConfig.instance2 for appveyor` intact for debugging purposes. +# Required - script block for Parameters.Keys +$actualParameters = $command.Parameters.Keys | Where-Object { $PSItem -notin "WhatIf", "Confirm" } +``` -- **SQL Server-Specific Scenarios:** - - If you encounter any SQL Server-specific testing scenarios that require special handling, implement the necessary adjustments while maintaining the integrity of the tests. +## DO NOT +- DO NOT use `$MyInvocation.MyCommand.Name` to get command names +- DO NOT use the old `knownParameters` validation approach +- DO NOT include loose code outside of proper test blocks +- DO NOT remove comments like "#TestConfig.instance3" or "#$TestConfig.instance2 for appveyor" -- **Consistency with Example:** - - Follow the structure and conventions used in the example Pester v5 test script provided below. +## Examples -## Example Pester v5 Test Script +### Good Parameter Test ```powershell -#Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0"} -param($ModuleName = 'dbatools') -$global:TestConfig = Get-TestConfig - -Describe "Measure-DbaDiskSpaceRequirement" -Tag "UnitTests" { - Context "Parameter validation" { - BeforeAll { - $command = Get-Command $TestConfig.CommandName - - $expectedParameters = @( - 'Source' - 'Database' - 'SourceSqlCredential' - 'Destination' - 'DestinationDatabase' - 'DestinationSqlCredential' - 'Credential' - 'EnableException' - ) - - $actualParameters = $command.Parameters | Where-Object Keys -notin 'WhatIf', 'Confirm' - } - - It "Should have the expected number of parameters" { - $difference = Compare-Object -ReferenceObject $expectedParameters -DifferenceObject $actualParameters.Keys - $difference | Should -BeNullOrEmpty - } - - # You can still include your individual parameter checks too - It "Has parameter: <_>" -ForEach $expectedParameters { - $command | Should -HaveParameter $PSItem - } - } +Describe "Get-DbaDatabase" -Tag "UnitTests" { + Context "Parameter validation" { + BeforeAll { + $command = Get-Command Get-DbaDatabase + $expectedParameters = $TestConfig.CommonParameters + + $expectedParameters += @( + "SqlInstance", + "SqlCredential", + "Database" + ) + } + + It "Should have exactly the expected parameters" { + $actualParameters = $command.Parameters.Keys | Where-Object { $PSItem -notin "WhatIf", "Confirm" } + Compare-Object -ReferenceObject $expectedParameters -DifferenceObject $actualParameters | Should -BeNullOrEmpty + } + + It "Has parameter: <_>" -ForEach $expectedParameters { + $command | Should -HaveParameter $PSItem + } + } } +``` -Describe "Measure-DbaDiskSpaceRequirement" -Tag "IntegrationTests" { - Context "Successfully connects using newly created login" -ForEach $TestConfig.Instances { +### Good Integration Test +```powershell +Describe "Get-DbaDatabase" -Tag "IntegrationTests" { + Context "When connecting to SQL Server" -ForEach $TestConfig.Instances { BeforeAll { - $loginName = "dbatoolsci_login_$(Get-Random)" - $securePassword = ConvertTo-SecureString -String "P@ssw0rd$(Get-Random)" -AsPlainText -Force - $credential = [PSCredential]::new($loginName, $securePassword) - New-DbaLogin -SqlInstance $PSItem -Login $loginName -Password $securePassword -Confirm:$false + $databases = Get-DbaDatabase -SqlInstance $PSItem } - AfterAll { - Remove-DbaLogin -SqlInstance $PSItem -Login $loginName -Confirm:$false + It "Returns database objects with required properties" { + $databases | Should -BeOfType Microsoft.SqlServer.Management.Smo.Database + $databases[0].Name | Should -Not -BeNullOrEmpty } - It "Connects successfully" { - $instance = Connect-DbaInstance -SqlInstance $PSItem -SqlCredential $credential - $instance.Name | Should -Be $PSItem.Split('\')[0] + It "Always includes system databases" { + $systemDbs = $databases | Where-Object Name -in "master", "model", "msdb", "tempdb" + $systemDbs.Count | Should -Be 4 } } } ``` -## Example Pester v5 Test Script with TestCases - -```powershell -#Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0"} -param($ModuleName = 'dbatools') -$global:TestConfig = Get-TestConfig - -Describe "Measure-DbaSomething" { - It "Should calculate the correct result" -ForEach @( - @{ Input1 = 1; Input2 = 2; Expected = 3 } - @{ Input1 = 2; Input2 = 3; Expected = 5 } - @{ Input1 = 3; Input2 = 4; Expected = 7 } - ) { - $result = Add-Numbers -Number1 $Input1 -Number2 $Input2 - $result | Should -Be $Expected - } -} -``` - -## Additional Guidelines -* Start with `#Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0"}` like in the example above -* Second line must be `param($ModuleName = 'dbatools')` like in the example above -* -Skip:(whatever) should return true or false, not a string -* Update our Contexts to be more descriptive of the tests - -## Style and instructions - -Remember to REMOVE the knownparameters and validate parameters this way: +### Good parameter splat usage +- Use splatting for commands with 4+ parameters +- Use $splat/@splat variable names ```powershell -It "Has parameter: <_>" -ForEach $expectedParameters { - $command | Should -HaveParameter $PSItem +# Good - direct parameters for 1-3 parameters +$ag = New-DbaLogin -SqlInstance $instance -Login $loginName -Password $password + +# Good - splatting for 4+ parameters +$splat = @{ + Primary = $TestConfig.instance3 + Name = $agname + ClusterType = "None" + FailoverMode = "Manual" + Certificate = "dbatoolsci_AGCert" + Confirm = $false } -``` - -## DO NOT list parameters like this - -```powershell -$parms = @('SqlInstance','SqlCredential','Database') -``` - -## DO list parameters like this - -```powershell -$parms = @( - "SqlInstance", - "SqlCredential", - "Database" -) -``` - -## Important instructions - -DO NOT USE: -$CommandName = $MyInvocation.MyCommand.Name.Replace(".Tests.ps1", "") - -DO USE: -The static command name provided in the prompt - -DO USE: -Double quotes when possible. We are a SQL Server module and single quotes are reserved in T-SQL. - -DO NOT: -Add back constants.ps1 or the old style $knownParameters test: we removed those requirements \ No newline at end of file +$ag = New-DbaAvailabilityGroup @splat +``` \ No newline at end of file diff --git a/private/testing/Get-TestConfig.ps1 b/private/testing/Get-TestConfig.ps1 index 39f7441431..02787491a2 100644 --- a/private/testing/Get-TestConfig.ps1 +++ b/private/testing/Get-TestConfig.ps1 @@ -65,5 +65,7 @@ function Get-TestConfig { $config['CommandName'] = "Unknown" } + $config['CommonParameters'] = [System.Management.Automation.PSCmdlet]::CommonParameters + [pscustomobject]$config } \ No newline at end of file