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

What names should Assert-Throw parameters have? #13

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

What names should Assert-Throw parameters have? #13

nohwnd opened this issue Aug 3, 2017 · 5 comments

Comments

@nohwnd
Copy link
Owner

nohwnd commented Aug 3, 2017

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
Copy link
Owner Author

nohwnd commented Aug 3, 2017

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
Copy link
Owner Author

nohwnd commented Aug 3, 2017

Settling down to:

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

@alx9r
Copy link

alx9r commented Nov 15, 2017

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
Copy link
Owner Author

nohwnd commented Nov 15, 2017

@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
Copy link
Owner Author

nohwnd commented Nov 16, 2017

At the moment it is CustomMessage.

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

2 participants