groovy
  1. groovy
  2. GROOVY-4075

shouldFailWithCause no longer works for unchecked exceptions

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7.0
    • Fix Version/s: 1.7.2, 1.8-beta-1
    • Component/s: None
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      So we upgraded from 1.6.3 to Groovy 1.7.0 and realized that one of our tests started failing... it seems that in 1.7.0 unchecked exceptions are simply rethrown (groovy.lang.Closure#throwRuntimeException(Throwable) called from #call(Object[])). Which in groovy.util.GroovyTestCase#shouldFailWithCause(Class,Closure), the exception falls to the catch Throwable branch and the cause is not attempted to be determined beyond that, which causes the test to fail.

      Example test showing a successful checked and unsuccessful unchecked exception:

      package dummy;
      
      import groovy.util.GroovyTestCase;
      
      import java.io.IOException;
      
      import org.junit.Test;
      
      /**
       * Demonstrates cases of should fail with cause.
       * @author Charlie Huggard-Lee 
       */
      class DummyTest extends GroovyTestCase {
          
          public static void failChecked() throws Exception {
              throw new Exception(new IOException());
          }
          
          public static void failUnchecked() {
              throw new RuntimeException(new IOException());
          }
          
          
          @Test
          public void testcheckedFailure() {
              //Hurray!
              shouldFailWithCause(IOException.class, {DummyTest.failChecked()})    
          }
          
          @Test
          public void testuncheckedFailure() {
              //BOO!!!
              shouldFailWithCause(IOException.class, {DummyTest.failUnchecked()})    
          }
          
      }
      

        Activity

        Hide
        Roshan Dawrani added a comment -

        Paul, the JDK 1.5 builds are failing because in the tests you have used the following 2 constructors of IOException and the JDK 1.5 version of IOException does not have both of these.

        IOException(Throwable)
        
        IOException(String, Throwable)
        

        The tests need to use some other exception that has such suitable constructors in both JDK 1.5 and 1.6.

        Show
        Roshan Dawrani added a comment - Paul, the JDK 1.5 builds are failing because in the tests you have used the following 2 constructors of IOException and the JDK 1.5 version of IOException does not have both of these. IOException(Throwable) IOException(String, Throwable) The tests need to use some other exception that has such suitable constructors in both JDK 1.5 and 1.6.
        Hide
        Roshan Dawrani added a comment -

        Here is a suggestion for IOException replacement in tests : java.util.concurrent.ExecutionException - has both the needed constructors.

        Show
        Roshan Dawrani added a comment - Here is a suggestion for IOException replacement in tests : java.util.concurrent.ExecutionException - has both the needed constructors.
        Hide
        Paul King added a comment -

        Yes, I ran the tests locally on 1.5 and spotted that too. Just fixing now ...

        Show
        Paul King added a comment - Yes, I ran the tests locally on 1.5 and spotted that too. Just fixing now ...
        Hide
        Paul King added a comment -

        I ended up going with IllegalArgumentException. The other lesson learned from the CI logs was that we weren't seeing the full error message. It made sense to unwrap GroovyRuntimeExceptions and print out all of the exception messages in the error message. It also no longer made sense to compare against the top level exception as that is now the same as shouldFail. So I updated the logic and the javadoc.

        Show
        Paul King added a comment - I ended up going with IllegalArgumentException. The other lesson learned from the CI logs was that we weren't seeing the full error message. It made sense to unwrap GroovyRuntimeExceptions and print out all of the exception messages in the error message. It also no longer made sense to compare against the top level exception as that is now the same as shouldFail. So I updated the logic and the javadoc.
        Hide
        Paul King added a comment -

        Just chatting with Jochen. Apparently there is a use case that we have encountered in the past where an exception has itself as its own cause. Sounds bad style but we should handle this case gracefully in any case. I'll add in a check for this case too to avoid the possibility of an infinite loop.

        Show
        Paul King added a comment - Just chatting with Jochen. Apparently there is a use case that we have encountered in the past where an exception has itself as its own cause. Sounds bad style but we should handle this case gracefully in any case. I'll add in a check for this case too to avoid the possibility of an infinite loop.

          People

          • Assignee:
            Roshan Dawrani
            Reporter:
            Charlie Huggard-Lee
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: