Acceptance-tests: Fix 'verify: false' bug

Sporadic unit tests failures are caused by long withstanding bug in Jcraft
SSH library.  Despite announcements in the release notes for the versions
0.1.50 and 0.1.51 of this library, claiming that this bug was fixed, that
bug still exists and can be easily reproduced.

This change introduces an unit test, that reliably reproduces the
'verify: false' bug in one test method (that is running against
established SSH port number, so that random port assignments can be
eliminated as the reason for the sporadic unit test failures).

The workaround introduced in this change is to conditionally use
bouncycastle library. Per default bouncycastle library is used. To run unit
tests without it, set NO_BOUNCYCASTLE library to any value:

  NO_BOUNCYCASTLE=1 buck test --all

Change-Id: I9d5fdf992082b3658542d84ed9551ac2bf146414

Documentation/dev-buck.txt

@@ -480,13 +480,22 @@ heap size:
 
 == Sporadic failures of unit tests
 
-In case unit tests are failing for non obvious reasons, failed tests may need
-to be repeated:
+Due to implementation of ephemeral port assignment for HTTP and SSH daemons,
+(the reason is explained in depth in this thread:
+https://groups.google.com/forum/#!topic/repo-discuss/db14RJ59ubs),
+unit tests can randomly fail with "BindException: Address already in use".
+In this case, failed tests need to be repeated:
 
 ----
 FAIL  32,0s  7 Passed   1 Failed   com.google.gerrit.acceptance.rest.group.AddRemoveGroupMembersIT
-FAILURE includeRemoveGroup: verify: false
-com.jcraft.jsch.JSchException: verify: false
+FAILURE includeRemoveGroup: Cannot bind to localhost:40955
+        at com.google.gerrit.sshd.SshDaemon.start(SshDaemon.java:281)
+        at com.google.gerrit.lifecycle.LifecycleManager.start(LifecycleManager.java:74)
+        at com.google.gerrit.pgm.Daemon.start(Daemon.java:288)
+        at com.google.gerrit.acceptance.GerritServer.start(GerritServer.java:81)
+[...]
+Caused by: java.net.BindException: Address already in use
+        at sun.nio.ch.Net.bind0(Native Method)
 ----
 
 Because the test execution results are cached by Buck, they must be removed
@@ -516,6 +525,14 @@ An alternative approach is to use a Buck feature:
 ----
 
 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
 ------

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());
+  }
+}

gerrit-acceptance-tests/tests.defs

@@ -1,8 +1,17 @@
+# 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 = [],
     source_under_test = [],
     vm_args = ['-Xmx256m']):
+  from os import environ
+  if not environ.get('NO_BOUNCYCASTLE'):
+    deps = BOUNCYCASTLE + deps
   for j in srcs:
     java_test(
       name = j[:-len('.java')],