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

Catch exceptions directly in the expected type #23

Closed
sedubois opened this issue Apr 4, 2015 · 10 comments
Closed

Catch exceptions directly in the expected type #23

sedubois opened this issue Apr 4, 2015 · 10 comments

Comments

@sedubois
Copy link

sedubois commented Apr 4, 2015

I have a custom BusinessRuleException with an extra method getCode(). With catch-exception 1.4.2, my tests look like this (didn't find a better way):

when(service).doSomething();

then(caughtException()).isInstanceOf(BusinessRuleException.class);
BusinessRuleException e = (BusinessRuleException) caughtException();
then(e.getCode()).isEqualTo(MY_ERROR_CODE);

This has 2 issues:

  • on the third line, where caughtException() is called, IntelliJ IDEA 14.1 warns that I get an exception which is not rethrown
  • not fluent

I would expect to be able to write something like (or some variation with better API and taking into account what's required to avoid unchecked casts and whatnot):

when(service).doSomething();

then(caughtException())
    .isInstanceOfAndGet(BusinessRuleException.class)
    .getCode().isEqualTo(MY_ERROR_CODE);
@mariuszs
Copy link
Member

mariuszs commented Apr 4, 2015

We are using AssertJ now, so you can easily write custom assertions for your business exception http://joel-costigliola.github.io/assertj/assertj-core-custom-assertions.html.

Please show your code for catch-exception before 1.4.2.

I take look into Intellij warning...

@mariuszs
Copy link
Member

mariuszs commented Apr 4, 2015

Ok, custom assertion dosent help here. I need to think about this :) Thanks for the report

@mariuszs
Copy link
Member

mariuszs commented Apr 4, 2015

I have only one solution to this problems ->
mariuszs@2b9ff7a

 thenCaughtException()
                .isInstanceOf(IndexOutOfBoundsException.class) //
                .hasMessage("Index: 1, Size: 0") //
                .hasNoCause();

thenCaughtException is using AssertJ api

mariuszs added a commit that referenced this issue Apr 5, 2015
* Introducing BDDCatchException#thenCaughtException
* Introducing #then(CaughtException), this is fix for #23
@mariuszs
Copy link
Member

mariuszs commented Apr 5, 2015

Fixed in #27

@mariuszs mariuszs closed this as completed Apr 5, 2015
@mariuszs mariuszs reopened this Apr 6, 2015
@mariuszs
Copy link
Member

mariuszs commented Apr 9, 2015

I leave old behaviour for Java 6 and 7, for Java 8 this is not working, my proposition for java 8:

when(service).doSomething();

then(caughtException((BusinessRuleException.class))
    .isInstanceOfAndGet(BusinessRuleException.class)
    .getCode().isEqualTo(MY_ERROR_CODE);

3f0db06

@sedubois
Copy link
Author

sedubois commented Apr 9, 2015

Is clearly better, but

then(caughtException(BusinessRuleException.class))
.isInstanceOfAndGet(BusinessRuleException.class)

still very much looks like code duplication. isInstanceOfAndGet() should be automatically done inside the then(caughtException()) or similar.

@mariuszs
Copy link
Member

mariuszs commented Apr 9, 2015

You are absolutly right, but I dont know how to do that (for Java8).

http://stackoverflow.com/questions/29499847/ambiguous-method-in-java-8-why

@evilmilo
Copy link

Can we get a non-generic version of caughtException() added?

At moment the signature is

public static <E extends Exception> E caughtException()

java 8 is now more strict, and requires the caller to define that type. Where method overloading means multiple parameters could match, you now get the ambiguity error.

for most of the assert frameworks, we just want to pass Exception in. so it would be handier if the method signature was just:

public static Exception caughtException()

Which would remove the ambiguity. To avoid breaking existing code, could either add non generic method with different name, or create a new parallel class that doesn't use the generics.

@mariuszs
Copy link
Member

mariuszs commented Oct 1, 2015

Sorry for delay, I have missed your answer,

I have added Exception caughtException() to BDDCatchException

Btw, we can use something like org.assertj.core.api.AssertionsForClassTypes in Java8 with non-generic caughtException().

@mariuszs mariuszs closed this as completed Oct 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants