From 139e2fa6628da9d102cbf0d155af43939191adc4 Mon Sep 17 00:00:00 2001 From: Hendrik Schnepel Date: Sun, 13 Jul 2014 20:28:48 +0200 Subject: [PATCH 1/7] [#1849] Made an unrelated test a little bit more robust against timing issues --- .../just-test-cases/app/controllers/WithContinuations.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/samples-and-tests/just-test-cases/app/controllers/WithContinuations.java b/samples-and-tests/just-test-cases/app/controllers/WithContinuations.java index 643fc5caf5..0e1f3eaaed 100644 --- a/samples-and-tests/just-test-cases/app/controllers/WithContinuations.java +++ b/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("

This page should load progressively in about 3 second

"); + response.writeChunk("

This page should load progressively in about a few seconds

"); long s = System.currentTimeMillis(); for(int i=0; i<100; i++) { await(10); response.writeChunk("

Hello " + i + "

"); } 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)); } } }; From 29c7e3f4558b4dbfc67f35ff17a4099cb3296bc5 Mon Sep 17 00:00:00 2001 From: Hendrik Schnepel Date: Sun, 13 Jul 2014 20:34:19 +0200 Subject: [PATCH 2/7] [#1849] Added test cases that show the unexpected behavior --- .../app/controllers/StatusCodes.java | 36 ++++++++++ samples-and-tests/just-test-cases/conf/routes | 6 ++ .../test/FunctionalTestTest.java | 69 ++++++++++++++++++- 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 samples-and-tests/just-test-cases/app/controllers/StatusCodes.java diff --git a/samples-and-tests/just-test-cases/app/controllers/StatusCodes.java b/samples-and-tests/just-test-cases/app/controllers/StatusCodes.java new file mode 100644 index 0000000000..d95a99a556 --- /dev/null +++ b/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() { + @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 Exception("Whoops"); + } + +} diff --git a/samples-and-tests/just-test-cases/conf/routes b/samples-and-tests/just-test-cases/conf/routes index 1cd69ef827..9a2550af40 100644 --- a/samples-and-tests/just-test-cases/conf/routes +++ b/samples-and-tests/just-test-cases/conf/routes @@ -80,3 +80,9 @@ 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 diff --git a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java index 3916be0379..1b71547dc9 100644 --- a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java +++ b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java @@ -126,5 +126,72 @@ public void testGettingStaticFile() { Response response = GET("http://localhost:9003/public/session.test?req=1"); assertIsOk(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 = Exception.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 = Exception.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); + } + +} From ea60d35d6ec9bf95f93b6a8f163d4350f64e6db8 Mon Sep 17 00:00:00 2001 From: Hendrik Schnepel Date: Sun, 13 Jul 2014 20:35:07 +0200 Subject: [PATCH 3/7] [#1849] Adapted BinaryTest to reflect corrected expectations --- .../just-test-cases/test/BinaryTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/samples-and-tests/just-test-cases/test/BinaryTest.java b/samples-and-tests/just-test-cases/test/BinaryTest.java index beaf21c626..d4ec910663 100755 --- a/samples-and-tests/just-test-cases/test/BinaryTest.java +++ b/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); + } } } From f5419be4ee47c0011b7a7d70c101f025bafc47e1 Mon Sep 17 00:00:00 2001 From: Hendrik Schnepel Date: Sun, 13 Jul 2014 20:37:23 +0200 Subject: [PATCH 4/7] [#1849] Fixed that exceptions from controllers are being ignored --- framework/src/play/test/FunctionalTest.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/framework/src/play/test/FunctionalTest.java b/framework/src/play/test/FunctionalTest.java index 76ad2884b3..4dade645b1 100644 --- a/framework/src/play/test/FunctionalTest.java +++ b/framework/src/play/test/FunctionalTest.java @@ -36,6 +36,8 @@ 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; @@ -307,7 +309,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 { @@ -350,9 +352,16 @@ 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"); } + // We still call this to raise any exception that might have + // occurred during execution of the invocation. + invocationResult.get(); + if (savedCookies == null) { savedCookies = new HashMap(); } From 0dfbbfb8b7909badcd76d58be4f72c2b625c0147 Mon Sep 17 00:00:00 2001 From: Hendrik Schnepel Date: Mon, 14 Jul 2014 12:39:49 +0200 Subject: [PATCH 5/7] [#1849] Give tests more fine grained control over what exceptions they expect --- framework/src/play/test/FunctionalTest.java | 30 ++++++++++++++++++- .../app/controllers/StatusCodes.java | 2 +- .../test/FunctionalTestTest.java | 5 ++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/framework/src/play/test/FunctionalTest.java b/framework/src/play/test/FunctionalTest.java index 4dade645b1..db2b196214 100644 --- a/framework/src/play/test/FunctionalTest.java +++ b/framework/src/play/test/FunctionalTest.java @@ -31,6 +31,8 @@ import play.Invoker; import play.Invoker.InvocationContext; import play.classloading.enhancers.ControllersEnhancer.ControllerInstrumentation; +import play.exceptions.JavaExecutionException; +import play.exceptions.UnexpectedException; import play.mvc.ActionInvoker; import play.mvc.Controller; import play.mvc.Http; @@ -358,10 +360,21 @@ public InvocationContext getInvocationContext() { 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(); } @@ -383,6 +396,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(final Request request) { Response response = newResponse(); makeRequest(request, response); diff --git a/samples-and-tests/just-test-cases/app/controllers/StatusCodes.java b/samples-and-tests/just-test-cases/app/controllers/StatusCodes.java index d95a99a556..2be98462a2 100644 --- a/samples-and-tests/just-test-cases/app/controllers/StatusCodes.java +++ b/samples-and-tests/just-test-cases/app/controllers/StatusCodes.java @@ -30,7 +30,7 @@ public String doJobWithResult() throws Exception { } public static void throwsException() throws Exception { - throw new Exception("Whoops"); + throw new UnsupportedOperationException("Whoops"); } } diff --git a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java index 1b71547dc9..28c53d07dc 100644 --- a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java +++ b/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; @@ -140,7 +141,7 @@ public void testOk() { /** * When a route is called that is not even defined, an exception is expected. */ - @Test(expected = Exception.class) + @Test(expected = NotFound.class) public void testNoRoute() { GET("/status/route-not-defined/"); } @@ -159,7 +160,7 @@ public void testNotFound() { * When a controller throws a normal exception, an exception is expected in * the test method as well. */ - @Test(expected = Exception.class) + @Test(expected = UnsupportedOperationException.class) public void testFailure() { GET("/status/failure/"); } From b41ebc3b75bce961e7fe6b6b7a9b6e755b38b978 Mon Sep 17 00:00:00 2001 From: Fraser Hardy Date: Wed, 18 Jul 2018 14:51:16 +0100 Subject: [PATCH 6/7] Resolved merge conflict --- samples-and-tests/just-test-cases/test/FunctionalTestTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java index 2a075ad9b4..d82d00d045 100644 --- a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java +++ b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java @@ -197,7 +197,6 @@ public void testWriteChunks() { assertContentType("text/plain", response); assertContentEquals("abcæøåæøå", response); } -} /** * Even when a controller makes use of continuations, e.g. by calling and waiting for a From 28b06cd43a89d12002cae0f0c87eca89d4485e6c Mon Sep 17 00:00:00 2001 From: Fraser Hardy Date: Thu, 19 Jul 2018 18:07:05 +0100 Subject: [PATCH 7/7] Fixed issue with Transactional Tests Fixed issue with Static test --- .../just-test-cases/app/controllers/Transactional.java | 5 ++--- .../just-test-cases/test/FunctionalTestTest.java | 4 ++-- .../just-test-cases/test/TransactionalJPATest.java | 7 ++----- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/samples-and-tests/just-test-cases/app/controllers/Transactional.java b/samples-and-tests/just-test-cases/app/controllers/Transactional.java index 724bee0463..f24da190ae 100644 --- a/samples-and-tests/just-test-cases/app/controllers/Transactional.java +++ b/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()); } diff --git a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java index d82d00d045..08d80be97f 100644 --- a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java +++ b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java @@ -122,9 +122,9 @@ 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); } diff --git a/samples-and-tests/just-test-cases/test/TransactionalJPATest.java b/samples-and-tests/just-test-cases/test/TransactionalJPATest.java index a248829236..452ad760c4 100644 --- a/samples-and-tests/just-test-cases/test/TransactionalJPATest.java +++ b/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