[#1849] Give tests more fine grained control over what exceptions they expect

framework/src/play/test/FunctionalTest.java

@@ -32,11 +32,15 @@
 import play.Invoker.InvocationContext;
 import play.classloading.enhancers.ControllersEnhancer.ControllerInstrumentation;
 import play.libs.F.Action;
+import play.exceptions.JavaExecutionException;
+import play.exceptions.UnexpectedException;
 import play.mvc.ActionInvoker;
 import play.mvc.Controller;
 import play.mvc.Http;
 import play.mvc.Http.Request;
 import play.mvc.Http.Response;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import play.mvc.Router.ActionDefinition;
 import play.mvc.Scope.RenderArgs;
 
@@ -310,7 +314,7 @@ public static Response DELETE(Request request, Object url) {
 
     public static void makeRequest(final Request request, final Response response) {
         final CountDownLatch actionCompleted = new CountDownLatch(1);
-        TestEngine.functionalTestsExecutor.submit(new Invoker.Invocation() {
+        final Future<?> invocationResult = TestEngine.functionalTestsExecutor.submit(new Invoker.Invocation() {
 
             @Override
             public void execute() throws Exception {
@@ -353,9 +357,27 @@ public InvocationContext getInvocationContext() {
 
         });
         try {
+            // We can not simply wait on the future result because of how continuations
+            // are implemented. Only when the latch is counted down the action is really
+            // completed. Therefore, wait on the latch.
             if (!actionCompleted.await(30, TimeUnit.SECONDS)) {
                 throw new TimeoutException("Request did not complete in time");
             }
+        } catch (Exception ex) {
+            throw new RuntimeException(ex);
+        }
+        try {
+            // We still call this to raise any exception that might have
+            // occurred during execution of the invocation.
+            invocationResult.get();
+        }
+        catch (ExecutionException e) {
+            throw unwrapOriginalException(e);
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+        try {
             if (savedCookies == null) {
                 savedCookies = new HashMap<>();
             }
@@ -377,6 +399,21 @@ public InvocationContext getInvocationContext() {
         }
     }
 
+    private static RuntimeException unwrapOriginalException(final ExecutionException e) {
+        // Check if the original exceptions fits the usual patterns. If yes, throw the very
+        // original runtime exception
+        final Throwable executionCause = e.getCause();
+        if (executionCause != null
+              && (executionCause instanceof JavaExecutionException || executionCause instanceof UnexpectedException)) {
+            final Throwable originalCause = executionCause.getCause();
+            if (originalCause != null && originalCause instanceof RuntimeException) {
+                throw (RuntimeException) originalCause;
+            }
+        }
+        // As a last fallback, just wrap everything up
+        return new RuntimeException(e);
+    }
+
     public static Response makeRequest(Request request) {
         Response response = newResponse();
         makeRequest(request, response);

samples-and-tests/just-test-cases/app/controllers/StatusCodes.java

@@ -0,0 +1,36 @@
+package controllers;
+
+import play.jobs.Job;
+import play.mvc.Controller;
+import play.mvc.Http.Response;
+
+public class StatusCodes extends Controller {
+
+  public static void justOkay() {
+    renderText("Okay");
+  }
+
+  public static void rendersNotFound() {
+    notFound();
+  }
+
+  public static void rendersUnauthorized() {
+    unauthorized();
+  }
+
+  public static void usesContinuation() {
+    final String text = await(new Job<String>() {
+      @Override
+      public String doJobWithResult() throws Exception {
+        return "Job completed successfully";
+      }
+    }.now());
+    Response.current().status = Integer.valueOf(201);
+    renderText(text);
+  }
+
+  public static void throwsException() throws Exception {
+    throw new UnsupportedOperationException("Whoops");
+  }
+
+}

samples-and-tests/just-test-cases/app/controllers/Transactional.java

@@ -19,9 +19,8 @@ public static void readOnlyTest() {
         tag2.name = "TransactionalTest";
         post.tags.add(tag1);
         post.tags.add(tag2);
-        post.save();
-        // since this is read only the count will not go up with successive 
-        // calls as the Post we just stored will be rolled back
+        post.save(); // since this is read only the request will fail with javax.persistence.TransactionRequiredException
+
         renderText("Wrote 1 post: total is now " + Post.count());
     }
 

samples-and-tests/just-test-cases/app/controllers/WithContinuations.java

@@ -163,14 +163,14 @@ public static void rollbackWithContinuationsThatWorks() {
 
     public static void streamedResult() {
         response.contentType = "text/html";
-        response.writeChunk("<h1>This page should load progressively in about 3 second</h1>");
+        response.writeChunk("<h1>This page should load progressively in about a few seconds</h1>");
         long s = System.currentTimeMillis();
         for(int i=0; i<100; i++) {
             await(10);
             response.writeChunk("<h2>Hello " + i + "</h2>");
         }
         long t = System.currentTimeMillis() - s;
-        response.writeChunk("Time: " + t + ", isOk->" + (t > 1000 && t < 10000));
+        response.writeChunk("Time: " + t + ", isOk->" + (t > 1000 && t < 25000));
     }
 
     public static void loopWithCallback() {
@@ -206,7 +206,7 @@ public void invoke() {
                     await(10, this);
                 } else {
                     long t = System.currentTimeMillis() - s.get();
-                    response.writeChunk("Time: " + t + ", isOk->" + (t > 1000 && t < 10000));
+                    response.writeChunk("Time: " + t + ", isOk->" + (t > 1000 && t < 25000));
                 }                
             }
         };

samples-and-tests/just-test-cases/conf/routes

@@ -83,5 +83,11 @@ GET     /databinding/changeLanguage/{lang}/?                DataBinding.changeLa
 
 GET		/useAwaitViaOtherClass				WithContinuations.ControllerWithoutContinuations.useAwaitViaOtherClass
 
+GET   /status/ok/             StatusCodes.justOkay
+GET   /status/not-found/      StatusCodes.rendersNotFound
+GET   /status/unauthorized/   StatusCodes.rendersUnauthorized
+POST  /status/job/            StatusCodes.usesContinuation
+GET   /status/failure/        StatusCodes.throwsException
+
 # module
 *         /               module:secure

samples-and-tests/just-test-cases/test/BinaryTest.java

@@ -28,7 +28,7 @@ public void setUp() {
         Response deletedResponse = GET(deleteURL);
         assertStatus(200, deletedResponse);
     }
-    
+
     @Test
     public void testUploadSomething() {
         URL imageURL = reverse(); {
@@ -235,11 +235,13 @@ public void testGetEmptyBinary() {
         assertTrue(Binary.emptyInputStreamClosed);
     }
 
-    @Test
+    @Test(expected = Exception.class)
     public void testGetErrorBinary() {
-        Response response = GET("/binary/getErrorBinary");
-        // This does not work. See Lighthouse ticket #1637.
-        // assertStatus(500, response);
-        assertTrue(Binary.errorInputStreamClosed);
+        try {
+            GET("/binary/getErrorBinary");
+        }
+        finally {
+            assertTrue(Binary.errorInputStreamClosed);
+        }
     }
 }

samples-and-tests/just-test-cases/test/FunctionalTestTest.java

@@ -3,6 +3,7 @@
 import play.test.*;
 import play.libs.WS;
 import play.mvc.Http.*;
+import play.mvc.results.*;
 import models.*;
 
 import java.util.HashMap;
@@ -121,13 +122,13 @@ public void canGetRenderArgs() {
         assertEquals("Guillaume", u.name);
     }
 
-    @Test
+    @Test(expected = RenderStatic.class)
     public void testGettingStaticFile() {
-	Response response = GET("http://localhost:9003/public/session.test?req=1");
+	Response response = GET("/public/session.test?req=1");
 	assertIsOk(response);
     }
-
-    /**
+    
+      /**
      * This is a regression test for [#2140], which is a bug in FunctionalTest that prevented it from
      * testing a controller action that uses {@link Response#writeChunk(Object)}.
      */
@@ -139,5 +140,73 @@ public void testWriteChunks() {
         assertContentType("text/plain", response);
         assertContentEquals("abcæøåæøå", response);
     }
-}
+
+
+    /**
+     * A simple call that should always work.
+     */
+    @Test
+    public void testOk() {
+        final Response response = GET("/status/ok/");
+        assertStatus(200, response);
+        assertContentEquals("Okay", response);
+    }
+
+    /**
+     * When a route is called that is not even defined, an exception is expected.
+     */
+    @Test(expected = NotFound.class)
+    public void testNoRoute() {
+        GET("/status/route-not-defined/");
+    }
+
+    /**
+     * When a defined route is called but the controller decides to render a 404,
+     * the test code is expected to pass and we can assert on the status.
+     */
+    @Test
+    public void testNotFound() {
+      final Response response = GET("/status/not-found/");
+      assertStatus(404, response);
+    }
 
+    /**
+     * When a controller throws a normal exception, an exception is expected in
+     * the test method as well.
+     */
+    @Test(expected = UnsupportedOperationException.class)
+    public void testFailure() {
+      GET("/status/failure/");
+    }
+
+    /**
+     * When a controller renders a non-standard result code (which is, actually, implemented
+     * through exception), the call is expected to pass and we can assert on the status.
+     */
+    @Test
+    public void testUnauthorized() {
+      final Response response = GET("/status/unauthorized/");
+      assertStatus(401, response);
+    }
+
+    /**
+     * Even when a controller makes use of continuations, e.g. by calling and waiting for a
+     * job, it is expected that we can assert on the status code.
+     */
+    @Test
+    public void testContinuationCustomStatus() {
+      final Response response = POST("/status/job/");
+      assertStatus(201, response);
+    }
+
+    /**
+     * Even when a controller makes use of continuations, e.g. by calling and waiting for a
+     * job, it is expected that we can assert on the content.
+     */
+    @Test
+    public void testContinuationContent() {
+      final Response response = POST("/status/job/");
+      assertContentEquals("Job completed successfully", response);
+    }
+
+}

samples-and-tests/just-test-cases/test/TransactionalJPATest.java

@@ -2,16 +2,13 @@
 
 import play.mvc.Http.Response;
 import play.test.FunctionalTest;
+import javax.persistence.TransactionRequiredException;
 
 public class TransactionalJPATest extends FunctionalTest {
 
-    @Test
+    @Test(expected = TransactionRequiredException.class)
     public void testImport() {
         Response response = GET("/Transactional/readOnlyTest");
-        assertIsOk(response);
-        response = GET("/Transactional/echoHowManyPosts");
-        assertIsOk(response);
-        assertEquals("There are 0 posts", getContent(response));
     }
 
     @Test