nohwnd / Assert

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

What names should Assert-Throw parameters have? #13

Open nohwnd opened 7 years ago

nohwnd commented 7 years ago

I think it would be useful for Assert-Throw to provide parameters that would filter exceptions based on Message, Type and FullyQualifiedErrorId, but I cannot decide how to call those parameters.

1) At the moment I am using ExceptionMessage (because Message is taken for custom assertion message). Should I use ExceptionMessage for the parameter name? Should I rename Message on every assertion to AssertionMessage and use Message instead?

2) The most common case is that you are simply expecting any exception. The parameter could be called Expected but I would rather call it Type. The complication here is that you mostly get ErrorRecords not Exceptions directly and so the type of the incoming object is not what you are asserting on, instead you often need to extract the exception from the error record and so ExceptionType seems more appropriate.

3) the input to the assertion is a script block, and often those are called simply ScriptBlock, I could call it Actual but that would imply that I am expecting some value, which is not correct.

All in all there are basically those two forms I am deciding between:

# option A
{ throw "some exception" } | Assert-Throw `
    -ExceptionType ([Exception]) `
    -ExceptionMessage '*some*' `
    -FullyQualifiedErrorId '*abc*' `
    -Message 'custom message for the assertion'
# option B 
{ throw "some exception" } | Assert-Throw `
    -Type ([Exception]) `
    -Message '*some*' `
    -FullyQualifiedErrorId '*abc*' `
    -AssertionMessage 'custom message for the assertion'

Which one do you like more and why? Is there a third, better option?

nohwnd commented 7 years ago

Looking at it I think I like A better. I also start thinking that renaming -Message to -AssertionMessage on all assertions is a good idea. -Message seems too general for parameter that is not intended to be used all the time.

nohwnd commented 7 years ago

Settling down to:

{ throw "some exception" } | Assert-Throw `
    -ExceptionType ([Exception]) `
    -ExceptionMessage '*some*' `
    -FullyQualifiedErrorId '*abc*' `
    -AssertionMessage 'custom message for the assertion'
alx9r commented 6 years ago

I wonder if the the confusion of #24 could have been avoided if AssertionMessage were renamed to make it clear that it impacts the output rather than matching. ExceptionType, ExceptionMessage, and FullyQualifiedErrorId are each matcher inputs. That's implicit given the nature of the function. The odd parameter semantically is AssertionMessage. My thoughts reading the example just now went something like this: "Is AssertMessage for matching some sort of assertion-specific exception message? Hmm...maybe there's a whole part of this module that I don't even know about. No that doesn't seem like a good explanation. Ooooh...it must affect the output message."

How do you feel about OutputMessage instead of AssertionMessage?

nohwnd commented 6 years ago

@alx9r I get your confusion, AssertionMessage is still confusing. I am making it CustomMessage at the moment, I could even make it CustomAssertionMessage if you think that's better. To me it sounds better, but a bit long.

nohwnd commented 6 years ago

At the moment it is CustomMessage.