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
diff --git a/Documentation/dev-buck.txt b/Documentation/dev-buck.txt
index db36732..c2c2a2d 100644
--- a/Documentation/dev-buck.txt
+++ b/Documentation/dev-buck.txt
@@ -469,6 +469,62 @@
EOF
----
+== Sporadic failures of unit tests
+
+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: 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
+before retrying:
+
+----
+ $ rm -rf buck-out/bin/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/.AddRemoveGroupMembersIT/
+----
+
+After clearing the cache and repeating of the failed tests, they are successful:
+
+----
+ $ 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/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..1ea77d8 100644
--- a/gerrit-acceptance-tests/tests.defs
+++ b/gerrit-acceptance-tests/tests.defs
@@ -1,7 +1,16 @@
+# 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
+ if not environ.get('NO_BOUNCYCASTLE'):
+ deps = BOUNCYCASTLE + deps
for j in srcs:
java_test(
name = j[:-len('.java')],
