You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2019/06/18 14:50:09 UTC

[sling-whiteboard] 03/06: Add tests for client-api enforced timeouts

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch feature/test-improvements
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit ad73ce22eea7f67ea478fad09d0c5bc6b5a44cef
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jun 18 15:46:41 2019 +0200

    Add tests for client-api enforced timeouts
    
    This validates that the agent-set timeouts only kick in when no explicit timeouts are set from the client api.
---
 .../java/org/apache/sling/uca/impl/AgentIT.java    | 113 ++++++++++++++++++---
 1 file changed, 97 insertions(+), 16 deletions(-)

diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
index 21be70a..f42dfb4 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
@@ -34,12 +34,14 @@ import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.EnumMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
@@ -47,7 +49,8 @@ import org.apache.commons.httpclient.ConnectTimeoutException;
 import org.apache.sling.uca.impl.HttpClientLauncher.ClientType;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -60,7 +63,6 @@ import org.slf4j.LoggerFactory;
  * 
  * <p>It does so by reusing the same JVM as the one running the test. Validation is done by looking for a
  * Throwable information in the stderr and recording the exception class name and the message.</p>
- *
  */
 @ExtendWith(MisbehavingServerExtension.class)
 public class AgentIT {
@@ -82,6 +84,37 @@ public class AgentIT {
                 "Connect to repo1.maven.org:81 \\[.*\\] failed: connect timed out", "Read timed out"));
         errorDescriptors.put(OkHttp, new ErrorDescriptor(SocketTimeoutException.class, "connect timed out", "timeout"));
     }
+
+    /**
+     * Creates a matrix of all arguments to use for the read and connect timeout tests
+     * 
+     * <p>This matrix uses all client types and pairs them with each of the timeouts, for now:
+     * 
+     * <ol>
+     *   <li>default timeouts, which mean the agent-set default timeout will kick in</li>
+     *   <li>lower client API timeouts, which mean that the client-enforced timeouts will be applied</li>
+     * </ol>
+     * 
+     * </p>
+     * 
+     * @return a list of arguments to use for the tests
+     */
+    static List<Arguments> argumentsMatrix() {
+        
+        List<Arguments> args = new ArrayList<Arguments>();
+        
+        TestTimeouts clientLower = new TestTimeouts.Builder()
+            .agentTimeouts(Duration.ofMinutes(1), Duration.ofMinutes(1))
+            .clientTimeouts(Duration.ofSeconds(CONNECT_TIMEOUT_SECONDS), Duration.ofSeconds(READ_TIMEOUT_SECONDS))
+            .build();
+    
+        for ( ClientType client : ClientType.values() )
+            for ( TestTimeouts timeout : new TestTimeouts[] { TestTimeouts.DEFAULT, clientLower } )
+                args.add(Arguments.of(client, timeout));
+        
+        return args;
+    }
+
     
     /**
      * Validates that connecting to a unaccessible port on an existing port fails with a connect 
@@ -95,11 +128,12 @@ public class AgentIT {
      * @throws IOException various I/O problems 
      */
     @ParameterizedTest
