You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "huwh (via GitHub)" <gi...@apache.org> on 2023/09/20 08:05:10 UTC
[GitHub] [flink] huwh commented on a diff in pull request #23242: [FLINK-32851][runtime][JUnit5 Migration] The rest package of flink-runtime module
huwh commented on code in PR #23242:
URL: https://github.com/apache/flink/pull/23242#discussion_r1328092633
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/async/CompletedOperationCacheTest.java:
##########
@@ -154,35 +149,35 @@ public void testCacheTimeoutCanBeConfigured() throws Exception {
manualTicker.advanceTime(baseTimeout.multipliedBy(2).getSeconds(), TimeUnit.SECONDS);
- assertTrue(completedOperationCache.get(TEST_OPERATION_KEY).isPresent());
+ assertThat(completedOperationCache.get(TEST_OPERATION_KEY)).isPresent();
}
@Test
- public void containsReturnsFalseForUnknownOperation() {
- assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY), is(false));
+ void containsReturnsFalseForUnknownOperation() {
+ assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY)).isFalse();
}
@Test
- public void containsChecksOnoingOperations() {
+ void containsChecksOnoingOperations() {
completedOperationCache.registerOngoingOperation(
TEST_OPERATION_KEY, new CompletableFuture<>());
- assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY), is(true));
+ assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY)).isTrue();
}
@Test
- public void containsChecksCompletedOperations() {
+ void containsChecksCompletedOperations() {
completedOperationCache.registerOngoingOperation(
TEST_OPERATION_KEY, CompletableFuture.completedFuture(null));
- assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY), is(true));
+ assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY)).isTrue();
}
@Test
- public void containsDoesNotMarkResultAsAccessed() {
+ void containsDoesNotMarkResultAsAccessed() {
completedOperationCache.registerOngoingOperation(
TEST_OPERATION_KEY, CompletableFuture.completedFuture(null));
- assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY), is(true));
- assertThat(
- completedOperationCache.closeAsync(),
- FlinkMatchers.willNotComplete(Duration.ofMillis(10)));
+ assertThat(completedOperationCache.containsOperation(TEST_OPERATION_KEY)).isTrue();
+
+ assertThat(completedOperationCache.closeAsync())
Review Comment:
```suggestion
assertThatFuture(completedOperationCache.closeAsync())
.willNotCompleteWithin(Duration.ofMillis(10));
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointITCase.java:
##########
@@ -543,21 +529,23 @@ public void testDefaultVersionRouting() throws Exception {
.build();
try (final Response response = client.newCall(request).execute()) {
- assertEquals(HttpResponseStatus.ACCEPTED.code(), response.code());
+ assertThat(response.code()).isEqualTo(HttpResponseStatus.ACCEPTED.code());
}
}
- @Test
- public void testNonSslRedirectForEnabledSsl() throws Exception {
- Assume.assumeTrue(config.getBoolean(SecurityOptions.SSL_REST_ENABLED));
+ @TestTemplate
+ void testNonSslRedirectForEnabledSsl() throws Exception {
+ if (!config.getBoolean(SecurityOptions.SSL_REST_ENABLED)) {
Review Comment:
ditto
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/job/metrics/AggregatingMetricsHandlerTestBase.java:
##########
@@ -185,9 +183,9 @@ public void getStores() throws Exception {
.sorted()
.collect(Collectors.toList());
- assertEquals(1, sortedMetrics1.size());
+ assertThat(sortedMetrics1).hasSize(1);
Review Comment:
ditto
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/messages/json/JobResultDeserializerTest.java:
##########
@@ -78,20 +74,19 @@ public void testInvalidType() throws Exception {
+ "}",
JobResult.class);
} catch (final JsonMappingException e) {
- assertThat(
- e.getMessage(),
- containsString("Expected token VALUE_NUMBER_INT (was VALUE_STRING)"));
+ assertThat(e.getMessage())
Review Comment:
```suggestion
assertThatThrownBy(
() ->
objectMapper.readValue(
"{\n"
+ "\t\"id\": \"1bb5e8c7df49938733b7c6a73678de6a\",\n"
+ "\t\"net-runtime\": \"invalid\"\n"
+ "}",
JobResult.class))
.isInstanceOf(JsonMappingException.class)
.hasMessageContaining("Expected token VALUE_NUMBER_INT (was VALUE_STRING)");
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/messages/json/JobResultDeserializerTest.java:
##########
@@ -78,20 +74,19 @@ public void testInvalidType() throws Exception {
+ "}",
JobResult.class);
} catch (final JsonMappingException e) {
- assertThat(
- e.getMessage(),
- containsString("Expected token VALUE_NUMBER_INT (was VALUE_STRING)"));
+ assertThat(e.getMessage())
+ .contains("Expected token VALUE_NUMBER_INT (was VALUE_STRING)");
}
}
@Test
- public void testIncompleteJobResult() throws Exception {
+ void testIncompleteJobResult() throws Exception {
try {
objectMapper.readValue(
"{\n" + "\t\"id\": \"1bb5e8c7df49938733b7c6a73678de6a\"\n" + "}",
JobResult.class);
} catch (final JsonMappingException e) {
- assertThat(e.getMessage(), containsString("Could not deserialize JobResult"));
+ assertThat(e.getMessage()).contains("Could not deserialize JobResult");
Review Comment:
ditto
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/job/metrics/AggregatingMetricsHandlerTestBase.java:
##########
@@ -145,10 +143,10 @@ public void getStores() throws Exception {
.sorted()
.collect(Collectors.toList());
- assertEquals(2, sortedMetrics1.size());
+ assertThat(sortedMetrics1).hasSize(2);
Review Comment:
```suggestion
assertThat(sortedMetrics1).containsExactly("1", "3");
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/job/metrics/AggregatingMetricsHandlerTestBase.java:
##########
@@ -221,13 +219,13 @@ public void testListMetrics() throws Exception {
.sorted()
.collect(Collectors.toList());
- assertEquals(2, availableMetrics.size());
- assertEquals("abc.metric1", availableMetrics.get(0));
- assertEquals("abc.metric2", availableMetrics.get(1));
+ assertThat(availableMetrics).hasSize(2);
Review Comment:
ditto
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/job/metrics/AggregatingMetricsHandlerTestBase.java:
##########
@@ -196,14 +194,14 @@ public void getStores() throws Exception {
.sorted()
.collect(Collectors.toList());
- assertEquals(1, sortedMetrics2.size());
+ assertThat(sortedMetrics2).hasSize(1);
Review Comment:
ditto
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestServerEndpointITCase.java:
##########
@@ -517,20 +499,24 @@ public void testVersionSelection() throws Exception {
Collections.emptyList(),
RuntimeRestAPIVersion.V1);
- try {
- version2Response.get(5, TimeUnit.SECONDS);
- fail();
- } catch (ExecutionException ee) {
- RestClientException rce = (RestClientException) ee.getCause();
- assertEquals(HttpResponseStatus.ACCEPTED, rce.getHttpResponseStatus());
- }
+ assertThatFuture(version2Response)
+ .failsWithin(5, TimeUnit.SECONDS)
+ .withThrowableOfType(ExecutionException.class)
+ .withCauseInstanceOf(RestClientException.class)
+ .satisfies(
+ e ->
+ assertThat(
+ ((RestClientException) e.getCause())
+ .getHttpResponseStatus())
+ .isEqualTo(HttpResponseStatus.ACCEPTED));
}
- @Test
- public void testDefaultVersionRouting() throws Exception {
- Assume.assumeFalse(
- "Ignoring SSL-enabled test to keep OkHttp usage simple.",
- config.getBoolean(SecurityOptions.SSL_REST_ENABLED));
+ @TestTemplate
+ void testDefaultVersionRouting() throws Exception {
+ // Ignoring SSL-enabled test to keep OkHttp usage simple.
+ if (config.getBoolean(SecurityOptions.SSL_REST_ENABLED)) {
Review Comment:
It's betther to use org.assertj.core.api.Assumptions.assumeThat
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/legacy/DefaultExecutionGraphCacheTest.java:
##########
@@ -79,20 +75,20 @@ public void testExecutionGraphCaching() throws Exception {
CompletableFuture<ExecutionGraphInfo> executionGraphInfoFuture =
executionGraphCache.getExecutionGraphInfo(expectedJobId, restfulGateway);
- assertEquals(expectedExecutionGraphInfo, executionGraphInfoFuture.get());
+ assertThat(executionGraphInfoFuture.get()).isEqualTo(expectedExecutionGraphInfo);
Review Comment:
shall we change all the future related assert to `FlinkAssertions.assertThatFuture` ?
##########
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).hasSize(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).hasSize(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:
```suggestion
final CompletableFuture<TestResponse> response =
sendRequestToTestHandler(new TestRequest());
FlinkAssertions.assertThatFuture(response)
.eventuallyFailsWith(ExecutionException.class)
.as("Request must fail with 2 times redirected URL")
.withMessageContaining(REDIRECT2_URL);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/handler/job/metrics/AggregatingMetricsHandlerTestBase.java:
##########
@@ -157,9 +155,9 @@ public void getStores() throws Exception {
.sorted()
.collect(Collectors.toList());
- assertEquals(1, sortedMetrics2.size());
+ assertThat(sortedMetrics2).hasSize(1);
Review Comment:
ditto
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org