Pester migration - Second batch (#9530)

dataplat · Oct 26, 2024 · f2d3146 · f2d3146
1 parent 81cd54f
commit f2d3146
Show file tree

Hide file tree

Showing 35 changed files with 1,866 additions and 1,049 deletions.
diff --git a/.aider/aider.psm1 b/.aider/aider.psm1
diff --git a/.aider/prompts/conventions.md b/.aider/prompts/conventions.md
@@ -3,10 +3,12 @@
 ## Core Requirements
 ```powershell
 #Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0"}
-param($ModuleName = "dbatools")
-$global:TestConfig = Get-TestConfig
+param(
+    $ModuleName = "dbatools",
+    $PSDefaultParameterValues = ($TestConfig = Get-TestConfig).Defaults
+)
 ```
-These three lines must start every test file.
+These lines must start every test file.
 
 ## Test Structure
 
@@ -45,20 +47,6 @@ Describe "Get-DbaDatabase" -Tag "IntegrationTests" {
 }
 ```
 
-## TestCases
-Use the `-ForEach` parameter in `It` blocks for multiple test cases:
-
-```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
-}
-```
-
 ## Style Guidelines
 - Use double quotes for strings (we're a SQL Server module)
 - Array declarations should be on multiple lines:
@@ -72,21 +60,65 @@ $array = @(
 - 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:
-
 ```powershell
 # Good - direct property comparison
 $master = $databases | Where-Object Name -eq "master"
 $systemDbs = $databases | Where-Object Name -in "master", "model", "msdb", "tempdb"
 
 # Required - script block for Parameters.Keys
-$actualParameters = $command.Parameters.Keys | Where-Object { $PSItem -notin "WhatIf", "Confirm" }
+$newParameters = $command.Parameters.Values.Name | Where-Object { $PSItem -notin "WhatIf", "Confirm" }
 ```
 
-## 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"
+### Parameter & Variable Naming Rules
+- Use direct parameters for 1-2 parameters
+- Use `$splat<Purpose>` for 3+ parameters (never plain `$splat`)
+
+```powershell
+# Direct parameters
+$ag = Get-DbaLogin -SqlInstance $instance -Login $loginName
+
+# Splat with purpose suffix
+$splatPrimary = @{
+    Primary = $TestConfig.instance3
+    Name = $primaryAgName
+    ClusterType = "None"
+    FailoverMode = "Manual"
+    Certificate = "dbatoolsci_AGCert"
+    Confirm = $false
+}
+$primaryAg = New-DbaAvailabilityGroup @splatPrimary
+```
+
+### Unique names across scopes
+
+- Use unique, descriptive variable names across scopes to avoid collisions
+- Play particlar attention to variable names in the BeforeAll
+
+```powershell
+Describe "Add-DbaAgReplica" -Tag "IntegrationTests" {
+    BeforeAll {
+        $primaryAgName = "dbatoolsci_agroup"
+        $splatPrimary = @{
+            Primary = $TestConfig.instance3
+            Name = $primaryAgName
+            ...
+        }
+        $ag = New-DbaAvailabilityGroup @splatPrimary
+    }
+
+    Context "When adding AG replicas" {
+        BeforeAll {
+            $replicaAgName = "dbatoolsci_add_replicagroup"
+            $splatRepAg = @{
+                Primary = $TestConfig.instance3
+                Name = $replicaAgName
+                ...
+            }
+            $replicaAg = New-DbaAvailabilityGroup @splatRepAg
+        }
+    }
+}
+```
 
 ## Examples
 
@@ -97,22 +129,23 @@ Describe "Get-DbaDatabase" -Tag "UnitTests" {
    Context "Parameter validation" {
        BeforeAll {
            $command = Get-Command Get-DbaDatabase
-           $expectedParameters  = $TestConfig.CommonParameters
-
-           $expectedParameters += @(
+           $expected = $TestConfig.CommonParameters
+           $expected += @(
                "SqlInstance",
                "SqlCredential",
-               "Database"
+               "Database",
+               "Confirm",
+               "WhatIf"
            )
        }
 
-       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 $expected {
+           $command | Should -HaveParameter $PSItem
        }
 
-       It "Has parameter: <_>" -ForEach $expectedParameters {
-           $command | Should -HaveParameter $PSItem
+       It "Should have exactly the number of expected parameters ($($expected.Count))" {
+           $hasparms = $command.Parameters.Values.Name
+           Compare-Object -ReferenceObject $expected -DifferenceObject $hasparms | Should -BeNullOrEmpty
        }
    }
 }
@@ -139,35 +172,11 @@ Describe "Get-DbaDatabase" -Tag "IntegrationTests" {
 }
 ```
 
