Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pester migration - Second batch #9530

Merged
merged 29 commits into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8b93039
revert to re-fix
potatoqualitee Oct 25, 2024
c4bcd7b
update instructions to be a little shorter
potatoqualitee Oct 25, 2024
43ce0a5
follow slightly new format
potatoqualitee Oct 25, 2024
7c61359
uncomment skipper
potatoqualitee Oct 25, 2024
69b7dc3
Update tests
potatoqualitee Oct 25, 2024
fca2562
fix param checker - we will be checking for whatif and confirm
potatoqualitee Oct 25, 2024
b5be793
shorten var, fix test
potatoqualitee Oct 25, 2024
a09937f
lez see
potatoqualitee Oct 25, 2024
664904e
move param test
potatoqualitee Oct 25, 2024
a6958ca
fix failures
potatoqualitee Oct 25, 2024
eaabf51
one change
potatoqualitee Oct 25, 2024
ebe6660
ReorgParams
potatoqualitee Oct 25, 2024
8e085ac
i dunno
potatoqualitee Oct 25, 2024
a92e29f
maybe
potatoqualitee Oct 25, 2024
9ac5114
trying to get it to work locally
potatoqualitee Oct 26, 2024
e48f744
who knows, im about to skip it
potatoqualitee Oct 26, 2024
3c085e6
add defaults
potatoqualitee Oct 26, 2024
4e85fba
move things
potatoqualitee Oct 26, 2024
b3f5560
add defaults
potatoqualitee Oct 26, 2024
03d1cc6
move it back to top, expected is needed for the foreach
potatoqualitee Oct 26, 2024
2e02db1
aider cleanup
potatoqualitee Oct 26, 2024
244e129
fix test
potatoqualitee Oct 26, 2024
720dc05
it should just be skipped, it shouldnt try to bring over the notifica…
potatoqualitee Oct 26, 2024
f23f7c6
that test got all kinds of messed up
potatoqualitee Oct 26, 2024
2339c66
oops, forgot to actually run the code (do Add-DbaPfDataCollectorCounter)
potatoqualitee Oct 26, 2024
e26b8b7
fix scope
potatoqualitee Oct 26, 2024
881bfc2
fix scope
potatoqualitee Oct 26, 2024
ffddf03
fix prompt
potatoqualitee Oct 26, 2024
addd41f
turns out the scoping was right all along
potatoqualitee Oct 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
478 changes: 291 additions & 187 deletions .aider/aider.psm1

Large diffs are not rendered by default.

137 changes: 73 additions & 64 deletions .aider/prompts/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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

Expand All @@ -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
}
}
}
Expand All @@ -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
6 changes: 3 additions & 3 deletions .aider/prompts/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
9 changes: 6 additions & 3 deletions private/testing/Get-TestConfig.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -53,7 +56,7 @@ function Get-TestConfig {
}

if ($env:appveyor) {
$config['PSDefaultParameterValues'] = @{
$config['Defaults'] = [System.Management.Automation.DefaultParameterDictionary]@{
'*:WarningAction' = 'SilentlyContinue'
}
}
Expand Down
Loading
Loading