Using 'should throw' or 'should throwWithMessage' never succeeds unless function returns unit about fsunit HOT 15 CLOSED

Oh wow! @CaptnCodr that’s awesome! I’ll try it out.

from fsunit.

Hey @abelbraaksma,
thanks for the little repro, I'll look at it this week. 🙂

from fsunit.

you don't need ignore anymore as in your opening post:

exactly, but…

Also on functions that return obj or a generic type 'a. That is valid for throwWithMessage.

this didn’t work

tested it with throw without ignore, that test failed for xUnit. I'll put it on my list (#225).

Awesome, tx!

from fsunit.

Thank you @abelbraaksma for pointing out. 🙂

@sergey-tihon How do we proceed with this issue?
Maybe taking an additional note as comment at line 92-93 in the docs?

(fun () -> failwith "BOOM!" |> ignore) |> should throw typeof<System.Exception> // |> ignore is important here.
(fun () -> failwith "BOOM!" |> ignore) |> should (throwWithMessage "BOOM!") typeof<System.Exception> // |> ignore is important here aswell.

Any other suggestions or smth else?

from fsunit.

Isn't it possible to just catch & verify the exception? The return type shouldn't be a problem, right? It would alleviate the subtle requirement of using ignore and as such behave more like Assert.Throws (or whatever the xUnit method is).

from fsunit.

We have 2 options

1. Support functions that return obj and merge #220
2. Fail in runtime and ask to refactor testFunc to return unit

I have no idea why unit -> obj was not supported before, it was way before me 🧐

from fsunit.

Hello @abelbraaksma,
a fix (#220) has been released in v5.0.5 and is already published on Nuget. 🙂

ignore is not necessary anymore. 😉

from fsunit.

@CaptnCodr, are you sure it is part of 5.0.5? I just double-checked that I have that version, but I get the following:

    FsUnit.Xunit+MatchException : Exception of type 'FsUnit.Xunit+MatchException' was thrown.
Expected: System.Text.Json.JsonException "Expected a string, but given a Json Token Type of Number for 'amount'."
Actual:   <fun:Pipe #1 input at line 29@29>

The function looked like this, which is then called from the tests:

let shouldThrowWith msg data =
    fun () -> data |> deserialize<Money>
    |> should (throwWithMessage msg) typeof<JsonException>

It works with ignore.

Did I hit a fringe case or is it possible that the fix didn't make it to 5.0.5?

from fsunit.

are you sure it is part of 5.0.5?

Yes, definitely! I downloaded the nuget package and decompiled the library and the change is in version 5.0.5.

May you try to capsulate the actual part with paranthesis? Like this:

let shouldThrowWith msg data =
    (fun () -> data |> deserialize<Money>)
    |> should (throwWithMessage msg) typeof<JsonException>

or did you try that already?

from fsunit.

I'll try to make a minimal repro, I'll get back to this.

from fsunit.

Hey @abelbraaksma, is here any progress?
Let me know when I can help you.

from fsunit.

@CaptnCodr, thanks for pinging me. Sorry, I kinda forgot about this but now, cleaning up my messages, I'm back ;).

Using your latest version, this is a simple example of a function that does throw, but does not successfully pass (adopted from a similar function in my TaskSeq library, where I just hit this issue again recently):

// minimal repro:

[<Fact>]
let ``Null source is invalid`` () =
    let test1 () = Seq.empty |> Seq.append null
    test1 |> should throw typeof<ArgumentNullException>

This gives the follow (wrong) error:

Message: 
  FsUnit.Xunit+MatchException : Exception of type 'FsUnit.Xunit+MatchException' was thrown.
  Expected: System.ArgumentNullException
  Actual:   <fun:test1@28T>

Changing this by adding ignore as before, the test succeeds:

[<Fact>]
let ``Null source is invalid`` () =
    let test1 () = Seq.empty |> Seq.append null |> ignore
    test1 |> should throw typeof<ArgumentNullException>

What I currently use as a workaround is just a simple helper:

let inline assertThrows ty (f: unit -> 'U) = f >> ignore |> should throw ty
let inline assertNullArg f = assertThrows typeof<ArgumentNullException> f

from fsunit.

Hey @abelbraaksma,
ignore is definately needed here, the signature needs to be (unit -> unit). This is the intended way.
So, I would not implement something for your concern here.
Thank you.

from fsunit.

Well, I appreciate it, assuming this isn’t as simple to fix as I thought. Though you did write a few messages before:

ignore is not necessary anymore.

And I have seen some functions not needing it. @CaptnCodr do you know in what cases or why it is still needed sometimes?

from fsunit.

Hey @abelbraaksma,
when you throw an exception, you don't need ignore anymore as in your opening post:

(fun() -> raise(ApplicationException "BOOM!"))
|> should (throwWithMessage "BOOM!") typeof<ApplicationException>

Also on functions that return obj or a generic type 'a.
That is valid for throwWithMessage.

I tested it with throw without ignore, that test failed for xUnit. I'll put it on my list (#225).

from fsunit.