[GitHub] [nifi] pgyori commented on a change in pull request #4847: NIFI-8263: Maximum Thread Pool Size property introduced in ListenHTTP

[GitBox <gi...@apache.org>]

pgyori commented on a change in pull request #4847:
URL: https://github.com/apache/nifi/pull/4847#discussion_r583907815



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListenHTTP.java
##########
@@ -405,6 +409,84 @@ public void testSecureServerRejectsUnsupportedTlsProtocolVersion() throws Except
         assertThrows(SSLHandshakeException.class, sslSocket::startHandshake);
     }
 
+    @Test
+    public void testMaxThreadPoolSizeTooLow() {
+        // GIVEN, WHEN
+        runner.setProperty(ListenHTTP.PORT, Integer.toString(availablePort));
+        runner.setProperty(ListenHTTP.BASE_PATH, HTTP_BASE_PATH);
+        runner.setProperty(ListenHTTP.MAX_THREAD_POOL_SIZE, "7");
+
+        // THEN
+        runner.assertNotValid();
+    }
+
+    @Test
+    public void testMaxThreadPoolSizeTooHigh() {
+        // GIVEN, WHEN
+        runner.setProperty(ListenHTTP.PORT, Integer.toString(availablePort));
+        runner.setProperty(ListenHTTP.BASE_PATH, HTTP_BASE_PATH);
+        runner.setProperty(ListenHTTP.MAX_THREAD_POOL_SIZE, "1001");
+
+        // THEN
+        runner.assertNotValid();
+    }
+
+    @Test
+    public void testMaxThreadPoolSizeOkLowerBound() {
+        // GIVEN, WHEN
+        runner.setProperty(ListenHTTP.PORT, Integer.toString(availablePort));
+        runner.setProperty(ListenHTTP.BASE_PATH, HTTP_BASE_PATH);
+        runner.setProperty(ListenHTTP.MAX_THREAD_POOL_SIZE, "8");
+
+        // THEN
+        runner.assertValid();
+    }
+
+    @Test
+    public void testMaxThreadPoolSizeOkUpperBound() {
+        // GIVEN, WHEN
+        runner.setProperty(ListenHTTP.PORT, Integer.toString(availablePort));
+        runner.setProperty(ListenHTTP.BASE_PATH, HTTP_BASE_PATH);
+        runner.setProperty(ListenHTTP.MAX_THREAD_POOL_SIZE, "1000");
+
+        // THEN
+        runner.assertValid();
+    }
+
+    @Test
+    public void testWhenServerIsStartedCreateQueuedThreadPoolIsCalledWithMaxThreadPoolSize() {
+        // GIVEN
+        int maxThreadPoolSize = 200;
+        runner.setProperty(ListenHTTP.PORT, Integer.toString(availablePort));
+        runner.setProperty(ListenHTTP.BASE_PATH, HTTP_BASE_PATH);
+        runner.setProperty(ListenHTTP.MAX_THREAD_POOL_SIZE, Integer.toString(maxThreadPoolSize));
+
+        // WHEN
+        startWebServer();
+
+        // THEN
+        Mockito.verify(proc).createQueuedThreadPool(maxThreadPoolSize);
+    }
+
+    @Test
+    public void testWhenServerIsStartedCreateCreateServerIsCalledWithTheRightQueuedThreadPool() {
+        // GIVEN
+        int maxThreadPoolSize = 200;
+        QueuedThreadPool queuedThreadPool = new QueuedThreadPool(maxThreadPoolSize);
+
+        runner.setProperty(ListenHTTP.PORT, Integer.toString(availablePort));
+        runner.setProperty(ListenHTTP.BASE_PATH, HTTP_BASE_PATH);
+        runner.setProperty(ListenHTTP.MAX_THREAD_POOL_SIZE, Integer.toString(maxThreadPoolSize));
+
+        Mockito.when(proc.createQueuedThreadPool(maxThreadPoolSize)).thenReturn(queuedThreadPool);

Review comment:
       I don't think we need to test whether the Server(ThreadPool pool) constructor works correctly, because that should be covered in the Jetty server's unit tests. All we need to make sure is that the constructor is called with the right parameter.
   The other thing is that the server creation is called from onTrigger. I believe that what we need to test here is not that the server creator method can correctly create a server (in isolation) with the right thread pool size, but that when the processor is started, server creation is executed with the right parameter. That is why I called startWebServer() in the unit test, which calls onTrigger() in which all the server initialization happens, and I needed the Spy to make sure that the createServer() method is called with the right parameter object. This way, if someone removes my modification from the code by replacing the new:
   final Server server = createServer(threadPool);
   with the old:
   final Server server = new Server(threadPool);
   the unit test will fail.




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