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 18:32:05 UTC

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

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