Include actual exception when shouldThrowTheException fails

[Adeynack]

Actual situation

When a should throw the exception fails, the message states:

Expected foo.bar.TheExpectedException to be thrown expected:<class [foo.bar.TheExpected]Exception> but was:<class [kotlin.KotlinNullPointer]Exception>

Then I can see the stack trace of that ComparisonFailure (at least, in the IDE).

Problem

That indicates only the point of failure in the test (the assertion itself), but does not give me any information about the exception that did occur in reality (that was not expected). I need then to trace and/or modify temporarily my test code so it just fails (remove the assertion) and then I can see what happened.

Proposition

Include the actual exception as a cause of the thrown ComparisonFailure. That way, we can look at the stack trace to know exactly what happened instead of our expected exception.

In Exceptions.kt:
Before:

        else throw ComparisonFailure("Expected ${expectedException.javaObjectType} to be thrown", "${expectedException.javaObjectType}", "${e.javaClass}")

After:

        else throw ComparisonFailure("Expected ${expectedException.javaObjectType} to be thrown", "${expectedException.javaObjectType}", "${e.javaClass}", e)

That logic should/could be applied to every assertions on exceptions, including the withMessage and withCause.

After effect

From the IDE, we can then click on the stack trace of the exception that caused the assertion to fail to be brought to the line that caused it in the code AND know exactly what happened from its message.

[Adeynack]

It is something I could create a pull-request for, if you:

1. feel that it is in interesting feature (it would definitely make MY life as a user of kluent easier!)
2. it is something you would not work on anytime soon

[Adeynack]

ComparisonException would need a constructor that accepts a cause.

[MarkusAmshove]

This looks like a really nice addition.

ComparisonFailure is a type from junit, so we wouldn't be able to add a constructor.
But maybe subtyping it works.

I initially chose to use the junit exceptions because the IDEs I use (IntelliJ and Eclipse) recognize them and can show a specific window (like a diff window for assertEquals failures).

However, the diff window wouldn't be very useful here, as it just shows the two types.

I`d appreciate a pull request, because I don't have a lot time in June at the moment, but would gladly review, merge and release it :-)
Otherwise I can look into it in July

[Adeynack]

Thanks for the answer, @MarkusAmshove . I will get to it in the next days probably. Already copied a bit of your code in my project to move forward with it, so it is basically "already started" ;)

[Adeynack]

@MarkusAmshove : How should we proceed for the contribution? I cannot find a CONTRIBUTING file to guide me.

1. I am added to the project and create a pull-request directly on it
2. I fork and then create a pull request from my fork to yours

Please advise!

[MarkusAmshove]

You're right, sorry. I'll put it on my TODO list.

In the past the contributers all forked this repository and opened a pull request from within the fork :-)

[Adeynack]

I will do that then! Thanks for the response.

[Adeynack]

@MarkusAmshove : You can assign this issue to me (I cannot perform that change), so no one starts work in parallel. My fork is not at [email protected]:Adeynack/Kluent.git. I will create a PR when ready.

[Adeynack]

@MarkusAmshove : I just realised what you meant by IDE support earlier. I was wondering what you were talking about with the diff view and stuff like that. I just realised that I am using Kluent enclusively with Spek, and when one do so, those nice IDE features disappear. In my current setup (Spek/Kluent), I can only rely on the exception's message to understand what went wrong in my test. Not super nice. There is a LOT of work to do in the Spek plugin for IntelliJ.

[MarkusAmshove]

Yeah, I've wanted to port the unittests from Kluent to plain old JUnit, but we now have around 900 tests, so I just keep them that way :-)

We're using an old Spek version because it doesn't rely on the IntelliJ plugin (thinking about Eclipse users), maybe it would work as expected on the most recent version of Spek.
I'll look into updating it in the future.

[Adeynack]

Did you mean "from Spek to JUnit"?

[MarkusAmshove]

Yeah, also because Gradle doesn't work well with the Spek version we use (it counts over a thousand tests, which aren't there). But I think this will be fixed when I upgrade Spek

[lucasqueiroz]

Any updates on this?

[Adeynack]

I will de-a assign myself from it. I haven't used Kotlin in almost 2 years. My focus is on completely different things now. My apologies for never completing this change.