You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/04/25 17:42:21 UTC

[GitHub] [pulsar] david-streamlio opened a new pull request #10369: Added more unit tests to the JavaInstanceTest class

david-streamlio opened a new pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369


   ### Motivation
   
   Adding some more unit tests for the JavaInstance class, and doing a sanity check to see if a simple PR is able to build successfully in the OSS environment. 
   
   ### Modifications
   
   Added more unit tests for the JavaInstanceTest class, that's it, nothing else.
   
   ### Verifying this change
   
   This change is the addition of new unit tests to an existing Test class
   
   ### Does this pull request potentially affect one of the following parts:
   No
   
   ### Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (not applicable)
     - If a feature is not applicable for documentation, explain why? I am just adding new unit tests
   


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

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



[GitHub] [pulsar] david-streamlio commented on a change in pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
david-streamlio commented on a change in pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#discussion_r619878733



##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);

Review comment:
       Just being consistent with the behavior of the other Async function tests

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        // TODO Change this

Review comment:
       Removed




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

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#discussion_r619856505



##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -54,6 +55,52 @@ public void testLambda() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningFunction() throws Exception  {
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                (Function<String, String>) (input, context) -> null,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	instance.close();
+    }
+
+    @Test
+    public void testUserExceptionThrowingFunction() throws Exception  {
+    	Function<String, String> func = (input, context) -> {
+    		throw new UserException("Boom");
+    	};
+
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                func,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	assertNotNull(result.get().getUserException());

Review comment:
       What about using assertSame? And test about the absence or presence of wrapper?

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        // TODO Change this

Review comment:
       Please do not leave TODOs

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);

Review comment:
       Why do you need this sleep?

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));

Review comment:
       Probably this is not the best way to simulate this.
   You may set Thread.currentThread().interrupt() or better,  call Thread.interrupt.
   But I am not sure it is worth to make it so complicated WDYT?




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

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



[GitHub] [pulsar] david-streamlio commented on a change in pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
david-streamlio commented on a change in pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#discussion_r619878659



##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -54,6 +55,52 @@ public void testLambda() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningFunction() throws Exception  {
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                (Function<String, String>) (input, context) -> null,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	instance.close();
+    }
+
+    @Test
+    public void testUserExceptionThrowingFunction() throws Exception  {
+    	Function<String, String> func = (input, context) -> {
+    		throw new UserException("Boom");
+    	};
+
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                func,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	assertNotNull(result.get().getUserException());

Review comment:
       I have updated the code to use assertSame




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

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#discussion_r619856505



##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -54,6 +55,52 @@ public void testLambda() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningFunction() throws Exception  {
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                (Function<String, String>) (input, context) -> null,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	instance.close();
+    }
+
+    @Test
+    public void testUserExceptionThrowingFunction() throws Exception  {
+    	Function<String, String> func = (input, context) -> {
+    		throw new UserException("Boom");
+    	};
+
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                func,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	assertNotNull(result.get().getUserException());

Review comment:
       What about using assertSame? And test about the absence or presence of wrapper?

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        // TODO Change this

Review comment:
       Please do not leave TODOs

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);

Review comment:
       Why do you need this sleep?

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));

Review comment:
       Probably this is not the best way to simulate this.
   You may set Thread.currentThread().interrupt() or better,  call Thread.interrupt.
   But I am not sure it is worth to make it so complicated WDYT?




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

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



[GitHub] [pulsar] david-streamlio commented on a change in pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
david-streamlio commented on a change in pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#discussion_r619878887



##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));

Review comment:
       The call to `completeExceptionally` is required in order for the test to complete, any other approach, such as just throwing and exception results in the test hanging.




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

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



[GitHub] [pulsar] david-streamlio commented on a change in pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
david-streamlio commented on a change in pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#discussion_r619878659



##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -54,6 +55,52 @@ public void testLambda() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningFunction() throws Exception  {
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                (Function<String, String>) (input, context) -> null,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	instance.close();
+    }
+
+    @Test
+    public void testUserExceptionThrowingFunction() throws Exception  {
+    	Function<String, String> func = (input, context) -> {
+    		throw new UserException("Boom");
+    	};
+
+    	JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                func,
+                new InstanceConfig());
+    	String testString = "ABC123";
+    	CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+    	assertNull(result.get().getResult());
+    	assertNotNull(result.get().getUserException());

Review comment:
       I have updated the code to use assertSame

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);

Review comment:
       Just being consistent with the behavior of the other Async function tests

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        // TODO Change this

Review comment:
       Removed

##########
File path: pulsar-functions/instance/src/test/java/org/apache/pulsar/functions/instance/JavaInstanceTest.java
##########
@@ -86,6 +133,92 @@ public void testAsyncFunction() throws Exception {
         assertEquals(new String(testString + "-lambda"), result.get().getResult());
         instance.close();
     }
+    
+    @Test
+    public void testNullReturningAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+                try {
+                    Thread.sleep(500);
+                    result.complete(null);
+                } catch (Exception e) {
+                    result.completeExceptionally(e);
+                }
+            });
+
+            return result;
+        };
+
+        JavaInstance instance = new JavaInstance(
+                mock(ContextImpl.class),
+                function,
+                instanceConfig);
+        String testString = "ABC123";
+        CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString);
+        assertNull(result.get().getResult());
+        instance.close();
+    }
+
+    @Test
+    public void testSystemExceptionThrowingAsyncFunction() throws Exception {
+        InstanceConfig instanceConfig = new InstanceConfig();
+        @Cleanup("shutdownNow")
+        ExecutorService executor = Executors.newCachedThreadPool();
+
+        Function<String, CompletableFuture<String>> function = (input, context) -> {
+            log.info("input string: {}", input);
+            CompletableFuture<String> result  = new CompletableFuture<>();
+            executor.submit(() -> {
+            	result.completeExceptionally(new InterruptedException(""));

Review comment:
       The call to `completeExceptionally` is required in order for the test to complete, any other approach, such as just throwing and exception results in the test hanging.




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

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



[GitHub] [pulsar] jerrypeng merged pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
jerrypeng merged pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369


   


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

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



[GitHub] [pulsar] codelipenghui commented on pull request #10369: Added more unit tests to the JavaInstanceTest class

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #10369:
URL: https://github.com/apache/pulsar/pull/10369#issuecomment-845638591


   Depends by #10618, cherry-pick this PR to branch-2.7


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

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