From 1a83e195535a90db1365111ba1a2546e62610639 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Tue, 31 Jul 2018 23:14:45 +0200 Subject: [PATCH] [#1849] restore checks in TransactionalJPATest that were accidentally deleted in PR 1246 P.S. it implements my comments for PR https://github.com/playframework/play1/pull/1246 * [#1849] improve design of BinaryTest P.S. it implements my comments for PR https://github.com/playframework/play1/pull/1246 * [#1849] add method assertIsStaticFile() and special case allowing to test RenderStatic in functional tests P.S. it implements my comments for PR https://github.com/playframework/play1/pull/1246 --- framework/src/play/test/FunctionalTest.java | 132 ++++++++++-------- .../app/controllers/Binary.java | 20 +-- .../just-test-cases/test/BinaryTest.java | 28 ++-- .../test/FunctionalTestTest.java | 26 ++-- .../test/TransactionalJPATest.java | 12 +- 5 files changed, 121 insertions(+), 97 deletions(-) diff --git a/framework/src/play/test/FunctionalTest.java b/framework/src/play/test/FunctionalTest.java index 86d2e07aac..9f4fc0e5bf 100644 --- a/framework/src/play/test/FunctionalTest.java +++ b/framework/src/play/test/FunctionalTest.java @@ -1,33 +1,13 @@ package play.test; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.UnsupportedEncodingException; -import java.net.MalformedURLException; -import java.nio.channels.Channels; -import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.regex.Pattern; - -import org.apache.commons.lang.ArrayUtils; -import org.junit.Before; - import com.ning.http.client.FluentCaseInsensitiveStringsMap; import com.ning.http.client.multipart.FilePart; import com.ning.http.client.multipart.MultipartBody; import com.ning.http.client.multipart.MultipartUtils; import com.ning.http.client.multipart.Part; import com.ning.http.client.multipart.StringPart; - +import org.apache.commons.lang.ArrayUtils; +import org.junit.Before; import play.Invoker; import play.Invoker.InvocationContext; import play.classloading.enhancers.ControllersEnhancer.ControllerInstrumentation; @@ -39,10 +19,29 @@ 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; +import play.mvc.results.RenderStatic; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.net.MalformedURLException; +import java.nio.channels.Channels; +import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.regex.Pattern; /** * Application tests support @@ -82,7 +81,7 @@ public static Response GET(Object url, boolean followRedirect) { Http.Header redirectedTo = response.headers.get("Location"); String location = redirectedTo.value(); if (location.contains("http")) { - java.net.URL redirectedUrl = null; + java.net.URL redirectedUrl; try { redirectedUrl = new java.net.URL(redirectedTo.value()); } catch (MalformedURLException e) { @@ -106,7 +105,7 @@ public static Response GET(Object url, boolean followRedirect) { * @return the response */ public static Response GET(Request request, Object url) { - String path = ""; + String path; String queryString = ""; String turl = url.toString(); if (turl.contains("?")) { @@ -160,7 +159,7 @@ public static Response POST(Object url, String contenttype, InputStream body) { * @return the response */ public static Response POST(Request request, Object url, String contenttype, InputStream body) { - String path = ""; + String path; String queryString = ""; String turl = url.toString(); if (turl.contains("?")) { @@ -196,7 +195,7 @@ public static Response POST(Object url, Map parameters, Map parameters) { - return POST(newRequest(), url, parameters, new HashMap()); + return POST(newRequest(), url, parameters, new HashMap<>()); } public static Response POST(Request request, Object url, Map parameters, Map files) { @@ -215,11 +214,11 @@ public static Response POST(Request request, Object url, Map par } } - MultipartBody requestEntity = null; + MultipartBody requestEntity; /* * ^1 MultipartBody::read is not working (if parts.isEmpty() == true) byte[] array = null; **/ - _ByteArrayOutputStream baos = null; + _ByteArrayOutputStream baos; try { requestEntity = MultipartUtils.newMultipartBody(parts, new FluentCaseInsensitiveStringsMap()); request.headers.putAll(ArrayUtils @@ -237,7 +236,7 @@ public static Response POST(Request request, Object url, Map par } // InputStream body = new ByteArrayInputStream(array != null ? array : // new byte[0]); // ^1 - InputStream body = new ByteArrayInputStream(baos != null ? baos.getByteArray() : new byte[0]); + InputStream body = new ByteArrayInputStream(baos.getByteArray()); return POST(request, url, MULTIPART_FORM_DATA, body); } @@ -259,7 +258,7 @@ public static Response PUT(Object url, String contenttype, String body) { * @return the response */ public static Response PUT(Request request, Object url, String contenttype, String body) { - String path = ""; + String path; String queryString = ""; String turl = url.toString(); if (turl.contains("?")) { @@ -293,7 +292,7 @@ public static Response DELETE(String url) { * @return the response */ public static Response DELETE(Request request, Object url) { - String path = ""; + String path; String queryString = ""; String turl = url.toString(); if (turl.contains("?")) { @@ -317,7 +316,7 @@ public static void makeRequest(final Request request, final Response response) { final Future invocationResult = TestEngine.functionalTestsExecutor.submit(new Invoker.Invocation() { @Override - public void execute() throws Exception { + public void execute() { renderArgs.clear(); ActionInvoker.invoke(request, response); @@ -372,7 +371,14 @@ public InvocationContext getInvocationContext() { invocationResult.get(); } catch (ExecutionException e) { - throw unwrapOriginalException(e); + RuntimeException originalException = unwrapOriginalException(e); + if (originalException instanceof RenderStatic) { + response.status = 200; + response.direct = ((RenderStatic) originalException).file; + } + else { + throw originalException; + } } catch (Exception e) { throw new RuntimeException(e); @@ -399,15 +405,16 @@ public InvocationContext getInvocationContext() { } } + /** + * Check if the original exceptions fits the usual patterns. + * If yes, return the very original runtime exception. + */ 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; + Throwable executionCause = e.getCause(); + if (executionCause instanceof JavaExecutionException || executionCause instanceof UnexpectedException) { + Throwable originalCause = executionCause.getCause(); + if (originalCause instanceof RuntimeException) { + return (RuntimeException) originalCause; } } // As a last fallback, just wrap everything up @@ -441,23 +448,20 @@ public static Response newResponse() { // Register an onWriteChunk action so that Response.writeChunk() won't throw // an unhandled exception if the controller action calls it. response.onWriteChunk( - new Action() { - @Override - public void invoke(Object chunk) { - // Mimic the behavior of PlayHandler$LazyChunkedInput.writeChunk() - if (chunk != null) { - try { - byte[] bytes; - if (chunk instanceof byte[]) { - bytes = (byte[]) chunk; - } else { - bytes = chunk.toString().getBytes(response.encoding); - } - response.out.write(bytes); - } catch (Exception exception) { - // Something is wrong with the chunk. - throw new RuntimeException(exception); + chunk -> { + // Mimic the behavior of PlayHandler$LazyChunkedInput.writeChunk() + if (chunk != null) { + try { + byte[] bytes; + if (chunk instanceof byte[]) { + bytes = (byte[]) chunk; + } else { + bytes = chunk.toString().getBytes(response.encoding); } + response.out.write(bytes); + } catch (Exception exception) { + // Something is wrong with the chunk. + throw new RuntimeException(exception); } } }); @@ -466,8 +470,7 @@ public void invoke(Object chunk) { } public static Request newRequest() { - Request request = Request.createRequest(null, "GET", "/", "", null, null, null, null, false, 80, "localhost", false, null, null); - return request; + return Request.createRequest(null, "GET", "/", "", null, null, null, null, false, 80, "localhost", false, null, null); } // Assertions @@ -503,6 +506,13 @@ public static void assertStatus(int status, Response response) { assertEquals("Response status ", (Object) status, response.status); } + public static void assertIsStaticFile(Response response, String filePath) { + assertIsOk(response); + + String file = (String) response.direct; + assertEquals(filePath, file); + } + /** * Exact equality assertion on response body * diff --git a/samples-and-tests/just-test-cases/app/controllers/Binary.java b/samples-and-tests/just-test-cases/app/controllers/Binary.java index 7d826b7f4c..98e1ed4dec 100644 --- a/samples-and-tests/just-test-cases/app/controllers/Binary.java +++ b/samples-and-tests/just-test-cases/app/controllers/Binary.java @@ -1,16 +1,16 @@ package controllers; -import play.*; +import models.UserWithAvatar; import play.data.Upload; -import play.mvc.*; +import play.mvc.Controller; +import play.mvc.Http; import play.test.Fixtures; +import java.io.ByteArrayInputStream; import java.io.File; -import java.util.*; - -import models.*; -import play.vfs.VirtualFile; -import java.io.*; +import java.io.IOException; +import java.io.InputStream; +import java.util.List; public class Binary extends Controller { @@ -114,12 +114,12 @@ public ErrorInputStream() { @Override public int read() throws IOException { - throw new IOException(); + throw new IOException("io ups"); } @Override public int read(byte[] b, int off, int len) throws IOException { - throw new IOException(); + throw new IOException("io failed"); } @Override @@ -128,7 +128,7 @@ public long skip(long n) throws IOException { } @Override - public void close() throws IOException { + public void close() { errorInputStreamClosed = true; } } diff --git a/samples-and-tests/just-test-cases/test/BinaryTest.java b/samples-and-tests/just-test-cases/test/BinaryTest.java index d4ec910663..033f29a47b 100755 --- a/samples-and-tests/just-test-cases/test/BinaryTest.java +++ b/samples-and-tests/just-test-cases/test/BinaryTest.java @@ -1,20 +1,17 @@ -import java.io.File; -import java.util.HashMap; -import java.util.Map; - +import controllers.Binary; import org.junit.Before; import org.junit.Test; - -import play.Logger; import play.Play; -import play.data.MemoryUpload; -import play.data.Upload; -import play.mvc.Http; +import play.exceptions.UnexpectedException; import play.mvc.Http.Response; -import play.mvc.results.Redirect; import play.test.Fixtures; import play.test.FunctionalTest; -import controllers.Binary; + +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ExecutionException; public class BinaryTest extends FunctionalTest { @@ -235,10 +232,17 @@ public void testGetEmptyBinary() { assertTrue(Binary.emptyInputStreamClosed); } - @Test(expected = Exception.class) + @Test public void testGetErrorBinary() { try { GET("/binary/getErrorBinary"); + fail("expected wrapped IOException"); + } + catch (RuntimeException expected) { + assertTrue(expected.getCause() instanceof ExecutionException); + assertTrue(expected.getCause().getCause() instanceof UnexpectedException); + assertTrue(expected.getCause().getCause().getCause() instanceof IOException); + assertEquals("io failed", expected.getCause().getCause().getCause().getMessage()); } finally { assertTrue(Binary.errorInputStreamClosed); diff --git a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java index 327a5b2605..a1f54f9de9 100644 --- a/samples-and-tests/just-test-cases/test/FunctionalTestTest.java +++ b/samples-and-tests/just-test-cases/test/FunctionalTestTest.java @@ -1,17 +1,19 @@ -import org.junit.*; - -import play.test.*; -import play.libs.WS; -import play.mvc.Http.*; -import play.mvc.results.*; -import models.*; +import models.User; +import org.junit.Test; +import play.mvc.Http.Cookie; +import play.mvc.Http.Request; +import play.mvc.Http.Response; +import play.mvc.results.NotFound; +import play.test.Fixtures; +import play.test.FunctionalTest; +import play.test.UnitTest; import java.util.HashMap; public class FunctionalTestTest extends FunctionalTest { @org.junit.Before - public void setUp() throws Exception { + public void setUp() { Fixtures.deleteDatabase(); Fixtures.loadModels("users.yml"); } @@ -115,17 +117,17 @@ public void usingRedirection() { @Test public void canGetRenderArgs() { - Response response = GET("/users/edit"); + Response response = GET("/users/edit"); assertIsOk(response); assertNotNull(renderArgs("u")); User u = (User) renderArgs("u"); assertEquals("Guillaume", u.name); } - @Test(expected = RenderStatic.class) + @Test public void testGettingStaticFile() { - Response response = GET("/public/session.test?req=1"); - assertIsOk(response); + Response response = GET("/public/session.test?req=1"); + assertIsStaticFile(response, "public/session.test"); } /** diff --git a/samples-and-tests/just-test-cases/test/TransactionalJPATest.java b/samples-and-tests/just-test-cases/test/TransactionalJPATest.java index 452ad760c4..a73e49f325 100644 --- a/samples-and-tests/just-test-cases/test/TransactionalJPATest.java +++ b/samples-and-tests/just-test-cases/test/TransactionalJPATest.java @@ -6,9 +6,17 @@ public class TransactionalJPATest extends FunctionalTest { - @Test(expected = TransactionRequiredException.class) + @Test public void testImport() { - Response response = GET("/Transactional/readOnlyTest"); + try { + GET("/Transactional/readOnlyTest"); + throw new AssertionError("expected TransactionRequiredException"); + } + catch (TransactionRequiredException expected) { + } + Response response = GET("/Transactional/echoHowManyPosts"); + assertIsOk(response); + assertEquals("There are 0 posts", getContent(response)); } @Test