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

Consider using Assert-Error instead of Assert-Throw #16

Open
nohwnd opened this issue Aug 3, 2017 · 7 comments
Open

Consider using Assert-Error instead of Assert-Throw #16

nohwnd opened this issue Aug 3, 2017 · 7 comments

Comments

@nohwnd
Copy link
Owner

nohwnd commented Aug 3, 2017

Now that it was mentioned on twitter, I like the idea of calling the assertion for errors/exceptions Assert-Error instead of Assert-Throw. It seems more discoverable.

On the other hand, Assert-Throw seems to suggest that the input code will throw something after we execute it. While Assert-Error sound like we already got an error object and now we are inspecting it.

So simply by the names I would expect that Assert-Throw consumes scriptblocks:

{ throw [InvalidOperationException]"This is wrong!" } | Assert-Throw 

while Assert-Error consumes errors, without actually executing any code:

$err =  $error[0]
$err | Assert-Error

But maybe the developer in me is reading too much into the names.

Opinions?

@oising
Copy link

oising commented Aug 3, 2017

Well simply put, powershell commands take the form of verb-noun, not verb-verb :) I vote for Assert-Error and have parametersets for string (e.g. error message), FullyQualifiedErrorId and exceptions. FullyQualifiedErrorId is good because it is language (as in locale) agnostic.

@oising
Copy link

oising commented Aug 3, 2017

In fact, this can be simplified as the error message (e.g. throw "foobar foo") becomes the FullyQualifiedErrorId. So, if you use two sets - one for exception, the other for FullyQualifiedErrorId, you cover all three.

@brianbunke
Copy link

+1 for Assert-Error instead of Assert-Throw, mainly because of the "discoverability" argument.

@cdhunt
Copy link

cdhunt commented Aug 3, 2017

I personally use Write-Error over throw most of the time so that's what's intuitive to me.

@nohwnd
Copy link
Owner Author

nohwnd commented Aug 3, 2017

Gotcha, I like it as well, but I also want to be consistent with the other assertion names.

Thinking about it the assertions would be much better fit for custom operators than functions. In the end that's what they mostly are - operator adapter that throws for $false. Powershell unfortunately does not allow that. Just imagine it: 👍

$message ?eq 'olleh'

( It would also suck when the actual side (left-hand-side) would determine the type to use for the comparison, instead of the expected (right-hand-side). As we already witnessed in one of the early versions of Pester 4. :)) )

I'd like to move to proper nouns to follow the PowerShell naming conventions as much as possible, and I can imagine calling Assert-Equal Assert-Equality, but what about Assert-LessThanOrEqual and other assertions?

@DarkLite1
Copy link

IMHO Assert-Throw and Assert-Error are two different things. Like you said in your start post, throw is used to catch terminating errors and Write-Error is used for non terminating errors. So it would be better not to mix them.

@nohwnd
Copy link
Owner Author

nohwnd commented Nov 16, 2017

Revisiting this after some time and I am still not sure about the name.
There are three canditates:

  • Assert-Throw
    • is familiar from Pester
    • we expect the scriptblock to throw
    • but not every error is thrown
  • Assert-Error
    • likely most familiar to powershellers
    • not every error is error, some are exceptions
  • Assert-Exception
    • seems too specific

At the moment I am inclined to use Assert-Error, and avoid re-using the other candidates for anything else to avoid making the api ambiguous.

But what about non-terminating errors? Does anyone need to capture those and assert on them? Connected to that there would probably be call for asserting the other types of output, and so the assertions could becalled Assert-ErrorOutput, Assert-VerboseOutput ?


Maybe aliasing it would be permissible, would script analyzer then complain that you should use Assert-Error? That would be great, because the user would get many to choose from, and be steered to use the one true choice (and possibly confusing as hell, so let's wait wait this.)

In the future we might need assertions that will look at the actual errors and exceptions and those I would probably call Assert-ErrorRecord and Assert-ExceptionObject.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants