{"action":"created","issue":{"url":"https://api.github.com/repos/PowerShell/PowerShell/issues/2908","repository_url":"https://api.github.com/repos/PowerShell/PowerShell","labels_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/2908/labels{/name}","comments_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/2908/comments","events_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/2908/events","html_url":"https://github.com/PowerShell/PowerShell/pull/2908","id":196595922,"number":2908,"title":"Wrong expected exception assertion in tests","user":{"login":"2xmax","id":2508022,"avatar_url":"https://avatars.githubusercontent.com/u/2508022?v=3","gravatar_id":"","url":"https://api.github.com/users/2xmax","html_url":"https://github.com/2xmax","followers_url":"https://api.github.com/users/2xmax/followers","following_url":"https://api.github.com/users/2xmax/following{/other_user}","gists_url":"https://api.github.com/users/2xmax/gists{/gist_id}","starred_url":"https://api.github.com/users/2xmax/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/2xmax/subscriptions","organizations_url":"https://api.github.com/users/2xmax/orgs","repos_url":"https://api.github.com/users/2xmax/repos","events_url":"https://api.github.com/users/2xmax/events{/privacy}","received_events_url":"https://api.github.com/users/2xmax/received_events","type":"User","site_admin":false},"labels":[{"id":427063085,"url":"https://api.github.com/repos/PowerShell/PowerShell/labels/cla-already-signed","name":"cla-already-signed","color":"009800","default":false},{"id":486892834,"url":"https://api.github.com/repos/PowerShell/PowerShell/labels/Review%20-%20Needed","name":"Review - Needed","color":"bfd4f2","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2016-12-20T06:26:17Z","updated_at":"2016-12-20T20:34:44Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/PowerShell/PowerShell/pulls/2908","html_url":"https://github.com/PowerShell/PowerShell/pull/2908","diff_url":"https://github.com/PowerShell/PowerShell/pull/2908.diff","patch_url":"https://github.com/PowerShell/PowerShell/pull/2908.patch"},"body":"Hello, this is a follow-up pull request to [the comment I left](https://github.com/PowerShell/PowerShell/pull/2862#discussion_r92483509) about expected exception assertions in the current tests. \r\n\r\nTo put it briefly, there were two problems I found:\r\n1. It was forgotten to throw an exception (and calibrate the test too;)) if no exception was thrown.\r\n```\r\ntry\r\n{\r\n    Do-SomethingThatThrowsException\r\n    # no throw \"No Exception!\" or something\r\n}\r\ncatch\r\n{\r\n    $_.FullyQualifiedErrorId | should be \"SomeErrorId\"\r\n}\r\n```\r\nAs a result, in the case of no exception, the test would not become red. This command shows you lots mistakes like this:\r\n```\r\ngrep -Hn -B 4 -ir \"FullyQualifiedErrorId.*should\" --include *Test*.ps1 --group-separator=$'\\n\\n---------------------------------\\n\\n'\r\n```\r\n```\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-17- test-PositionalBinding1 1\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-18- }\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-19- catch\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-20- {\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:21: $_.FullyQualifiedErrorId | should be \"AmbiguousPositionalParameterNoName,test-PositionalBinding1\"\r\n\r\n---------------------------------\r\n\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-66- test-allownullattributes -Parameter2 1 -Parameter3 $null -ShowMe 1\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-67- }\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-68- catch\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-69- {\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:70: $_.FullyQualifiedErrorId | should be \"ParameterArgumentValidationErrorEmptyStringNotAllowed,test-allownullattributes\"\r\n\r\n---------------------------------\r\n\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-91- test-namedwithboolishargument -Parameter2 -Parameter1\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-92- }\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-93- catch\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-94- {\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:95: $_.FullyQualifiedErrorId | should be \"MissingArgument,test-namedwithboolishargument\"\r\n\r\n---------------------------------\r\n\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-150- test-singleintparameter -Parameter1 'dookie'\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-151- }\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-152- catch\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-153- {\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:154: $_.FullyQualifiedErrorId | should be \"ParameterArgumentTransformationError,test-singleintparameter\"\r\n\r\n---------------------------------\r\n\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-268- test-nameconflicts6 -Parameter2 1\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-269- }\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-270- catch\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1-271- {\r\npowershell/engine/ParameterBinding/ParameterBinding.Tests.ps1:272: $_.FullyQualifiedErrorId | should be \"ParameterNameConflictsWithAlias\"\r\n\r\n---------------------------------\r\npowershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-246- Get-Command testgetcommand-dynamicparametersdcr -testtorun returnduplicateparameter -ErrorAction SilentlyContinue\r\npowershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-247- }\r\npowershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-248- catch\r\npowershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1-249- {\r\npowershell/Modules/Microsoft.PowerShell.Core/TestGetCommand.Tests.ps1:250: $_.FullyQualifiedErrorId | Should Be \"GetCommandMetadataError,Microsoft.PowerShell.Commands.GetCommandCommand\"\r\n\r\n---------------------------------\r\n\r\npowershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-920- try {\r\npowershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-921- Set-ExecutionPolicy -Scope $policyScope -ExecutionPolicy Restricted\r\npowershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-922- }\r\npowershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1-923- catch {\r\npowershell/Modules/Microsoft.PowerShell.Security/ExecutionPolicy.Tests.ps1:924: $_.FullyQualifiedErrorId | Should Be \"CantSetGroupPolicy,Microsoft.PowerShell.Commands.SetExecutionPolicyCommand\"\r\n\r\n---------------------------------\r\n\r\npowershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-15- try {\r\npowershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-16- ConvertFrom-SecureString -secureString $null -key $null\r\npowershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-17- }\r\npowershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1-18- catch {\r\npowershell/Modules/Microsoft.PowerShell.Security/SecureString.Tests.ps1:19: $_.FullyQualifiedErrorId | should be \"ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.ConvertFromSecureStringCommand\"\r\n\r\n---------------------------------\r\n\r\npowershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-104- try{\r\npowershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-105- Export-Alias $fulltestpath abcd02\r\npowershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-106- }\r\npowershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1-107- catch{\r\npowershell/Modules/Microsoft.PowerShell.Utility/Export-Alias.Tests.ps1:108: $_.FullyQualifiedErrorId | Should be \"FileOpenFailure,Microsoft.PowerShell.Commands.ExportAliasCommand\"\r\n\r\n---------------------------------\r\n\r\npowershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-29- Format-Wide -InputObject $(Get-ChildItem) -Property CreationTime -View aoeu\r\npowershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-30- }\r\npowershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-31- catch\r\npowershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1-32- {\r\npowershell/Modules/Microsoft.PowerShell.Utility/Format-Wide.Tests.ps1:33: $_.FullyQualifiedErrorId | Should be \"FormatCannotSpecifyViewAndProperty,Microsoft.PowerShell.Commands.FormatWideCommand\"\r\n```\r\n\r\n2. Unclear error reporting if no exception was thrown (Expected: {SomeException}, But was: {})\r\n```\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-156-\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-157- It \"Call Set-TimeZone with invalid Id\" {\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-158- $exception = $null\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-159- try { Set-TimeZone -Id \"zzInvalidID\" } catch { $exception = $_ }\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1:160: $exception.FullyQualifiedErrorID | Should Be \"TimeZoneNotFound,Microsoft.PowerShell.Commands.SetTimeZoneCommand\"\r\n\r\n---------------------------------\r\n\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-179-\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-180- It \"Call Set-TimeZone with invalid Name\" {\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-181- $exception = $null\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1-182- try { Set-TimeZone -Name \"zzINVALID_Name\" } catch { $exception = $_ }\r\npowershell/Modules/Microsoft.PowerShell.Management/TimeZone.Tests.ps1:183: $exception.FullyQualifiedErrorID | Should Be \"TimeZoneNotFound,Microsoft.PowerShell.Commands.SetTimeZoneCommand\"\r\n```\r\n\r\nP.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"},"comment":{"url":"https://api.github.com/repos/PowerShell/PowerShell/issues/comments/268351459","html_url":"https://github.com/PowerShell/PowerShell/pull/2908#issuecomment-268351459","issue_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/2908","id":268351459,"user":{"login":"msftclas","id":9287708,"avatar_url":"https://avatars.githubusercontent.com/u/9287708?v=3","gravatar_id":"","url":"https://api.github.com/users/msftclas","html_url":"https://github.com/msftclas","followers_url":"https://api.github.com/users/msftclas/followers","following_url":"https://api.github.com/users/msftclas/following{/other_user}","gists_url":"https://api.github.com/users/msftclas/gists{/gist_id}","starred_url":"https://api.github.com/users/msftclas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/msftclas/subscriptions","organizations_url":"https://api.github.com/users/msftclas/orgs","repos_url":"https://api.github.com/users/msftclas/repos","events_url":"https://api.github.com/users/msftclas/events{/privacy}","received_events_url":"https://api.github.com/users/msftclas/received_events","type":"User","site_admin":false},"created_at":"2016-12-20T20:34:44Z","updated_at":"2016-12-20T20:34:44Z","body":"Hi __@2xmax__, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!\r\n    <span>You've already signed the contribution license agreement. Thanks!</span>\r\n        <p>The agreement was validated by Microsoft and real humans are currently evaluating your PR.</p>\r\n\r\nTTYL, MSBOT;\r\n"}}
{"actor":{"display_login":"msftclas"}}