Wrong expected exception assertion in tests by 2xmax · Pull Request #2908 · PowerShell/PowerShell

Max Maximov (2xmax) · 2016-12-20T06:26:17Z

Hello, this is a follow-up pull request to the comment I left about expected exception assertions in the current tests.

To put it briefly, there were two problems I found:

It was forgotten to throw an exception (and calibrate the test too;)) if no exception was thrown.

try
{
    Do-SomethingThatThrowsException
    # no throw "No Exception!" or something
}
catch
{
    $_.FullyQualifiedErrorId | should be "SomeErrorId"
}

As a result, in the case of no exception, the test would not become red. This command shows you lots mistakes like this:

grep -Hn -B 4 -ir "FullyQualifiedErrorId.*should" --include *Test*.ps1 --group-separator=$'\n\n---------------------------------\n\n'

powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-17- test-PositionalBinding1 1
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-18- }
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-19- catch
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-20- {
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:21: $_.FullyQualifiedErrorId | should be "AmbiguousPositionalParameterNoName,test-PositionalBinding1"

---------------------------------

powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-66- test-allownullattributes -Parameter2 1 -Parameter3 $null -ShowMe 1
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-67- }
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-68- catch
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-69- {
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:70: $_.FullyQualifiedErrorId | should be "ParameterArgumentValidationErrorEmptyStringNotAllowed,test-allownullattributes"

---------------------------------

powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-91- test-namedwithboolishargument -Parameter2 -Parameter1
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-92- }
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-93- catch
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-94- {
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:95: $_.FullyQualifiedErrorId | should be "MissingArgument,test-namedwithboolishargument"

---------------------------------

powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-150- test-singleintparameter -Parameter1 'dookie'
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-151- }
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-152- catch
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-153- {
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:154: $_.FullyQualifiedErrorId | should be "ParameterArgumentTransformationError,test-singleintparameter"

---------------------------------

powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-268- test-nameconflicts6 -Parameter2 1
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-269- }
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-270- catch
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-271- {
powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:272: $_.FullyQualifiedErrorId | should be "ParameterNameConflictsWithAlias"

---------------------------------
powershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-246- Get-Command testgetcommand-dynamicparametersdcr -testtorun returnduplicateparameter -ErrorAction SilentlyContinue
powershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-247- }
powershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-248- catch
powershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-249- {
powershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1:250: $_.FullyQualifiedErrorId | Should Be "GetCommandMetadataError,Microsoft.PowerShell.Commands.GetCommandCommand"

---------------------------------

powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-920- try {
powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-921- Set-ExecutionPolicy -Scope $policyScope -ExecutionPolicy Restricted
powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-922- }
powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-923- catch {
powershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1:924: $_.FullyQualifiedErrorId | Should Be "CantSetGroupPolicy,Microsoft.PowerShell.Commands.SetExecutionPolicyCommand"

---------------------------------

powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-15- try {
powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-16- ConvertFrom-SecureString -secureString $null -key $null
powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-17- }
powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-18- catch {
powershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1:19: $_.FullyQualifiedErrorId | should be "ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.ConvertFromSecureStringCommand"

---------------------------------

powershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-104- try{
powershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-105- Export-Alias $fulltestpath abcd02
powershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-106- }
powershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-107- catch{
powershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1:108: $_.FullyQualifiedErrorId | Should be "FileOpenFailure,Microsoft.PowerShell.Commands.ExportAliasCommand"

---------------------------------

powershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-29- Format-Wide -InputObject $(Get-ChildItem) -Property CreationTime -View aoeu
powershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-30- }
powershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-31- catch
powershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-32- {
powershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1:33: $_.FullyQualifiedErrorId | Should be "FormatCannotSpecifyViewAndProperty,Microsoft.PowerShell.Commands.FormatWideCommand"

Unclear error reporting if no exception was thrown (Expected: {SomeException}, But was: {})

powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-156-
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-157- It "Call Set-TimeZone with invalid Id" {
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-158- $exception = $null
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-159- try { Set-TimeZone -Id "zzInvalidID" } catch { $exception = $_ }
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1:160: $exception.FullyQualifiedErrorID | Should Be "TimeZoneNotFound,Microsoft.PowerShell.Commands.SetTimeZoneCommand"

---------------------------------

powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-179-
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-180- It "Call Set-TimeZone with invalid Name" {
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-181- $exception = $null
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-182- try { Set-TimeZone -Name "zzINVALID_Name" } catch { $exception = $_ }
powershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1:183: $exception.FullyQualifiedErrorID | Should Be "TimeZoneNotFound,Microsoft.PowerShell.Commands.SetTimeZoneCommand"

P.S. We can also discuss how to prevent oversights like this in the future except for careful code-review, like using static code analysis or common fixtures

…/github.com/PowerShell/PowerShell/blob/611f7f9b77f7bb336bc4c4069135eac8520bb77a/docs/testing-guidelines/WritingPesterTests.md#L54

Microsoft Contribution License Agreements (msftclas) · 2016-12-20T06:26:21Z

Hi Max Maximov (@2xmax), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Ilya (iSazonov) · 2016-12-20T07:55:00Z

Max Maximov (@2xmax) Seems Travis-CI temporary failed. Try restart CI job (ex. resubmit commit).

Max Maximov (2xmax) · 2016-12-20T08:56:02Z

closed to rerun travis

Microsoft Contribution License Agreements (msftclas) · 2016-12-20T08:56:12Z

Hi Max Maximov (@2xmax), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Microsoft Contribution License Agreements (msftclas) · 2016-12-20T20:34:44Z

Hi Max Maximov (@2xmax), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Ilya (iSazonov) · 2016-12-22T03:14:24Z

Max Maximov (@2xmax) Just added another erroneous test. Maybe you fix this too?

Mike Richmond (mirichmo) · 2017-01-06T02:23:27Z

Max Maximov (@2xmax) Thanks for cleaning up the tests

fixed some expected exception tests according to the template https:/…

e29ddf7

…/github.com/PowerShell/PowerShell/blob/611f7f9b77f7bb336bc4c4069135eac8520bb77a/docs/testing-guidelines/WritingPesterTests.md#L54

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Dec 20, 2016

Max Maximov (2xmax) changed the title ~~fixed some expected exception tests according to the template https:/…~~ Wrong expected exception handling in tests Dec 20, 2016

Max Maximov (2xmax) changed the title ~~Wrong expected exception handling in tests~~ Wrong expected exception assertion in tests Dec 20, 2016

Max Maximov (2xmax) closed this Dec 20, 2016

Max Maximov (2xmax) reopened this Dec 20, 2016

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Dec 20, 2016

PowerShellTeam added the Review - Needed The PR is being reviewed label Dec 20, 2016

Max Maximov (2xmax) closed this Dec 20, 2016

Max Maximov (2xmax) reopened this Dec 20, 2016

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Dec 20, 2016

Mike Richmond (mirichmo) approved these changes Jan 6, 2017

View reviewed changes

Mike Richmond (mirichmo) merged commit 1ccd155 into PowerShell:master Jan 6, 2017

Navigation Menu

Repository navigation

Wrong expected exception assertion in tests - #2908

Wrong expected exception assertion in tests#2908
Mike Richmond (mirichmo) merged 1 commit into
PowerShell:masterfrom
2xmax:exp-ex-testsfixes

Max Maximov (2xmax) commented Dec 20, 2016 •

edited

Loading

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 20, 2016

Uh oh!

Ilya (iSazonov) commented Dec 20, 2016

Uh oh!

Max Maximov (2xmax) commented Dec 20, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 20, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 20, 2016

Uh oh!

Ilya (iSazonov) commented Dec 22, 2016

Uh oh!

Mike Richmond (mirichmo) commented Jan 6, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Max Maximov (2xmax) commented Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 20, 2016

Uh oh!

Ilya (iSazonov) commented Dec 20, 2016

Uh oh!

Max Maximov (2xmax) commented Dec 20, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 20, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 20, 2016

Uh oh!

Ilya (iSazonov) commented Dec 22, 2016

Uh oh!

Mike Richmond (mirichmo) commented Jan 6, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Max Maximov (2xmax) commented Dec 20, 2016 •

edited

Loading