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/17 16:20:05 UTC

[GitHub] [flink] snuyanzin opened a new pull request, #19752: [FLINK-27340][tests] Migrate module flink-python to JUnit5

snuyanzin opened a new pull request, #19752:
URL: https://github.com/apache/flink/pull/19752

   ## What is the purpose of the change
   
   Update the flink-python module to AssertJ and JUnit 5 following the [JUnit 5 Migration Guide](https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit)
   
   
   ## Brief change log
   
   Use JUnit5 and AssertJ in tests instead of JUnit4 and Hamcrest
   
   
   ## Verifying this change
   
   This change is a code cleanup without any test coverage.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): ( no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: ( no)
     - The serializers: ( no)
     - The runtime per-record code paths (performance sensitive): ( no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no)
     - The S3 file system connector: ( no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? ( no)
     - If yes, how is the feature documented? (not applicable)
   


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


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

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19752:
URL: https://github.com/apache/flink/pull/19752#issuecomment-1129072997

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c3f34db24204a4d4ae5544688139b3de3f143fc1",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "c3f34db24204a4d4ae5544688139b3de3f143fc1",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * c3f34db24204a4d4ae5544688139b3de3f143fc1 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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


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

Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19752:
URL: https://github.com/apache/flink/pull/19752#discussion_r875836056


##########
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:
   Good catch.
   Looks like an artifact after merge (since I started it before `SerializerTestBase` issue was solved) 
   Now I completely removed all changes for this class from this PR
   



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


[GitHub] [flink] zentol merged pull request #19752: [FLINK-27340][tests] Migrate module flink-python to JUnit5

Posted by GitBox <gi...@apache.org>.
zentol merged PR #19752:
URL: https://github.com/apache/flink/pull/19752


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


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

Posted by GitBox <gi...@apache.org>.
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