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 8736107e9f9b4bd03d6757955154adc244d1ecd4 Author: Robert Munteanu <[email protected]> AuthorDate: Tue Jun 18 16:06:57 2019 +0200 Move classes and methods out of AgentIT to make it simpler to understand --- .../java/org/apache/sling/uca/impl/AgentIT.java | 156 +-------------------- .../org/apache/sling/uca/impl/AgentLauncher.java | 107 ++++++++++++++ .../org/apache/sling/uca/impl/ErrorDescriptor.java | 37 +++++ .../apache/sling/uca/impl/HttpClientLauncher.java | 2 +- .../apache/sling/uca/impl/RecordedThrowable.java | 40 ++++++ .../org/apache/sling/uca/impl/TestTimeouts.java | 64 +++++++++ 6 files changed, 255 insertions(+), 151 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 f42dfb4..17638d7 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 @@ -26,9 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTimeout; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.File; import java.io.IOException; -import java.lang.ProcessBuilder.Redirect; import java.net.SocketTimeoutException; import java.net.URL; import java.nio.file.Files; @@ -36,13 +34,9 @@ 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; import org.apache.commons.httpclient.ConnectTimeoutException; @@ -67,11 +61,11 @@ import org.slf4j.LoggerFactory; @ExtendWith(MisbehavingServerExtension.class) public class AgentIT { - private static final int EXECUTION_TIMEOUT_SECONDS = 5; - private static final int CONNECT_TIMEOUT_SECONDS = 3; - private static final int READ_TIMEOUT_SECONDS = 3; + static final int EXECUTION_TIMEOUT_SECONDS = 5; + static final int CONNECT_TIMEOUT_SECONDS = 3; + static final int READ_TIMEOUT_SECONDS = 3; - private static final String EXCEPTION_MARKER = "Exception in thread \"main\" "; + static final String EXCEPTION_MARKER = "Exception in thread \"main\" "; private static final Path STDERR = Paths.get("target", "stderr.txt"); private static final Path STDOUT = Paths.get("target", "stdout.txt"); private static final Logger LOG = LoggerFactory.getLogger(AgentIT.class); @@ -159,7 +153,7 @@ public class AgentIT { private RecordedThrowable runTest(String urlSpec, ClientType clientType, TestTimeouts timeouts) throws IOException, InterruptedException { - Process process = runForkedCommandWithAgent(new URL(urlSpec), timeouts, clientType); + Process process = new AgentLauncher(new URL(urlSpec), timeouts, clientType, STDOUT, STDERR).launch(); boolean done = process.waitFor(timeouts.executionTimeout.toMillis(), TimeUnit.MILLISECONDS); LOG.info("Dump of stdout: "); @@ -184,147 +178,9 @@ public class AgentIT { } else { return Files.lines(STDERR) .filter( l -> l.startsWith(EXCEPTION_MARKER)) - .map( l -> newRecordedThrowable(l) ) + .map( RecordedThrowable::fromLine ) .findFirst() .orElseThrow(() -> new RuntimeException("Exit code was not zero ( " + exitCode + " ) but did not find any exception information in stderr.txt")); } } - - 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")) - .findFirst() - .orElseThrow( () -> new IllegalStateException("Did not find the agent jar. Did you run mvn package first?")); - - String classPath = buildClassPath(); - - String javaHome = System.getProperty("java.home"); - Path javaExe = Paths.get(javaHome, "bin", "java"); - ProcessBuilder pb = new ProcessBuilder( - javaExe.toString(), - "-showversion", - "-javaagent:" + jar +"=" + timeouts.agentConnectTimeout.toMillis() +"," + timeouts.agentReadTimeout.toMillis()+",v", - "-cp", - classPath, - HttpClientLauncher.class.getName(), - url.toString(), - clientType.toString(), - String.valueOf(timeouts.clientConnectTimeout.toMillis()), - String.valueOf(timeouts.clientReadTimeout.toMillis()) - ); - - pb.redirectInput(Redirect.INHERIT); - pb.redirectOutput(STDOUT.toFile()); - pb.redirectError(STDERR.toFile()); - - return pb.start(); - } - - private String buildClassPath() throws IOException { - - List<String> elements = new ArrayList<>(); - elements.add(Paths.get("target", "test-classes").toString()); - - Set<String> dependencies = new HashSet<>(Arrays.asList(new String[] { - "commons-httpclient.jar", - "commons-codec.jar", - "slf4j-simple.jar", - "slf4j-api.jar", - "jcl-over-slf4j.jar", - "httpclient.jar", - "httpcore.jar", - "okhttp.jar", - "okio.jar" - })); - - Files.list(Paths.get("target", "it-dependencies")) - .filter( p -> dependencies.contains(p.getFileName().toString()) ) - .forEach( p -> elements.add(p.toString())); - - return String.join(File.pathSeparator, elements); - } - - private RecordedThrowable newRecordedThrowable(String line) { - - line = line.replace(EXCEPTION_MARKER, ""); - - String className = line.substring(0, line.indexOf(':')); - String message = line.substring(line.indexOf(':') + 2); // ignore ':' and leading ' ' - - return new RecordedThrowable(className, message); - } - - /** - * Data class for defining specific error messages related to individual {@link ClientType client types}. - */ - static class ErrorDescriptor { - private Class<? extends IOException> connectTimeoutClass; - private String connectTimeoutMessageRegex; - private String readTimeoutMessage; - - public ErrorDescriptor(Class<? extends IOException> connectTimeoutClass, String connectTimeoutMessageRegex, - String readTimeoutMessage) { - this.connectTimeoutClass = connectTimeoutClass; - this.connectTimeoutMessageRegex = connectTimeoutMessageRegex; - this.readTimeoutMessage = readTimeoutMessage; - } - } - - /** - * Basic information about a {@link Throwable} that was recorded in a file - */ - static class RecordedThrowable { - String className; - String message; - - public RecordedThrowable(String className, String message) { - this.className = className; - 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; - } - } } diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentLauncher.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentLauncher.java new file mode 100644 index 0000000..93a006b --- /dev/null +++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentLauncher.java @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.uca.impl; + +import java.io.File; +import java.io.IOException; +import java.lang.ProcessBuilder.Redirect; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.apache.sling.uca.impl.HttpClientLauncher.ClientType; + +/** + * Launches the {@link HttpClientLauncher} as a separate process with the timeout agent enabled + * + */ +class AgentLauncher { + private final URL url; + private final TestTimeouts timeouts; + private final ClientType clientType; + private Path stdout; + private Path stderr; + + public AgentLauncher(URL url, TestTimeouts timeouts, ClientType clientType, Path stdout, Path stderr) { + this.url = url; + this.timeouts = timeouts; + this.clientType = clientType; + this.stdout = stdout; + this.stderr = stderr; + } + + public Process launch() throws IOException { + + Path jar = Files.list(Paths.get("target")) + .filter( p -> p.getFileName().toString().endsWith("-jar-with-dependencies.jar")) + .findFirst() + .orElseThrow( () -> new IllegalStateException("Did not find the agent jar. Did you run mvn package first?")); + + String classPath = buildClassPath(); + + String javaHome = System.getProperty("java.home"); + Path javaExe = Paths.get(javaHome, "bin", "java"); + ProcessBuilder pb = new ProcessBuilder( + javaExe.toString(), + "-showversion", + "-javaagent:" + jar +"=" + timeouts.agentConnectTimeout.toMillis() +"," + timeouts.agentReadTimeout.toMillis()+",v", + "-cp", + classPath, + HttpClientLauncher.class.getName(), + url.toString(), + clientType.toString(), + String.valueOf(timeouts.clientConnectTimeout.toMillis()), + String.valueOf(timeouts.clientReadTimeout.toMillis()) + ); + + pb.redirectInput(Redirect.INHERIT); + pb.redirectOutput(stdout.toFile()); + pb.redirectError(stderr.toFile()); + + return pb.start(); + } + + private String buildClassPath() throws IOException { + + List<String> elements = new ArrayList<>(); + elements.add(Paths.get("target", "test-classes").toString()); + + Set<String> dependencies = new HashSet<>(Arrays.asList(new String[] { + "commons-httpclient.jar", + "commons-codec.jar", + "slf4j-simple.jar", + "slf4j-api.jar", + "jcl-over-slf4j.jar", + "httpclient.jar", + "httpcore.jar", + "okhttp.jar", + "okio.jar" + })); + + Files.list(Paths.get("target", "it-dependencies")) + .filter( p -> dependencies.contains(p.getFileName().toString()) ) + .forEach( p -> elements.add(p.toString())); + + return String.join(File.pathSeparator, elements); + } +} \ No newline at end of file diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/ErrorDescriptor.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/ErrorDescriptor.java new file mode 100644 index 0000000..840ea05 --- /dev/null +++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/ErrorDescriptor.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.uca.impl; + +import java.io.IOException; + +import org.apache.sling.uca.impl.HttpClientLauncher.ClientType; + +/** + * Data class for defining specific error messages related to individual {@link ClientType client types}. + */ +class ErrorDescriptor { + Class<? extends IOException> connectTimeoutClass; + String connectTimeoutMessageRegex; + String readTimeoutMessage; + + public ErrorDescriptor(Class<? extends IOException> connectTimeoutClass, String connectTimeoutMessageRegex, + String readTimeoutMessage) { + this.connectTimeoutClass = connectTimeoutClass; + this.connectTimeoutMessageRegex = connectTimeoutMessageRegex; + this.readTimeoutMessage = readTimeoutMessage; + } +} \ No newline at end of file diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java index f24b5b5..d2408de 100644 --- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java +++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/HttpClientLauncher.java @@ -82,7 +82,7 @@ public class HttpClientLauncher { } /** - * A <tt>Consumer</tt> that allows throwing checked exceptions.</p> + * Thin wrapper for various http client abstractions * */ @FunctionalInterface diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/RecordedThrowable.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/RecordedThrowable.java new file mode 100644 index 0000000..63ba806 --- /dev/null +++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/RecordedThrowable.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.uca.impl; + +/** + * Basic information about a {@link Throwable} that was recorded in a file + */ +class RecordedThrowable { + + static RecordedThrowable fromLine(String line) { + line = line.replace(AgentIT.EXCEPTION_MARKER, ""); + + String className = line.substring(0, line.indexOf(':')); + String message = line.substring(line.indexOf(':') + 2); // ignore ':' and leading ' ' + + return new RecordedThrowable(className, message); + } + + String className; + String message; + + public RecordedThrowable(String className, String message) { + this.className = className; + this.message = message; + } +} \ No newline at end of file diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/TestTimeouts.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/TestTimeouts.java new file mode 100644 index 0000000..0d5cb36 --- /dev/null +++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/TestTimeouts.java @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.uca.impl; + +import java.time.Duration; +import java.util.Objects; + +/** + * Data class for holding information about various timeouts set in the tests + */ +class TestTimeouts { + + Duration executionTimeout = Duration.ofSeconds(AgentIT.EXECUTION_TIMEOUT_SECONDS); + Duration agentConnectTimeout = Duration.ofSeconds(AgentIT.CONNECT_TIMEOUT_SECONDS); + Duration agentReadTimeout = Duration.ofSeconds(AgentIT.READ_TIMEOUT_SECONDS); + Duration clientConnectTimeout = Duration.ZERO; + Duration clientReadTimeout = Duration.ZERO; + + public static TestTimeouts DEFAULT = new TestTimeouts(); + + static class Builder { + private TestTimeouts timeouts = new TestTimeouts(); + + public TestTimeouts.Builder executionTimeout(Duration duration) { + timeouts.executionTimeout = Objects.requireNonNull(duration); + return this; + } + + public TestTimeouts.Builder agentTimeouts(Duration connectTimeout, Duration readTimeout) { + timeouts.agentConnectTimeout = Objects.requireNonNull(connectTimeout); + timeouts.agentReadTimeout = Objects.requireNonNull(readTimeout); + return this; + } + + public TestTimeouts.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; + } +} \ No newline at end of file
