zentol commented on code in PR #19752:
URL: https://github.com/apache/flink/pull/19752#discussion_r875572184


##########
flink-python/src/test/java/org/apache/flink/table/runtime/typeutils/serializers/python/timestamp/TimestampSerializerTest.java:
##########
@@ -16,10 +16,11 @@
  * limitations under the License.
  */
 
-package org.apache.flink.table.runtime.typeutils.serializers.python;
+package org.apache.flink.table.runtime.typeutils.serializers.python.timestamp;

Review Comment:
   This doesn't seem correct. shouldn't it be in the same package as the 
serializer?



##########
flink-python/src/test/java/org/apache/flink/client/python/PythonDriverTest.java:
##########
@@ -44,33 +45,33 @@ public void testStartGatewayServer() throws 
ExecutionException, InterruptedExcep
     }
 
     @Test
-    public void testConstructCommandsWithEntryPointModule() {
+    void testConstructCommandsWithEntryPointModule() {
         List<String> args = new ArrayList<>();
         args.add("--input");
         args.add("in.txt");
 
         PythonDriverOptions pythonDriverOptions = new 
PythonDriverOptions("xxx", null, args);
         List<String> commands = 
PythonDriver.constructPythonCommands(pythonDriverOptions);
         // verify the generated commands
-        Assert.assertEquals(4, commands.size());
-        Assert.assertEquals(commands.get(0), "-m");
-        Assert.assertEquals(commands.get(1), "xxx");
-        Assert.assertEquals(commands.get(2), "--input");
-        Assert.assertEquals(commands.get(3), "in.txt");
+        assertThat(commands).hasSize(4);
+        assertThat(commands.get(0)).isEqualTo("-m");
+        assertThat(commands.get(1)).isEqualTo("xxx");
+        assertThat(commands.get(2)).isEqualTo("--input");
+        assertThat(commands.get(3)).isEqualTo("in.txt");

Review Comment:
   ```suggestion
           assertThat(commands).containsExactly("-m", "xxx", "--input", 
"in.txt");
   ```



##########
flink-python/src/test/java/org/apache/flink/python/chain/PythonOperatorChainingOptimizerTest.java:
##########
@@ -99,46 +96,49 @@ public void 
testChainedTransformationPropertiesCorrectlySet() {
 
         List<Transformation<?>> optimized =
                 PythonOperatorChainingOptimizer.optimize(transformations);
-        assertEquals(2, optimized.size());
+        assertThat(optimized).hasSize(2);
 
         OneInputTransformation<?, ?> chainedTransformation =
                 (OneInputTransformation<?, ?>) optimized.get(1);
-        assertEquals(2, chainedTransformation.getParallelism());
-        assertEquals(sourceTransformation.getOutputType(), 
chainedTransformation.getInputType());
-        assertEquals(processOperator.getProducedType(), 
chainedTransformation.getOutputType());
-        assertEquals(keyedProcessTransformation.getUid(), 
chainedTransformation.getUid());
-        assertEquals("group", 
chainedTransformation.getSlotSharingGroup().get().getName());
-        assertEquals("col", chainedTransformation.getCoLocationGroupKey());
-        assertEquals(64, chainedTransformation.getMaxParallelism());
-        assertEquals(500L, chainedTransformation.getBufferTimeout());
-        assertEquals(
-                15,
-                (int)
+        assertThat(chainedTransformation.getParallelism()).isEqualTo(2);
+        assertThat(sourceTransformation.getOutputType())
+                .isEqualTo(chainedTransformation.getInputType());
+        assertThat(processOperator.getProducedType())
+                .isEqualTo(chainedTransformation.getOutputType());
+        
assertThat(keyedProcessTransformation.getUid()).isEqualTo(chainedTransformation.getUid());
+        
assertThat(chainedTransformation.getSlotSharingGroup().get().getName()).isEqualTo("group");
+        
assertThat(chainedTransformation.getCoLocationGroupKey()).isEqualTo("col");
+        assertThat(chainedTransformation.getMaxParallelism()).isEqualTo(64);
+        assertThat(chainedTransformation.getBufferTimeout()).isEqualTo(500L);
+        assertThat(
+                        (int)
+                                chainedTransformation
+                                        
.getManagedMemoryOperatorScopeUseCaseWeights()
+                                        
.getOrDefault(ManagedMemoryUseCase.OPERATOR, 0))
+                .isEqualTo(15);
+        
assertThat(chainedTransformation.getOperatorFactory().getChainingStrategy())
+                .isEqualTo(ChainingStrategy.HEAD);
+        assertThat(
                         chainedTransformation
-                                .getManagedMemoryOperatorScopeUseCaseWeights()
-                                .getOrDefault(ManagedMemoryUseCase.OPERATOR, 
0));
-        assertEquals(
-                ChainingStrategy.HEAD,
-                
chainedTransformation.getOperatorFactory().getChainingStrategy());
-        assertTrue(
-                chainedTransformation
-                        .getManagedMemorySlotScopeUseCases()
-                        .contains(ManagedMemoryUseCase.PYTHON));
-        assertTrue(
-                chainedTransformation
-                        .getManagedMemorySlotScopeUseCases()
-                        .contains(ManagedMemoryUseCase.STATE_BACKEND));
+                                .getManagedMemorySlotScopeUseCases()
+                                .contains(ManagedMemoryUseCase.PYTHON))
+                .isTrue();
+        assertThat(
+                        chainedTransformation
+                                .getManagedMemorySlotScopeUseCases()
+                                .contains(ManagedMemoryUseCase.STATE_BACKEND))
+                .isTrue();

Review Comment:
   another instance directly above



##########
flink-python/src/test/java/org/apache/flink/client/python/PythonDriverOptionsParserFactoryTest.java:
##########
@@ -21,61 +21,64 @@
 import org.apache.flink.runtime.entrypoint.FlinkParseException;
 import org.apache.flink.runtime.entrypoint.parser.CommandLineParser;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import java.util.List;
 
-import static org.junit.Assert.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /** Tests for the {@link PythonDriverOptionsParserFactory}. */
-public class PythonDriverOptionsParserFactoryTest {
+class PythonDriverOptionsParserFactoryTest {
 
     private static final CommandLineParser<PythonDriverOptions> 
commandLineParser =
             new CommandLineParser<>(new PythonDriverOptionsParserFactory());
 
     @Test
-    public void testPythonDriverOptionsParsing() throws FlinkParseException {
+    void testPythonDriverOptionsParsing() throws FlinkParseException {
         final String[] args = {"--python", "xxx.py", "--input", "in.txt"};
         verifyPythonDriverOptionsParsing(args);
     }
 
     @Test
-    public void testPymoduleOptionParsing() throws FlinkParseException {
+    void testPymoduleOptionParsing() throws FlinkParseException {
         final String[] args = {"--pyModule", "xxx", "--input", "in.txt"};
         verifyPythonDriverOptionsParsing(args);
     }
 
     @Test
-    public void testShortOptions() throws FlinkParseException {
+    void testShortOptions() throws FlinkParseException {
         final String[] args = {"-py", "xxx.py", "--input", "in.txt"};
         verifyPythonDriverOptionsParsing(args);
     }
 
-    @Test(expected = FlinkParseException.class)
-    public void testMultipleEntrypointsSpecified() throws FlinkParseException {
+    @Test
+    void testMultipleEntrypointsSpecified() {
         final String[] args = {"--python", "xxx.py", "--pyModule", "yyy", 
"--input", "in.txt"};
-        commandLineParser.parse(args);
+        assertThatThrownBy(() -> commandLineParser.parse(args))
+                .isInstanceOf(FlinkParseException.class);
     }
 
-    @Test(expected = FlinkParseException.class)
-    public void testEntrypointNotSpecified() throws FlinkParseException {
+    @Test
+    void testEntrypointNotSpecified() {
         final String[] args = {"--input", "in.txt"};
-        commandLineParser.parse(args);
+        assertThatThrownBy(() -> commandLineParser.parse(args))
+                .isInstanceOf(FlinkParseException.class);
     }
 
     private void verifyPythonDriverOptionsParsing(final String[] args) throws 
FlinkParseException {
         final PythonDriverOptions pythonCommandOptions = 
commandLineParser.parse(args);
 
         if (pythonCommandOptions.getEntryPointScript().isPresent()) {
-            assertEquals("xxx.py", 
pythonCommandOptions.getEntryPointScript().get());
+            
assertThat(pythonCommandOptions.getEntryPointScript().get()).isEqualTo("xxx.py");
         } else {
-            assertEquals("xxx", pythonCommandOptions.getEntryPointModule());
+            
assertThat(pythonCommandOptions.getEntryPointModule()).isEqualTo("xxx");
         }
 
         // verify the python program arguments
         final List<String> programArgs = pythonCommandOptions.getProgramArgs();
-        assertEquals(2, programArgs.size());
-        assertEquals("--input", programArgs.get(0));
-        assertEquals("in.txt", programArgs.get(1));
+        assertThat(programArgs).hasSize(2);
+        assertThat(programArgs.get(0)).isEqualTo("--input");
+        assertThat(programArgs.get(1)).isEqualTo("in.txt");

Review Comment:
   ```suggestion
           assertThat(programArgs).containsExactly("--input", "in.txt");
   ```



##########
flink-python/src/test/java/org/apache/flink/client/python/PythonDriverTest.java:
##########
@@ -44,33 +45,33 @@ public void testStartGatewayServer() throws 
ExecutionException, InterruptedExcep
     }
 
     @Test
-    public void testConstructCommandsWithEntryPointModule() {
+    void testConstructCommandsWithEntryPointModule() {
         List<String> args = new ArrayList<>();
         args.add("--input");
         args.add("in.txt");
 
         PythonDriverOptions pythonDriverOptions = new 
PythonDriverOptions("xxx", null, args);
         List<String> commands = 
PythonDriver.constructPythonCommands(pythonDriverOptions);
         // verify the generated commands
-        Assert.assertEquals(4, commands.size());
-        Assert.assertEquals(commands.get(0), "-m");
-        Assert.assertEquals(commands.get(1), "xxx");
-        Assert.assertEquals(commands.get(2), "--input");
-        Assert.assertEquals(commands.get(3), "in.txt");
+        assertThat(commands).hasSize(4);
+        assertThat(commands.get(0)).isEqualTo("-m");
+        assertThat(commands.get(1)).isEqualTo("xxx");
+        assertThat(commands.get(2)).isEqualTo("--input");
+        assertThat(commands.get(3)).isEqualTo("in.txt");
     }
 
     @Test
-    public void testConstructCommandsWithEntryPointScript() {
+    void testConstructCommandsWithEntryPointScript() {
         List<String> args = new ArrayList<>();
         args.add("--input");
         args.add("in.txt");
 
         PythonDriverOptions pythonDriverOptions = new 
PythonDriverOptions(null, "xxx.py", args);
         List<String> commands = 
PythonDriver.constructPythonCommands(pythonDriverOptions);
-        Assert.assertEquals(4, commands.size());
-        Assert.assertEquals(commands.get(0), "-m");
-        Assert.assertEquals(commands.get(1), "xxx");
-        Assert.assertEquals(commands.get(2), "--input");
-        Assert.assertEquals(commands.get(3), "in.txt");
+        assertThat(commands).hasSize(4);
+        assertThat(commands.get(0)).isEqualTo("-m");
+        assertThat(commands.get(1)).isEqualTo("xxx");
+        assertThat(commands.get(2)).isEqualTo("--input");
+        assertThat(commands.get(3)).isEqualTo("in.txt");

Review Comment:
   ```suggestion
           assertThat(commands).containsExactly("-m", "xxx", "--input", 
"in.txt");
   ```



##########
flink-python/src/test/java/org/apache/flink/python/chain/PythonOperatorChainingOptimizerTest.java:
##########
@@ -99,46 +96,49 @@ public void 
testChainedTransformationPropertiesCorrectlySet() {
 
         List<Transformation<?>> optimized =
                 PythonOperatorChainingOptimizer.optimize(transformations);
-        assertEquals(2, optimized.size());
+        assertThat(optimized).hasSize(2);
 
         OneInputTransformation<?, ?> chainedTransformation =
                 (OneInputTransformation<?, ?>) optimized.get(1);
-        assertEquals(2, chainedTransformation.getParallelism());
-        assertEquals(sourceTransformation.getOutputType(), 
chainedTransformation.getInputType());
-        assertEquals(processOperator.getProducedType(), 
chainedTransformation.getOutputType());
-        assertEquals(keyedProcessTransformation.getUid(), 
chainedTransformation.getUid());
-        assertEquals("group", 
chainedTransformation.getSlotSharingGroup().get().getName());
-        assertEquals("col", chainedTransformation.getCoLocationGroupKey());
-        assertEquals(64, chainedTransformation.getMaxParallelism());
-        assertEquals(500L, chainedTransformation.getBufferTimeout());
-        assertEquals(
-                15,
-                (int)
+        assertThat(chainedTransformation.getParallelism()).isEqualTo(2);
+        assertThat(sourceTransformation.getOutputType())
+                .isEqualTo(chainedTransformation.getInputType());
+        assertThat(processOperator.getProducedType())
+                .isEqualTo(chainedTransformation.getOutputType());
+        
assertThat(keyedProcessTransformation.getUid()).isEqualTo(chainedTransformation.getUid());
+        
assertThat(chainedTransformation.getSlotSharingGroup().get().getName()).isEqualTo("group");
+        
assertThat(chainedTransformation.getCoLocationGroupKey()).isEqualTo("col");
+        assertThat(chainedTransformation.getMaxParallelism()).isEqualTo(64);
+        assertThat(chainedTransformation.getBufferTimeout()).isEqualTo(500L);
+        assertThat(
+                        (int)
+                                chainedTransformation
+                                        
.getManagedMemoryOperatorScopeUseCaseWeights()
+                                        
.getOrDefault(ManagedMemoryUseCase.OPERATOR, 0))
+                .isEqualTo(15);
+        
assertThat(chainedTransformation.getOperatorFactory().getChainingStrategy())
+                .isEqualTo(ChainingStrategy.HEAD);
+        assertThat(
                         chainedTransformation
-                                .getManagedMemoryOperatorScopeUseCaseWeights()
-                                .getOrDefault(ManagedMemoryUseCase.OPERATOR, 
0));
-        assertEquals(
-                ChainingStrategy.HEAD,
-                
chainedTransformation.getOperatorFactory().getChainingStrategy());
-        assertTrue(
-                chainedTransformation
-                        .getManagedMemorySlotScopeUseCases()
-                        .contains(ManagedMemoryUseCase.PYTHON));
-        assertTrue(
-                chainedTransformation
-                        .getManagedMemorySlotScopeUseCases()
-                        .contains(ManagedMemoryUseCase.STATE_BACKEND));
+                                .getManagedMemorySlotScopeUseCases()
+                                .contains(ManagedMemoryUseCase.PYTHON))
+                .isTrue();
+        assertThat(
+                        chainedTransformation
+                                .getManagedMemorySlotScopeUseCases()
+                                .contains(ManagedMemoryUseCase.STATE_BACKEND))
+                .isTrue();

Review Comment:
   ```suggestion
           assertThat(
                           chainedTransformation
                                   .getManagedMemorySlotScopeUseCases())
                   .contains(ManagedMemoryUseCase.STATE_BACKEND);
   ```



-- 
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]

Reply via email to