nohwnd / Assert

A set of advanced assertions for Pester to simplify how you write tests.
MIT License
101 stars 12 forks source link

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

Open nohwnd opened 7 years ago

nohwnd commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

cdhunt commented 7 years ago

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

nohwnd commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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

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.