XComp commented on code in PR #19914:
URL: https://github.com/apache/flink/pull/19914#discussion_r983676804
##########
flink-connectors/flink-connector-rabbitmq/src/test/java/org/apache/flink/streaming/connectors/rabbitmq/RMQSourceITCase.java:
##########
@@ -112,7 +117,8 @@ public void setUp() throws Exception {
}
@Test
- public void testStopWithSavepoint() throws Exception {
+ void testStopWithSavepoint(@InjectClusterClient RestClusterClient<?>
clusterClient)
Review Comment:
```suggestion
void testStopWithSavepoint(@InjectClusterClient RestClusterClient<?>
clusterClient, @TempDir Path tmp)
```
nit: We could also just path the temporary directory into this method
instead of having it as a field variable. ...since it's the only location where
the temporary directory is used. But that's just personal taste, I guess.
:thinking:
##########
flink-connectors/flink-connector-rabbitmq/src/test/java/org/apache/flink/streaming/connectors/rabbitmq/common/RMQConnectionConfigTest.java:
##########
@@ -18,63 +18,74 @@
package org.apache.flink.streaming.connectors.rabbitmq.common;
import com.rabbitmq.client.ConnectionFactory;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.net.URISyntaxException;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
/** Tests for the {@link RMQConnectionConfig}. */
-public class RMQConnectionConfigTest {
+class RMQConnectionConfigTest {
- @Test(expected = NullPointerException.class)
- public void shouldThrowNullPointExceptionIfHostIsNull()
- throws NoSuchAlgorithmException, KeyManagementException,
URISyntaxException {
- RMQConnectionConfig connectionConfig =
- new RMQConnectionConfig.Builder()
- .setPort(1000)
- .setUserName("guest")
- .setPassword("guest")
- .setVirtualHost("/")
- .build();
- connectionConfig.getConnectionFactory();
+ @Test
+ void shouldThrowNullPointExceptionIfHostIsNull() {
+ assertThatThrownBy(
+ () -> {
+ RMQConnectionConfig connectionConfig =
+ new RMQConnectionConfig.Builder()
+ .setPort(1000)
+ .setUserName("guest")
+ .setPassword("guest")
+ .setVirtualHost("/")
+ .build();
+ connectionConfig.getConnectionFactory();
+ })
+ .isInstanceOf(NullPointerException.class);
}
- @Test(expected = NullPointerException.class)
- public void shouldThrowNullPointExceptionIfPortIsNull()
+ @Test
+ void shouldThrowNullPointExceptionIfPortIsNull()
throws NoSuchAlgorithmException, KeyManagementException,
URISyntaxException {
Review Comment:
```suggestion
```
unused
##########
flink-connectors/flink-connector-rabbitmq/src/test/java/org/apache/flink/streaming/connectors/rabbitmq/RMQSourceITCase.java:
##########
@@ -112,7 +117,8 @@ public void setUp() throws Exception {
}
@Test
- public void testStopWithSavepoint() throws Exception {
+ void testStopWithSavepoint(@InjectClusterClient RestClusterClient<?>
clusterClient)
Review Comment:
```suggestion
void testStopWithSavepoint(@InjectClusterClient RestClusterClient<?>
clusterClient, @TempDir Path tmp)
```
nit: we could inject the `@TempDir` as a parameter since it's only used once
in the test class.
##########
flink-connectors/flink-connector-rabbitmq/src/test/java/org/apache/flink/streaming/connectors/rabbitmq/RMQSourceTest.java:
##########
@@ -423,37 +418,38 @@ public void testConstructorParams() throws Exception {
// connection fails but check if args have been passed correctly
}
- assertEquals("hostTest", testObj.getFactory().getHost());
- assertEquals(999, testObj.getFactory().getPort());
- assertEquals("userTest", testObj.getFactory().getUsername());
- assertEquals("passTest", testObj.getFactory().getPassword());
+ assertThat(testObj.getFactory().getHost()).isEqualTo("hostTest");
+ assertThat(testObj.getFactory().getPort()).isEqualTo(999);
+ assertThat(testObj.getFactory().getUsername()).isEqualTo("userTest");
+ assertThat(testObj.getFactory().getPassword()).isEqualTo("passTest");
}
- @Test(timeout = 30000L)
- public void testCustomIdentifiers() throws Exception {
+ @Test
+ @Timeout(30)
Review Comment:
```suggestion
@Timeout(value = 30_000, unit = TimeUnit.MILLISECONDS)
```
I feel like adding the time unit here helps the readability. Especially,
because readers might be more used to using milliseconds as a timeout unit.
`@Timeout(30)` might leave the wrong impression. WDYT?
##########
flink-connectors/flink-connector-rabbitmq/src/test/java/org/apache/flink/streaming/connectors/rabbitmq/common/RMQConnectionConfigTest.java:
##########
@@ -18,63 +18,74 @@
package org.apache.flink.streaming.connectors.rabbitmq.common;
import com.rabbitmq.client.ConnectionFactory;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.net.URISyntaxException;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
/** Tests for the {@link RMQConnectionConfig}. */
-public class RMQConnectionConfigTest {
+class RMQConnectionConfigTest {
- @Test(expected = NullPointerException.class)
- public void shouldThrowNullPointExceptionIfHostIsNull()
- throws NoSuchAlgorithmException, KeyManagementException,
URISyntaxException {
- RMQConnectionConfig connectionConfig =
- new RMQConnectionConfig.Builder()
- .setPort(1000)
- .setUserName("guest")
- .setPassword("guest")
- .setVirtualHost("/")
- .build();
- connectionConfig.getConnectionFactory();
+ @Test
+ void shouldThrowNullPointExceptionIfHostIsNull() {
+ assertThatThrownBy(
+ () -> {
+ RMQConnectionConfig connectionConfig =
+ new RMQConnectionConfig.Builder()
+ .setPort(1000)
+ .setUserName("guest")
+ .setPassword("guest")
+ .setVirtualHost("/")
+ .build();
+ connectionConfig.getConnectionFactory();
+ })
+ .isInstanceOf(NullPointerException.class);
}
- @Test(expected = NullPointerException.class)
- public void shouldThrowNullPointExceptionIfPortIsNull()
+ @Test
+ void shouldThrowNullPointExceptionIfPortIsNull()
throws NoSuchAlgorithmException, KeyManagementException,
URISyntaxException {
- RMQConnectionConfig connectionConfig =
- new RMQConnectionConfig.Builder()
- .setHost("localhost")
- .setUserName("guest")
- .setPassword("guest")
- .setVirtualHost("/")
- .build();
- connectionConfig.getConnectionFactory();
+ assertThatThrownBy(
+ () -> {
+ RMQConnectionConfig connectionConfig =
+ new RMQConnectionConfig.Builder()
+ .setHost("localhost")
+ .setUserName("guest")
+ .setPassword("guest")
+ .setVirtualHost("/")
+ .build();
+ connectionConfig.getConnectionFactory();
+ })
+ .isInstanceOf(NullPointerException.class);
}
- @Test(expected = NullPointerException.class)
- public void shouldSetDefaultValueIfConnectionTimeoutNotGiven()
+ @Test
+ void shouldSetDefaultValueIfConnectionTimeoutNotGiven()
throws NoSuchAlgorithmException, KeyManagementException,
URISyntaxException {
Review Comment:
```suggestion
```
unused
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
