Re: [PR] [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module [flink]
1996fanrui merged PR #23932: URL: https://github.com/apache/flink/pull/23932 -- 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
Re: [PR] [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module [flink]
RocMarshal commented on code in PR #23932:
URL: https://github.com/apache/flink/pull/23932#discussion_r1427882696
##
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/DeclarativeSlotPoolBridgeResourceDeclarationTest.java:
##
@@ -141,40 +137,38 @@ public void
testRequirementsDecreasedOnAllocationTimeout() throws Exception {
.get();
// waiting for the timeout
-assertThat(
-allocationFuture,
-
FlinkMatchers.futureWillCompleteExceptionally(Duration.ofMinutes(1)));
-
+
assertThatFuture(allocationFuture).failsWithin(Duration.ofMinutes(1));
+Thread.sleep(5L);
Review Comment:
Nice catch!
sorry for it. Maybe it's a dirty line from the cp.
I'll remove it and check it on the other corresponding PRs.
--
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
Re: [PR] [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module [flink]
1996fanrui commented on code in PR #23932:
URL: https://github.com/apache/flink/pull/23932#discussion_r1427755866
##
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/DeclarativeSlotPoolBridgeRequestCompletionTest.java:
##
@@ -43,27 +44,24 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.nullValue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests how the {@link DeclarativeSlotPoolBridge} completes slot requests. */
-public class DeclarativeSlotPoolBridgeRequestCompletionTest extends TestLogger
{
+@ExtendWith(TestLoggerExtension.class)
Review Comment:
```suggestion
```
##
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/DeclarativeSlotPoolBridgeTest.java:
##
@@ -55,34 +55,30 @@
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.fail;
+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;
/** Tests for the {@link DeclarativeSlotPoolBridge}. */
-@RunWith(Parameterized.class)
-public class DeclarativeSlotPoolBridgeTest extends TestLogger {
+@ExtendWith({TestLoggerExtension.class, ParameterizedTestExtension.class})
Review Comment:
```suggestion
@ExtendWith(ParameterizedTestExtension.class)
```
##
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/DeclarativeSlotPoolBridgeResourceDeclarationTest.java:
##
@@ -141,40 +137,38 @@ public void
testRequirementsDecreasedOnAllocationTimeout() throws Exception {
.get();
// waiting for the timeout
-assertThat(
-allocationFuture,
-
FlinkMatchers.futureWillCompleteExceptionally(Duration.ofMinutes(1)));
-
+
assertThatFuture(allocationFuture).failsWithin(Duration.ofMinutes(1));
+Thread.sleep(5L);
Review Comment:
Why is it needed here?
##
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/DeclarativeSlotPoolBridgeResourceDeclarationTest.java:
##
@@ -44,37 +45,33 @@
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
+import static org.apache.flink.core.testutils.FlinkAssertions.assertThatFuture;
import static
org.apache.flink.runtime.jobmaster.slotpool.DeclarativeSlotPoolBridgeTest.createAllocatedSlot;
import static
org.apache.flink.runtime.jobmaster.slotpool.DeclarativeSlotPoolBridgeTest.createDeclarativeSlotPoolBridge;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for the {@link DeclarativeSlotPoolBridge}. */
-@RunWith(Parameterized.class)
-public class DeclarativeSlotPoolBridgeResourceDeclarationTest extends
TestLogger {
+@ExtendWith({TestLoggerExtension.class, ParameterizedTestExtension.class})
Review Comment:
```suggestion
@ExtendWith(ParameterizedTestExtension.class)
```
##
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/DeclarativeSlotPoolBridgePreferredAllocationsTest.java:
##
@@ -45,10 +45,10 @@
import static org.assertj.core.api.Assertions.assertThat;
@ExtendWith(TestLoggerExtension.class)
Review Comment:
```suggestion
```
`TestLoggerExtension` has been added in the module level.
--
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
Re: [PR] [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module [flink]
RocMarshal commented on PR #23932: URL: https://github.com/apache/flink/pull/23932#issuecomment-1857550752 Hi, @1996fanrui Could you help review this pr if you're in the free time? Thank you~ -- 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
Re: [PR] [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module [flink]
flinkbot commented on PR #23932: URL: https://github.com/apache/flink/pull/23932#issuecomment-1857412172 ## CI report: * bd328df2d2f1dea0cbe121e16a83734451e19306 UNKNOWN Bot commands The @flinkbot bot supports the following commands: - `@flinkbot run azure` re-run the last Azure build -- 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
[PR] [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module [flink]
RocMarshal opened a new pull request, #23932: URL: https://github.com/apache/flink/pull/23932 ## What is the purpose of the change [FLINK-33853][runtime][JUnit5 Migration] Migrate Junit5 for DeclarativeSlotPoolBridge test classes of runtime module ## Brief change log - DeclarativeSlotPoolBridgeRequestCompletionTest - DeclarativeSlotPoolBridgeResourceDeclarationTest - DeclarativeSlotPoolBridgePreferredAllocationsTest - DeclarativeSlotPoolBridgeTest ## Verifying this change N.A ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (yes / **no**) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**) - The serializers: (yes / **no** / don't know) - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / **no** / don't know) - The S3 file system connector: (yes / **no** / don't know) ## Documentation - Does this pull request introduce a new feature? (yes / **no**) - If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented) -- 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
