Skip to content

Commit 98bc1ed

Browse files
Copilotjohlju
andcommitted
Address code review feedback: remove ValueFromPipelineByPropertyName, use Should -Invoke assertions, and remove redundant tests
Co-authored-by: johlju <[email protected]>
1 parent e839d49 commit 98bc1ed

File tree

4 files changed

+3
-30
lines changed

4 files changed

+3
-30
lines changed

source/Public/Test-SqlDscIsLoginEnabled.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ function Test-SqlDscIsLoginEnabled
7676
[Microsoft.SqlServer.Management.Smo.Login]
7777
$LoginObject,
7878

79-
[Parameter(ParameterSetName = 'ServerObject', Mandatory = $true, ValueFromPipelineByPropertyName = $true)]
79+
[Parameter(ParameterSetName = 'ServerObject', Mandatory = $true)]
8080
[System.String]
8181
$Name,
8282

tests/Unit/Public/Disable-SqlDscLogin.Tests.ps1

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,19 +177,17 @@ Describe 'Disable-SqlDscLogin' -Tag 'Public' {
177177
} -Force
178178

179179
Mock -CommandName Get-SqlDscLogin -MockWith {
180-
$script:mockGetSqlDscLoginWasRun += 1
181180
return @($mockLoginObject)
182181
}
183182
}
184183

185184
It 'Should call the correct methods and not invoke Get-SqlDscLogin' {
186185
$script:mockMethodDisableWasRun = 0
187-
$script:mockGetSqlDscLoginWasRun = 0
188186

189187
Disable-SqlDscLogin -LoginObject $mockLoginObject -Force
190188

191189
$script:mockMethodDisableWasRun | Should -Be 1
192-
$script:mockGetSqlDscLoginWasRun | Should -Be 0
190+
Should -Invoke -CommandName Get-SqlDscLogin -Exactly -Times 0 -Scope It
193191
}
194192

195193
It 'Should not call Disable method when using WhatIf' {

tests/Unit/Public/Enable-SqlDscLogin.Tests.ps1

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,19 +177,17 @@ Describe 'Enable-SqlDscLogin' -Tag 'Public' {
177177
} -Force
178178

179179
Mock -CommandName Get-SqlDscLogin -MockWith {
180-
$script:mockGetSqlDscLoginWasRun += 1
181180
return @($mockLoginObject)
182181
}
183182
}
184183

185184
It 'Should call the correct methods and not invoke Get-SqlDscLogin' {
186185
$script:mockMethodEnableWasRun = 0
187-
$script:mockGetSqlDscLoginWasRun = 0
188186

189187
Enable-SqlDscLogin -LoginObject $mockLoginObject -Force
190188

191189
$script:mockMethodEnableWasRun | Should -Be 1
192-
$script:mockGetSqlDscLoginWasRun | Should -Be 0
190+
Should -Invoke -CommandName Get-SqlDscLogin -Exactly -Times 0 -Scope It
193191
}
194192

195193
It 'Should not call Enable method when using WhatIf' {

tests/Unit/Public/Test-SqlDscIsLoginEnabled.Tests.ps1

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,6 @@ Describe 'Test-SqlDscIsLoginEnabled' -Tag 'Public' {
9191
($parameterInfo.Attributes.ValueFromPipeline -or $parameterInfo.Attributes.ValueFromPipelineByPropertyName) | Should -BeTrue
9292
}
9393

94-
It 'Should have Name parameter accept pipeline input by property name' {
95-
$parameterInfo = (Get-Command -Name 'Test-SqlDscIsLoginEnabled').Parameters['Name']
96-
$parameterInfo.Attributes.ValueFromPipelineByPropertyName | Should -BeTrue
97-
}
98-
9994
It 'Should have LoginObject parameter accept pipeline input' {
10095
$parameterInfo = (Get-Command -Name 'Test-SqlDscIsLoginEnabled').Parameters['LoginObject']
10196
($parameterInfo.Attributes.ValueFromPipeline -or $parameterInfo.Attributes.ValueFromPipelineByPropertyName) | Should -BeTrue
@@ -169,24 +164,6 @@ Describe 'Test-SqlDscIsLoginEnabled' -Tag 'Public' {
169164
} -Exactly -Times 1 -Scope It
170165
}
171166
}
172-
173-
Context 'When the login cannot be resolved' {
174-
BeforeAll {
175-
Mock -CommandName Get-SqlDscLogin -MockWith {
176-
# Simulate Get-SqlDscLogin behavior when login is not found with ErrorAction Stop
177-
$PSCmdlet.ThrowTerminatingError((New-Object System.Management.Automation.ErrorRecord ([System.Exception]"Login 'NonExistentLogin' does not exist."), 'GSDL0001', [System.Management.Automation.ErrorCategory]::ObjectNotFound, 'NonExistentLogin'))
178-
}
179-
}
180-
181-
It 'Should handle error when login is not found' {
182-
# The behavior will depend on ErrorAction preference - with 'Stop' it should throw
183-
{ Test-SqlDscIsLoginEnabled -ServerObject $mockServerObject -Name 'NonExistentLogin' } | Should -Throw
184-
185-
Should -Invoke -CommandName Get-SqlDscLogin -ParameterFilter {
186-
$ServerObject -eq $mockServerObject -and $Name -eq 'NonExistentLogin'
187-
} -Exactly -Times 1 -Scope It
188-
}
189-
}
190167
}
191168

192169
Context 'When using parameter set LoginObject' {

0 commit comments

Comments
 (0)