Jiabao-Sun commented on code in PR #23242:
URL: https://github.com/apache/flink/pull/23242#discussion_r1299803604
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestExternalHandlersITCase.java:
##########
@@ -128,29 +124,27 @@ void teardown() throws Exception {
void testHandlersMustBeLoaded() {
final List<InboundChannelHandlerFactory>
inboundChannelHandlerFactories =
serverEndpoint.getInboundChannelHandlerFactories();
- assertEquals(inboundChannelHandlerFactories.size(), 2);
- assertTrue(
- inboundChannelHandlerFactories.get(0) instanceof
Prio1InboundChannelHandlerFactory);
- assertTrue(
- inboundChannelHandlerFactories.get(1) instanceof
Prio0InboundChannelHandlerFactory);
+ assertThat(inboundChannelHandlerFactories.size()).isEqualTo(2);
+ assertThat(inboundChannelHandlerFactories.get(0))
+ .isInstanceOf(Prio1InboundChannelHandlerFactory.class);
+ assertThat(inboundChannelHandlerFactories.get(1))
+ .isInstanceOf(Prio0InboundChannelHandlerFactory.class);
final List<OutboundChannelHandlerFactory>
outboundChannelHandlerFactories =
restClient.getOutboundChannelHandlerFactories();
- assertEquals(outboundChannelHandlerFactories.size(), 2);
- assertTrue(
- outboundChannelHandlerFactories.get(0)
- instanceof Prio1OutboundChannelHandlerFactory);
- assertTrue(
- outboundChannelHandlerFactories.get(1)
- instanceof Prio0OutboundChannelHandlerFactory);
+ assertThat(outboundChannelHandlerFactories.size()).isEqualTo(2);
+ assertThat(outboundChannelHandlerFactories.get(0))
+ .isInstanceOf(Prio1OutboundChannelHandlerFactory.class);
+ assertThat(outboundChannelHandlerFactories.get(1))
+ .isInstanceOf(Prio0OutboundChannelHandlerFactory.class);
try {
final CompletableFuture<TestResponse> response =
sendRequestToTestHandler(new TestRequest());
response.get();
fail("Request must fail with 2 times redirected URL");
} catch (Exception e) {
- assertTrue(e.getMessage().contains(REDIRECT2_URL));
+ assertThat(e.getMessage()).contains(REDIRECT2_URL);
}
Review Comment:
I think we can use `assertThatFuture` as well.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointConfigurationTest.java:
##########
@@ -22,42 +22,38 @@
import org.apache.flink.configuration.RestOptions;
import org.apache.flink.configuration.WebOptions;
import org.apache.flink.util.ConfigurationException;
-import org.apache.flink.util.TestLogger;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
-import static org.hamcrest.CoreMatchers.containsString;
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for the {@link RestServerEndpointConfiguration}. */
-public class RestServerEndpointConfigurationTest extends TestLogger {
+class RestServerEndpointConfigurationTest {
private static final String ADDRESS = "123.123.123.123";
private static final String BIND_ADDRESS = "023.023.023.023";
private static final String BIND_PORT = "7282";
private static final int CONTENT_LENGTH = 1234;
- @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();
-
@Test
- public void testBasicMapping() throws ConfigurationException {
+ void testBasicMapping(@TempDir File file) throws ConfigurationException {
Review Comment:
```suggestion
void testBasicMapping(@TempDir File temporaryFolder) throws
ConfigurationException {
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointITCase.java:
##########
@@ -109,34 +108,26 @@
import static java.util.Objects.requireNonNull;
import static org.apache.flink.core.testutils.CommonTestUtils.assertThrows;
+import static org.apache.flink.core.testutils.FlinkAssertions.assertThatFuture;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.hamcrest.CoreMatchers.hasItems;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.lessThanOrEqualTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.fail;
/** IT cases for {@link RestClient} and {@link RestServerEndpoint}. */
-@RunWith(Parameterized.class)
-public class RestServerEndpointITCase extends TestLogger {
+@ExtendWith(ParameterizedTestExtension.class)
+public class RestServerEndpointITCase {
private static final JobID PATH_JOB_ID = new JobID();
private static final JobID QUERY_JOB_ID = new JobID();
private static final String JOB_ID_KEY = "jobid";
private static final Time timeout = Time.seconds(10L);
private static final int TEST_REST_MAX_CONTENT_LENGTH = 4096;
- @ClassRule
- public static final TestExecutorResource<ScheduledExecutorService>
EXECUTOR_RESOURCE =
- TestingUtils.defaultExecutorResource();
+ @RegisterExtension
+ public static final TestExecutorExtension<ScheduledExecutorService>
EXECUTOR_RESOURCE =
Review Comment:
```suggestion
private static final TestExecutorExtension<ScheduledExecutorService>
EXECUTOR_RESOURCE =
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointITCase.java:
##########
@@ -519,18 +503,19 @@ public void testVersionSelection() throws Exception {
try {
version2Response.get(5, TimeUnit.SECONDS);
- fail();
+ fail("Expected exception not thrown");
} catch (ExecutionException ee) {
RestClientException rce = (RestClientException) ee.getCause();
- assertEquals(HttpResponseStatus.ACCEPTED,
rce.getHttpResponseStatus());
+
assertThat(rce.getHttpResponseStatus()).isEqualTo(HttpResponseStatus.ACCEPTED);
}
Review Comment:
Same as above.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointITCase.java:
##########
@@ -340,60 +331,52 @@ public void testBadHandlerRequest() throws Exception {
} catch (ExecutionException ee) {
Throwable t = ExceptionUtils.stripExecutionException(ee);
- assertTrue(t instanceof RestClientException);
+ assertThat(t instanceof RestClientException).isTrue();
RestClientException rce = (RestClientException) t;
- assertEquals(HttpResponseStatus.BAD_REQUEST,
rce.getHttpResponseStatus());
+
assertThat(rce.getHttpResponseStatus()).isEqualTo(HttpResponseStatus.BAD_REQUEST);
Review Comment:
Maybe we can use `assertThatFuture` as well.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointITCase.java:
##########
@@ -501,10 +485,10 @@ public void testVersionSelection() throws Exception {
try {
version1Response.get(5, TimeUnit.SECONDS);
- fail();
+ fail("Expected exception not thrown");
} catch (ExecutionException ee) {
RestClientException rce = (RestClientException) ee.getCause();
- assertEquals(HttpResponseStatus.OK, rce.getHttpResponseStatus());
+
assertThat(rce.getHttpResponseStatus()).isEqualTo(HttpResponseStatus.OK);
}
Review Comment:
I think we can use
`assertThatFuture().eventuallyFailsWith(ExecutionException.class)...` as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
