Merge changes I944f73e9,Ib4971b20,I9d5fdf99 into stable-2.9 * changes: tests: Use insecure random for acceptance tests Acceptance-tests: Fix 'Address already in use' bug Acceptance-tests: Fix 'verify: false' bug
diff --git a/Documentation/dev-buck.txt b/Documentation/dev-buck.txt index db36732..c6faac5 100644 --- a/Documentation/dev-buck.txt +++ b/Documentation/dev-buck.txt
@@ -469,6 +469,45 @@ EOF ---- +== Rerun unit tests + +If for some reasons tests, that were already run must be repeated, unit test +cache must be removed fist. That's because the test execution results are +cached by Buck: + +---- + $ rm -rf buck-out/bin/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/.AddRemoveGroupMembersIT/ +---- + +After clearing the cache test can be rerun again: + +---- + $ buck test //gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group:AddRemoveGroupMembersIT + TESTING //gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group:AddRemoveGroupMembersIT + PASS 14,9s 8 Passed 0 Failed com.google.gerrit.acceptance.rest.group.AddRemoveGroupMembersIT + TESTS PASSED +---- + +An alternative approach is to use a Buck feature: +--test-selectors (-filters, -f) option: + +---- + buck test --all -f 'com.google.gerrit.acceptance.rest.change.SubmitByMergeAlwaysIT' + TESTING SELECTED TESTS + PASS 14,5s 6 Passed 0 Failed com.google.gerrit.acceptance.rest.change.SubmitByMergeAlwaysIT + TESTS PASSED +---- + +When this option is used, cache is disabled per design and doesn't need to be deleted. +Note: when -f option is used, the whole unit test cache is dropped. As a consequence, +repeating the + +---- +buck test --all +---- + +would re-execute all tests again. + GERRIT ------ Part of link:index.html[Gerrit Code Review]
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java index a8ce229..cbf2685 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
@@ -36,7 +36,6 @@ import java.lang.reflect.Field; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.ServerSocket; import java.net.URI; import java.net.UnknownHostException; import java.util.concurrent.BrokenBarrierException; @@ -123,23 +122,18 @@ private static void mergeTestConfig(Config cfg) throws IOException { - InetSocketAddress http = newPort(); - InetSocketAddress sshd = newPort(); - String url = "http://" + format(http) + "/"; - + String forceEphemeralPort = String.format("%s:0", + getLocalHost().getHostName()); + String url = "http://" + forceEphemeralPort + "/"; cfg.setString("gerrit", null, "canonicalWebUrl", url); cfg.setString("httpd", null, "listenUrl", url); - cfg.setString("sshd", null, "listenAddress", format(sshd)); + cfg.setString("sshd", null, "listenAddress", forceEphemeralPort); cfg.setString("cache", null, "directory", null); cfg.setBoolean("sendemail", null, "enable", false); cfg.setInt("cache", "projects", "checkFrequency", 0); cfg.setInt("plugins", null, "checkFrequency", 0); } - private static String format(InetSocketAddress s) { - return String.format("%s:%d", s.getAddress().getHostAddress(), s.getPort()); - } - private static Injector createTestInjector(Daemon daemon) throws Exception { Injector sysInjector = get(daemon, "sysInjector"); Module module = new FactoryModule() { @@ -160,15 +154,6 @@ return (T) f.get(obj); } - private static final InetSocketAddress newPort() throws IOException { - ServerSocket s = new ServerSocket(0, 0, getLocalHost()); - try { - return (InetSocketAddress) s.getLocalSocketAddress(); - } finally { - s.close(); - } - } - private static InetAddress getLocalHost() throws UnknownHostException { return InetAddress.getLoopbackAddress(); }
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/JschVerifyFalseBugIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/JschVerifyFalseBugIT.java new file mode 100644 index 0000000..9bbc125 --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/JschVerifyFalseBugIT.java
@@ -0,0 +1,67 @@ +// Copyright (C) 2014 The Android Open Source Project +// +// Licensed 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 com.google.gerrit.acceptance.ssh; + +import static com.google.gerrit.acceptance.GitUtil.cloneProject; +import static com.google.gerrit.acceptance.GitUtil.createProject; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; + +import org.junit.Assert; +import org.junit.Ignore; +import org.junit.Test; + +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +@NoHttpd +// To see this test failing with 'verify: false', at least in the Jcsh 0.1.51 +// remove bouncycastle libs from the classpath, and run: +// buck test //gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh:JschVerifyFalseBugIT +public class JschVerifyFalseBugIT extends AbstractDaemonTest { + + @Test + @Ignore// we know it works now, so let's not clone a project 500 times ;-) + public void test() throws Exception { + test(5); + } + + private void test(int threads) throws InterruptedException, + ExecutionException { + Callable<Void> task = new Callable<Void>() { + @Override + public Void call() throws Exception { + for (int i = 1; i < 100; i++) { + String p = "p" + i; + createProject(sshSession, p); + cloneProject(sshSession.getUrl() + "/" + p); + } + return null; + } + }; + List<Callable<Void>> nCopies = Collections.nCopies(threads, task); + List<Future<Void>> futures = Executors.newFixedThreadPool(threads) + .invokeAll(nCopies); + for (Future<Void> future : futures) { + future.get(); + } + Assert.assertEquals(threads, futures.size()); + } +}
diff --git a/gerrit-acceptance-tests/tests.defs b/gerrit-acceptance-tests/tests.defs index 3c8b4d5..0131887 100644 --- a/gerrit-acceptance-tests/tests.defs +++ b/gerrit-acceptance-tests/tests.defs
@@ -1,7 +1,19 @@ +# these need as workaround for the 'verify: false' bug in Jcraft SSH library +BOUNCYCASTLE = [ + '//lib/bouncycastle:bcpkix', + '//lib/bouncycastle:bcpg', +] + def acceptance_tests( srcs, deps = [], vm_args = ['-Xmx256m']): + from os import environ, path + if not environ.get('NO_BOUNCYCASTLE'): + deps = BOUNCYCASTLE + deps + if path.exists('/dev/urandom'): + vm_args = vm_args + ['-Djava.security.egd=file:/dev/./urandom'] + for j in srcs: java_test( name = j[:-len('.java')],
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java index 6ba54c9..067eeab 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -19,6 +19,7 @@ import com.google.common.base.Charsets; import com.google.common.base.Objects; +import com.google.common.base.Strings; import com.google.common.escape.Escaper; import com.google.common.html.HtmlEscapers; import com.google.common.io.ByteStreams; @@ -103,16 +104,33 @@ static class Lifecycle implements LifecycleListener { private final JettyServer server; + private final Config cfg; @Inject - Lifecycle(final JettyServer server) { + Lifecycle(final JettyServer server, @GerritServerConfig final Config cfg) { this.server = server; + this.cfg = cfg; } @Override public void start() { try { + String origUrl = cfg.getString("httpd", null, "listenUrl"); + boolean rewrite = !Strings.isNullOrEmpty(origUrl) + && origUrl.endsWith(":0/"); server.httpd.start(); + if (rewrite) { + Connector con = server.httpd.getConnectors()[0]; + if (con instanceof ServerConnector) { + @SuppressWarnings("resource") + ServerConnector serverCon = (ServerConnector)con; + String host = serverCon.getHost(); + int port = serverCon.getLocalPort(); + String url = String.format("http://%s:%d", host, port); + cfg.setString("gerrit", null, "canonicalWebUrl", url); + cfg.setString("httpd", null, "listenUrl", url); + } + } } catch (Exception e) { throw new IllegalStateException("Cannot start HTTP daemon", e); } @@ -259,7 +277,7 @@ } else { final URI r = u.parseServerAuthority(); c.setHost(r.getHost()); - c.setPort(0 < r.getPort() ? r.getPort() : defaultPort); + c.setPort(0 <= r.getPort() ? r.getPort() : defaultPort); } } catch (URISyntaxException e) { throw new IllegalArgumentException("Invalid httpd.listenurl " + u, e);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshDaemon.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshDaemon.java index 5338c16..f507920 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshDaemon.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshDaemon.java
@@ -18,6 +18,8 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; +import com.google.common.base.Strings; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.gerrit.common.Version; import com.google.gerrit.extensions.events.LifecycleListener; @@ -51,11 +53,11 @@ import org.apache.sshd.common.cipher.AES192CBC; import org.apache.sshd.common.cipher.AES256CBC; import org.apache.sshd.common.cipher.AES256CTR; +import org.apache.sshd.common.cipher.ARCFOUR128; +import org.apache.sshd.common.cipher.ARCFOUR256; import org.apache.sshd.common.cipher.BlowfishCBC; import org.apache.sshd.common.cipher.CipherNone; import org.apache.sshd.common.cipher.TripleDESCBC; -import org.apache.sshd.common.cipher.ARCFOUR128; -import org.apache.sshd.common.cipher.ARCFOUR256; import org.apache.sshd.common.compression.CompressionNone; import org.apache.sshd.common.file.FileSystemFactory; import org.apache.sshd.common.file.FileSystemView; @@ -100,6 +102,7 @@ import java.io.File; import java.io.IOException; import java.net.InetAddress; +import java.net.InetSocketAddress; import java.net.SocketAddress; import java.net.UnknownHostException; import java.security.InvalidKeyException; @@ -144,6 +147,7 @@ private final boolean keepAlive; private final List<HostKey> hostKeys; private volatile IoAcceptor acceptor; + private final Config cfg; @Inject SshDaemon(final CommandFactory commandFactory, final NoShell noShell, @@ -155,6 +159,7 @@ @SshAdvertisedAddresses final List<String> advertised) { setPort(IANA_SSH_PORT /* never used */); + this.cfg = cfg; this.listen = listen; this.advertised = advertised; keepAlive = cfg.getBoolean("sshd", "tcpkeepalive", true); @@ -276,7 +281,14 @@ acceptor = createAcceptor(); try { + String listenAddress = cfg.getString("sshd", null, "listenAddress"); + boolean rewrite = !Strings.isNullOrEmpty(listenAddress) + && listenAddress.endsWith(":0"); acceptor.bind(listen); + if (rewrite) { + SocketAddress bound = Iterables.getOnlyElement(acceptor.getBoundAddresses()); + cfg.setString("sshd", null, "listenAddress", format((InetSocketAddress)bound)); + } } catch (IOException e) { throw new IllegalStateException("Cannot bind to " + addressList(), e); } @@ -286,6 +298,10 @@ } } + private static String format(InetSocketAddress s) { + return String.format("%s:%d", s.getAddress().getHostAddress(), s.getPort()); + } + @Override public synchronized void stop() { if (acceptor != null) {