-### Parameter & Variable Naming Rules
-1. Use direct parameters for 1-3 parameters
-2. Use `$splat<Purpose>` for 4+ parameters (never plain `$splat`)
-3. Use unique, descriptive variable names across scopes
-
-```powershell
-# Direct parameters
-$ag = New-DbaLogin -SqlInstance $instance -Login $loginName -Password $password
+## Additional instructions
 
-# Splat with purpose suffix
-$splatPrimary = @{
-    Primary = $TestConfig.instance3
-    Name = $primaryAgName    # Descriptive variable name
-    ClusterType = "None"
-    FailoverMode = "Manual"
-    Certificate = "dbatoolsci_AGCert"
-    Confirm = $false
-}
-$primaryAg = New-DbaAvailabilityGroup @splatPrimary
-
-# Unique names across scopes
-Describe "New-DbaAvailabilityGroup" {
-    BeforeAll {
-        $primaryAgName = "primaryAG"
-    }
-    Context "Adding replica" {
-        BeforeAll {
-            $replicaAgName = "replicaAG"
-        }
-    }
-}
-```
+- 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"
+- DO NOT use $_ DO use $PSItem instead
+- Parameter validation is ALWAYS tagged as a Unit Test
diff --git a/.aider/prompts/template.md b/.aider/prompts/template.md
@@ -2,10 +2,10 @@
 
 You are an AI assistant created by Anthropic to help migrate Pester tests for the **dbatools PowerShell module** from version 4 to version 5. Analyze and update the file `/workspace/tests/--CMDNAME--.Tests.ps1` according to the instructions in conventions.md.
 
-Required parameters for this command:
---PARMZ--
-
 Command name:
 --CMDNAME--
 
+Parameters for this command:
+--PARMZ--
+
 Before responding, verify that your answer adheres to the specified coding and migration guidelines.
diff --git a/private/testing/Get-TestConfig.ps1 b/private/testing/Get-TestConfig.ps1
@@ -8,15 +8,18 @@ function Get-TestConfig {
         Write-Host "Tests will use local constants file: tests\constants.local.ps1." -ForegroundColor Cyan
         . $LocalConfigPath
         # Note: Local constants are sourced but not explicitly added to $config
-    } elseif ($env:CODESPACES -and ($env:TERM_PROGRAM -eq 'vscode' -and $env:REMOTE_CONTAINERS)) {
+    } elseif ($env:CODESPACES -or ($env:TERM_PROGRAM -eq 'vscode' -and $env:REMOTE_CONTAINERS)) {
+        $null = Set-DbatoolsInsecureConnection
         $config['Instance1'] = "dbatools1"
         $config['Instance2'] = "dbatools2"
         $config['Instance3'] = "dbatools3"
         $config['Instances'] = @($config['Instance1'], $config['Instance2'])
 
         $config['SqlCred'] = [PSCredential]::new('sa', (ConvertTo-SecureString $env:SA_PASSWORD -AsPlainText -Force))
-        $config['PSDefaultParameterValues'] = @{
+        $config['Defaults'] = [System.Management.Automation.DefaultParameterDictionary]@{
             "*:SqlCredential" = $config['SqlCred']
+            "*:SourceSqlCredential" = $config['SqlCred']
+            "*:DestinationSqlCredential" = $config['SqlCred']
         }
     } elseif ($env:GITHUB_WORKSPACE) {
         $config['DbaToolsCi_Computer'] = "localhost"
@@ -53,7 +56,7 @@ function Get-TestConfig {
     }
 
     if ($env:appveyor) {
-        $config['PSDefaultParameterValues'] = @{
+        $config['Defaults'] = [System.Management.Automation.DefaultParameterDictionary]@{
             '*:WarningAction' = 'SilentlyContinue'
         }
     }