-    @EnumSource(HttpClientLauncher.ClientType.class)
-    public void connectTimeout(ClientType clientType) throws IOException {
+    @MethodSource("argumentsMatrix")
+    public void connectTimeout(ClientType clientType, TestTimeouts timeouts) throws IOException {
 
         ErrorDescriptor ed =  requireNonNull(errorDescriptors.get(clientType), "Unhandled clientType " + clientType);
-        RecordedThrowable error = assertTimeout(ofSeconds(EXECUTION_TIMEOUT_SECONDS),  () -> runTest("http://repo1.maven.org:81", clientType));
+        RecordedThrowable error = assertTimeout(ofSeconds(EXECUTION_TIMEOUT_SECONDS),  
+            () -> runTest("http://repo1.maven.org:81", clientType, timeouts));
         
         assertEquals(ed.connectTimeoutClass.getName(), error.className);
         assertTrue(error.message.matches(ed.connectTimeoutMessageRegex), 
@@ -112,20 +146,21 @@ public class AgentIT {
      * @throws IOException various I/O problems
      */
     @ParameterizedTest
-    @EnumSource(HttpClientLauncher.ClientType.class)
-    public void readTimeout(ClientType clientType, MisbehavingServerControl server) throws IOException {
+    @MethodSource("argumentsMatrix")
+    public void readTimeout(ClientType clientType, TestTimeouts timeouts, MisbehavingServerControl server) throws IOException {
         
         ErrorDescriptor ed =  requireNonNull(errorDescriptors.get(clientType), "Unhandled clientType " + clientType);
-        RecordedThrowable error = assertTimeout(ofSeconds(EXECUTION_TIMEOUT_SECONDS),  () -> runTest("http://localhost:" + server.getLocalPort(), clientType));
+        RecordedThrowable error = assertTimeout(ofSeconds(EXECUTION_TIMEOUT_SECONDS),
+           () -> runTest("http://localhost:" + server.getLocalPort(), clientType, timeouts));
         
         assertEquals(SocketTimeoutException.class.getName(), error.className);
         assertEquals(ed.readTimeoutMessage, error.message);
     }
 
-    private RecordedThrowable runTest(String urlSpec, ClientType clientType) throws IOException, InterruptedException {
+    private RecordedThrowable runTest(String urlSpec, ClientType clientType, TestTimeouts timeouts) throws IOException, InterruptedException {
 
-        Process process = runForkedCommandWithAgent(new URL(urlSpec), CONNECT_TIMEOUT_SECONDS, READ_TIMEOUT_SECONDS, clientType);
-        boolean done = process.waitFor(EXECUTION_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+        Process process = runForkedCommandWithAgent(new URL(urlSpec), timeouts, clientType);
+        boolean done = process.waitFor(timeouts.executionTimeout.toMillis(), TimeUnit.MILLISECONDS);
         
         LOG.info("Dump of stdout: ");
         Files
@@ -139,7 +174,7 @@ public class AgentIT {
 
         if ( !done ) {
             process.destroy();
-            throw new IllegalStateException("Terminated process since it did not complete within " + EXECUTION_TIMEOUT_SECONDS + " seconds");
+            throw new IllegalStateException("Terminated process since it did not complete within " + timeouts.executionTimeout.toMillis() + " milliseconds");
         }
         int exitCode = process.exitValue();
         LOG.info("Exited with code {}", exitCode);
@@ -155,7 +190,7 @@ public class AgentIT {
         }
     }
     
-    private Process runForkedCommandWithAgent(URL url, int connectTimeoutSeconds, int readTimeoutSeconds, ClientType clientType) throws IOException {
+    private Process runForkedCommandWithAgent(URL url, TestTimeouts timeouts, ClientType clientType) throws IOException {
         
         Path jar = Files.list(Paths.get("target"))
             .filter( p -> p.getFileName().toString().endsWith("-jar-with-dependencies.jar"))
@@ -169,12 +204,14 @@ public class AgentIT {
         ProcessBuilder pb = new ProcessBuilder(
             javaExe.toString(),
             "-showversion",
-            "-javaagent:" + jar +"=" + TimeUnit.SECONDS.toMillis(connectTimeoutSeconds) +"," + TimeUnit.SECONDS.toMillis(readTimeoutSeconds)+",v",
+            "-javaagent:" + jar +"=" + timeouts.agentConnectTimeout.toMillis() +"," + timeouts.agentReadTimeout.toMillis()+",v",
             "-cp",
             classPath,
             HttpClientLauncher.class.getName(),
             url.toString(),
-            clientType.toString()
+            clientType.toString(),
+            String.valueOf(timeouts.clientConnectTimeout.toMillis()),
+            String.valueOf(timeouts.clientReadTimeout.toMillis())
         );
         
         pb.redirectInput(Redirect.INHERIT);
@@ -217,7 +254,7 @@ public class AgentIT {
 
         return new RecordedThrowable(className, message);
     }
-
+    
     /**
      * Data class for defining specific error messages related to individual {@link ClientType client types}. 
      */
@@ -246,4 +283,48 @@ public class AgentIT {
             this.message = message;
         }
     }
+    
+    /**
+     * Data class for holding information about various timeouts set in the tests
+     */
+    static class TestTimeouts {
+        
+        private Duration executionTimeout = Duration.ofSeconds(EXECUTION_TIMEOUT_SECONDS);
+        private Duration agentConnectTimeout = Duration.ofSeconds(CONNECT_TIMEOUT_SECONDS);
+        private Duration agentReadTimeout = Duration.ofSeconds(READ_TIMEOUT_SECONDS);
+        private Duration clientConnectTimeout = Duration.ZERO;
+        private Duration clientReadTimeout = Duration.ZERO;
+        
+        public static TestTimeouts DEFAULT = new TestTimeouts();
+        
+        static class Builder {
+            private TestTimeouts timeouts = new TestTimeouts();
+            
+            public Builder executionTimeout(Duration duration) {
+                timeouts.executionTimeout = Objects.requireNonNull(duration);
+                return this;
+            }
+
+            public Builder agentTimeouts(Duration connectTimeout, Duration readTimeout) {
+                timeouts.agentConnectTimeout = Objects.requireNonNull(connectTimeout);
+                timeouts.agentReadTimeout = Objects.requireNonNull(readTimeout);
+                return this;
+            }
+            
+            public Builder clientTimeouts(Duration connectTimeout, Duration readTimeout) {
+                timeouts.clientConnectTimeout = Objects.requireNonNull(connectTimeout);
+                timeouts.clientReadTimeout = Objects.requireNonNull(readTimeout);
+                return this;
+            }
+            
+            public TestTimeouts build() {
+                return timeouts;
+            }
+        }
+        
+        @Override
+        public String toString() {
+            return getClass().getSimpleName() + ": execution " + executionTimeout + ", agent: " + agentConnectTimeout + "/" + agentReadTimeout + ", client : " + clientConnectTimeout + "/" + clientReadTimeout;
+        }
+    }
 }