You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/18 07:53:37 UTC

[GitHub] [flink] zentol commented on a diff in pull request #19752: [FLINK-27340][tests] Migrate module flink-python to JUnit5

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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org