Merge branch 'stable-2.15'

* stable-2.15:
  Update git submodules
  [GitwebServlet] Use logger built-in formatting
  [GitwebServlet] Avoid empty error flooding the logs
  MoveChangeIT: Test moving a change to a branch where label does not exist
  GerritPersonIdentProvider: Sanitize user.name and user.email values
  OutgoingEmail: Reduce visibility of methods
  OutgoingEmail: Remove unused public methods
  OutgoingEmail: Annotate methods with @Nullable
  OutgoingEmail#getUserNameEmailFor: Protect against null accountId
  OutgoingEmail#getNameEmailFor: Protect against null accountId
  OutgoingEmail#getName{Email}For: Use helpers from Account
  AbstractIndexTests: Add --wide option to show-queue command invocation
  AbstractIndexTests: Add assertions on change index count
  AbstractIndexTests: Use correct command to index project
  acceptance/SshSession: Add helper methods to assert about success/failure
  dev-bazel: Add 'elastic' and 'docker' to list of test groups
  dev-bazel: Improve section about running Elasticsearch tests
  AbstractIndexTests: Add coverage for index projects command
  ReadOnlyChangeIndex: Reduce visibility to package
  AbstractIndexTests: Refactor to simplify and use Java stream API
  Elasticsearch: Improve introduction in elasticsearch section
  Elasticsearch: Add support for version 6.3.0
  ElasticVersionTest: Add explicit test for 5.6.10
  ElasticContainer: Update to version 5.6.10

Due to incompatibility with methods in the Account class, the following
changes done on stable-2.14 are reverted in this merge. A different
solution to the potential null account Ids will need to be done for the
master branch:

  I383efdcd7 - OutgoingEmail: Annotate methods with @Nullable
  Ibb6994a5b - OutgoingEmail#getUserNameEmailFor: Protect against null accountId
  I8145ca79d - OutgoingEmail#getNameEmailFor: Protect against null accountId
  I9d55ec3f1 - OutgoingEmail#getName{Email}For: Use helpers from Account

This reverts commit 4fbf7ffb18.
This reverts commit aac761785b.
This reverts commit d75352f042.
This reverts commit ddf27f44c8.

Change-Id: I467d26843e44d02394f2b4cc86d0306ba836368b

Documentation/config-gerrit.txt

@@ -2979,14 +2979,15 @@ Sample Lucene index configuration:
 [[elasticsearch]]
 === Section elasticsearch
 
-WARNING: The Elasticsearch support has only been tested with Elasticsearch
+WARNING: Support for Elasticsearch is still experimental and is not recommended
-versions 2.4, 5.6 and 6.2. Support for other versions is not guaranteed.
+for production use. Support has been tested with Elasticsearch versions 2.4, 5.6,
+6.2 and 6.3. Support for other versions is not guaranteed.
 
-Open and closed changes are indexed in a single index, separated into types
+When using Elasticsearch versions 2.4 and 5.6, the open and closed changes are
-`open_changes` and `closed_changes` respectively, if using Elasticsearch
+indexed in a single index, separated into types `open_changes` and `closed_changes`
-versions 2.4 or 5.6. Open and closed changes are merged into the default `_doc`
+respectively. When using version 6.2 or later, the open and closed changes are
-type otherwise. The latter is also used for accounts and groups indices starting
+merged into the default `_doc` type. The latter is also used for the accounts and
-with Elasticsearch 6.2.
+groups indices starting with Elasticsearch 6.2.
 
 [[elasticsearch.prefix]]elasticsearch.prefix::
 +

Documentation/dev-bazel.txt

@@ -266,7 +266,9 @@ The following values are currently supported for the group name:
 
 * annotation
 * api
+* docker
 * edit
+* elastic
 * git
 * notedb
 * pgm
@@ -277,11 +279,13 @@ The following values are currently supported for the group name:
 [[elasticsearch]]
 === Elasticsearch
 
-Successfully running the elasticsearch tests may require setting the local
+Successfully running the Elasticsearch tests requires Docker, and
+may require setting the local
 link:https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html[virtual memory].
 
-Bazel link:https://github.com/bazelbuild/bazel/issues/3476[does not currently make container failures visible],
+If Docker is not available, the Elasticsearch tests will be skipped.
-if any.
+Note that Bazel currently does not show
+link:https://github.com/bazelbuild/bazel/issues/3476[the skipped tests].
 
 == Dependencies
 

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -1572,16 +1572,38 @@ public abstract class AbstractDaemonTest {
   }
 
   protected void configLabel(String label, LabelFunction func) throws Exception {
+    configLabel(label, func, ImmutableList.of());
+  }
+
+  protected void configLabel(String label, LabelFunction func, List<String> refPatterns)
+      throws Exception {
     configLabel(
-        project, label, func, value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
+        project,
+        label,
+        func,
+        refPatterns,
+        value(1, "Passes"),
+        value(0, "No score"),
+        value(-1, "Failed"));
   }
 
   protected void configLabel(
       Project.NameKey project, String label, LabelFunction func, LabelValue... value)
       throws Exception {
+    configLabel(project, label, func, ImmutableList.of(), value);
+  }
+
+  private void configLabel(
+      Project.NameKey project,
+      String label,
+      LabelFunction func,
+      List<String> refPatterns,
+      LabelValue... value)
+      throws Exception {
     try (ProjectConfigUpdate u = updateProject(project)) {
       LabelType labelType = category(label, value);
       labelType.setFunction(func);
+      labelType.setRefPatterns(refPatterns);
       u.getConfig().getLabelSections().put(labelType.getName(), labelType);
       u.save();
     }

java/com/google/gerrit/acceptance/ChangeIndexedCounter.java

@@ -0,0 +1,53 @@
+// Copyright (C) 2018 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;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.util.concurrent.AtomicLongMap;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.events.ChangeIndexedListener;
+
+public class ChangeIndexedCounter implements ChangeIndexedListener {
+  private final AtomicLongMap<Integer> countsByChange = AtomicLongMap.create();
+
+  @Override
+  public void onChangeIndexed(String projectName, int id) {
+    countsByChange.incrementAndGet(id);
+  }
+
+  @Override
+  public void onChangeDeleted(int id) {
+    countsByChange.incrementAndGet(id);
+  }
+
+  public void clear() {
+    countsByChange.clear();
+  }
+
+  long getCount(ChangeInfo info) {
+    return countsByChange.get(info._number);
+  }
+
+  public void assertReindexOf(ChangeInfo info) {
+    assertReindexOf(info, 1);
+  }
+
+  public void assertReindexOf(ChangeInfo info, int expectedCount) {
+    assertThat(getCount(info)).isEqualTo(expectedCount);
+    assertThat(countsByChange).hasSize(1);
+    clear();
+  }
+}

java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java

@@ -24,14 +24,14 @@ import com.google.gerrit.server.index.change.ChangeIndex;
 import com.google.gerrit.server.query.change.ChangeData;
 import java.io.IOException;
 
-public class ReadOnlyChangeIndex implements ChangeIndex {
+class ReadOnlyChangeIndex implements ChangeIndex {
   private final ChangeIndex index;
 
-  public ReadOnlyChangeIndex(ChangeIndex index) {
+  ReadOnlyChangeIndex(ChangeIndex index) {
     this.index = index;
   }
 
-  public ChangeIndex unwrap() {
+  ChangeIndex unwrap() {
     return index;
   }
 

java/com/google/gerrit/acceptance/SshSession.java

@@ -15,6 +15,8 @@
 package com.google.gerrit.acceptance;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
 import com.jcraft.jsch.ChannelExec;
@@ -75,7 +77,7 @@ public class SshSession {
     return exec(command, null);
   }
 
-  public boolean hasError() {
+  private boolean hasError() {
     return error != null;
   }
 
@@ -83,6 +85,19 @@ public class SshSession {
     return error;
   }
 
+  public void assertSuccess() {
+    assertWithMessage(getError()).that(hasError()).isFalse();
+  }
+
+  public void assertFailure() {
+    assertThat(hasError()).isTrue();
+  }
+
+  public void assertFailure(String error) {
+    assertThat(hasError()).isTrue();
+    assertThat(getError()).contains(error);
+  }
+
   public void close() {
     if (session != null) {
       session.disconnect();

java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java

@@ -32,13 +32,13 @@ public class ElasticQueryAdapter {
 
   ElasticQueryAdapter(ElasticVersion version) {
     this.ignoreUnmapped = version == ElasticVersion.V2_4;
-    this.usePostV5Type = version == ElasticVersion.V6_2;
+    this.usePostV5Type = isV6(version);
-
+    this.versionDiscoveryUrl = isV6(version) ? "%s*" : "%s*/_aliases";
-    this.versionDiscoveryUrl = version == ElasticVersion.V6_2 ? "%s*" : "%s*/_aliases";
 
     switch (version) {
       case V5_6:
       case V6_2:
+      case V6_3:
         this.searchFilteringName = "_source";
         this.indicesExistParam = "?allow_no_indices=false";
         this.exactFieldType = "keyword";
@@ -58,6 +58,10 @@ public class ElasticQueryAdapter {
     }
   }
 
+  private boolean isV6(ElasticVersion version) {
+    return version == ElasticVersion.V6_2 || version == ElasticVersion.V6_3;
+  }
+
   void setIgnoreUnmapped(JsonObject properties) {
     if (ignoreUnmapped) {
       properties.addProperty("ignore_unmapped", true);

java/com/google/gerrit/elasticsearch/ElasticVersion.java

@@ -20,7 +20,8 @@ import java.util.regex.Pattern;
 public enum ElasticVersion {
   V2_4("2.4.*"),
   V5_6("5.6.*"),
-  V6_2("6.2.*");
+  V6_2("6.2.*"),
+  V6_3("6.3.*");
 
   private final String version;
   private final Pattern pattern;

java/com/google/gerrit/httpd/gitweb/GitwebServlet.java

@@ -80,6 +80,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -668,17 +669,17 @@ class GitwebServlet extends HttpServlet {
   private void copyStderrToLog(InputStream in) {
     new Thread(
             () -> {
-              StringBuilder b = new StringBuilder();
               try (BufferedReader br =
                   new BufferedReader(new InputStreamReader(in, ISO_8859_1.name()))) {
-                String line;
+                String err =
-                while ((line = br.readLine()) != null) {
+                    br.lines()
-                  if (b.length() > 0) {
+                        .filter(s -> !s.isEmpty())
-                    b.append('\n');
+                        .map(s -> "CGI: " + s)
-                  }
+                        .collect(Collectors.joining("\n"))
-                  b.append("CGI: ").append(line);
+                        .trim();
+                if (!err.isEmpty()) {
+                  logger.atSevere().log(err);
                 }
-                logger.atSevere().log(b.toString());
               } catch (IOException e) {
                 logger.atSevere().withCause(e).log("Unexpected error copying stderr from CGI");
               }

java/com/google/gerrit/server/GerritPersonIdentProvider.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server;
 
+import static com.google.common.base.MoreObjects.firstNonNull;
+
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -30,12 +32,14 @@ public class GerritPersonIdentProvider implements Provider<PersonIdent> {
 
   @Inject
   public GerritPersonIdentProvider(@GerritServerConfig Config cfg) {
-    String name = cfg.getString("user", null, "name");
+    StringBuilder name = new StringBuilder();
-    if (name == null) {
+    PersonIdent.appendSanitized(
-      name = "Gerrit Code Review";
+        name, firstNonNull(cfg.getString("user", null, "name"), "Gerrit Code Review"));
-    }
+    this.name = name.toString();
-    this.name = name;
+
-    email = cfg.get(UserConfig.KEY).getCommitterEmail();
+    StringBuilder email = new StringBuilder();
+    PersonIdent.appendSanitized(email, cfg.get(UserConfig.KEY).getCommitterEmail());
+    this.email = email.toString();
   }
 
   @Override

java/com/google/gerrit/server/mail/send/OutgoingEmail.java

@@ -356,7 +356,7 @@ public abstract class OutgoingEmail {
    * @param accountId user to fetch.
    * @return name/email of account, or Anonymous Coward if unset.
    */
-  public String getNameEmailFor(Account.Id accountId) {
+  protected String getNameEmailFor(Account.Id accountId) {
     Optional<Account> account = args.accountCache.get(accountId).map(AccountState::getAccount);
     if (account.isPresent()) {
       String name = account.get().getFullName();
@@ -379,7 +379,7 @@ public abstract class OutgoingEmail {
    * @param accountId user to fetch.
    * @return name/email of account, username, or null if unset.
    */
-  public String getUserNameEmailFor(Account.Id accountId) {
+  protected String getUserNameEmailFor(Account.Id accountId) {
     Optional<AccountState> accountState = args.accountCache.get(accountId);
     if (!accountState.isPresent()) {
       return null;
@@ -564,28 +564,6 @@ public abstract class OutgoingEmail {
     return soyTemplate(name, SanitizedContent.ContentKind.HTML);
   }
 
-  public String joinStrings(Iterable<Object> in, String joiner) {
-    return joinStrings(in.iterator(), joiner);
-  }
-
-  public String joinStrings(Iterator<Object> in, String joiner) {
-    if (!in.hasNext()) {
-      return "";
-    }
-
-    Object first = in.next();
-    if (!in.hasNext()) {
-      return safeToString(first);
-    }
-
-    StringBuilder r = new StringBuilder();
-    r.append(safeToString(first));
-    while (in.hasNext()) {
-      r.append(joiner).append(safeToString(in.next()));
-    }
-    return r.toString();
-  }
-
   protected void removeUser(Account user) {
     String fromEmail = user.getPreferredEmail();
     for (Iterator<Address> j = smtpRcptTo.iterator(); j.hasNext(); ) {
@@ -601,10 +579,6 @@ public abstract class OutgoingEmail {
     }
   }
 
-  private static String safeToString(Object obj) {
-    return obj != null ? obj.toString() : "";
-  }
-
   protected final boolean useHtml() {
     return args.settings.html && supportsHtml();
   }

javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java

@@ -35,6 +35,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.project.testing.Util;
+import java.util.Arrays;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -292,6 +293,35 @@ public class MoveChangeIT extends AbstractDaemonTest {
         .containsExactly((short) -2, (short) -1, (short) 0, (short) 0);
   }
 
+  @Test
+  public void moveToBranchWithoutLabel() throws Exception {
+    createBranch(new Branch.NameKey(project, "foo"));
+    String testLabelA = "Label-A";
+    configLabel(testLabelA, LabelFunction.MAX_WITH_BLOCK, Arrays.asList("refs/heads/master"));
+
+    AccountGroup.UUID registered = SystemGroupBackend.REGISTERED_USERS;
+    try (ProjectConfigUpdate u = updateProject(project)) {
+      Util.allow(
+          u.getConfig(), Permission.forLabel(testLabelA), -1, +1, registered, "refs/heads/master");
+      u.save();
+    }
+
+    String changeId = createChange().getChangeId();
+
+    ReviewInput input = new ReviewInput();
+    input.label(testLabelA, -1);
+    gApi.changes().id(changeId).current().review(input);
+
+    assertThat(gApi.changes().id(changeId).current().reviewer(admin.email).votes().keySet())
+        .containsExactly(testLabelA);
+    assertThat(gApi.changes().id(changeId).current().reviewer(admin.email).votes().values())
+        .containsExactly((short) -1);
+
+    move(changeId, "foo");
+
+    // TODO(dpursehouse): Assert about state of labels after move
+  }
+
   private void move(int changeNum, String destination) throws RestApiException {
     gApi.changes().id(changeNum).move(destination);
   }

javatests/com/google/gerrit/acceptance/ssh/AbandonRestoreIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
 
 import com.google.common.collect.ImmutableList;
@@ -63,7 +62,7 @@ public class AbandonRestoreIT extends AbstractDaemonTest {
       command.append(" --message ").append(message);
     }
     String response = adminSshSession.exec(command.toString());
-    assertWithMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isFalse();
+    adminSshSession.assertSuccess();
     assertThat(response.toLowerCase(Locale.US)).doesNotContain("error");
   }
 

javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java

@@ -15,32 +15,60 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.Joiner;
-import com.google.common.collect.FluentIterable;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ChangeIndexedCounter;
+import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.events.ChangeIndexedListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.inject.Inject;
 import com.google.inject.Injector;
 import java.util.List;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 
 @NoHttpd
 @UseSsh
 public abstract class AbstractIndexTests extends AbstractDaemonTest {
+  @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
+
+  private ChangeIndexedCounter changeIndexedCounter;
+  private RegistrationHandle changeIndexedCounterHandle;
+
   /** @param injector injector */
   public abstract void configureIndex(Injector injector) throws Exception;
 
+  @Before
+  public void addChangeIndexedCounter() {
+    changeIndexedCounter = new ChangeIndexedCounter();
+    changeIndexedCounterHandle = changeIndexedListeners.add(changeIndexedCounter);
+  }
+
+  @After
+  public void removeChangeIndexedCounter() {
+    if (changeIndexedCounterHandle != null) {
+      changeIndexedCounterHandle.remove();
+    }
+  }
+
   @Test
+  @GerritConfig(name = "index.autoReindexIfStale", value = "false")
   public void indexChange() throws Exception {
     configureIndex(server.getTestInjector());
 
     PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
     String changeId = change.getChangeId();
     String changeLegacyId = change.getChange().getId().toString();
+    ChangeInfo changeInfo = gApi.changes().id(changeId).get();
 
     disableChangeIndexWrites();
     amendChange(changeId, "second test", "test2.txt", "test2");
@@ -48,24 +76,55 @@ public abstract class AbstractIndexTests extends AbstractDaemonTest {
     assertChangeQuery("message:second", change.getChange(), false);
     enableChangeIndexWrites();
 
+    changeIndexedCounter.clear();
     String cmd = Joiner.on(" ").join("gerrit", "index", "changes", changeLegacyId);
     adminSshSession.exec(cmd);
+    adminSshSession.assertSuccess();
+
+    changeIndexedCounter.assertReindexOf(changeInfo, 1);
 
     assertChangeQuery("message:second", change.getChange(), true);
   }
 
-  protected void assertChangeQuery(String q, ChangeData change, Boolean assertTrue)
+  @Test
+  @GerritConfig(name = "index.autoReindexIfStale", value = "false")
+  public void indexProject() throws Exception {
+    configureIndex(server.getTestInjector());
+
+    PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
+    String changeId = change.getChangeId();
+    ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+
+    disableChangeIndexWrites();
+    amendChange(changeId, "second test", "test2.txt", "test2");
+
+    assertChangeQuery("message:second", change.getChange(), false);
+    enableChangeIndexWrites();
+
+    changeIndexedCounter.clear();
+    String cmd = Joiner.on(" ").join("gerrit", "index", "project", project.get());
+    adminSshSession.exec(cmd);
+    adminSshSession.assertSuccess();
+
+    boolean indexing = true;
+    while (indexing) {
+      String out = adminSshSession.exec("gerrit show-queue --wide");
+      adminSshSession.assertSuccess();
+      indexing = out.contains("Index all changes of project " + project.get());
+    }
+
+    changeIndexedCounter.assertReindexOf(changeInfo, 1);
+
+    assertChangeQuery("message:second", change.getChange(), true);
+  }
+
+  protected void assertChangeQuery(String q, ChangeData change, boolean assertTrue)
       throws Exception {
-    List<ChangeInfo> result = query(q);
+    List<Integer> ids = query(q).stream().map(c -> c._number).collect(toList());
-    Iterable<Integer> ids = ids(result);
     if (assertTrue) {
       assertThat(ids).contains(change.getId().get());
     } else {
       assertThat(ids).doesNotContain(change.getId().get());
     }
   }
-
-  protected static Iterable<Integer> ids(Iterable<ChangeInfo> changes) {
-    return FluentIterable.from(changes).transform(in -> in._number);
-  }
 }

javatests/com/google/gerrit/acceptance/ssh/BanCommitIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.gerrit.acceptance.GitUtil.pushHead;
 import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON;
 
@@ -36,7 +35,7 @@ public class BanCommitIT extends AbstractDaemonTest {
     RevCommit c = commitBuilder().add("a.txt", "some content").create();
 
     String response = adminSshSession.exec("gerrit ban-commit " + project.get() + " " + c.name());
-    assertWithMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isFalse();
+    adminSshSession.assertSuccess();
     assertThat(response.toLowerCase(Locale.US)).doesNotContain("error");
 
     RemoteRefUpdate u =

javatests/com/google/gerrit/acceptance/ssh/CreateProjectIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.UseSsh;
@@ -33,7 +32,7 @@ public class CreateProjectIT extends AbstractDaemonTest {
     String newProjectName = "newProject";
     adminSshSession.exec(
         "gerrit create-project --branch master --owner " + newGroupName + " " + newProjectName);
-    assertWithMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isFalse();
+    adminSshSession.assertSuccess();
     ProjectState projectState = projectCache.get(new Project.NameKey(newProjectName));
     assertThat(projectState).isNotNull();
   }
@@ -46,7 +45,7 @@ public class CreateProjectIT extends AbstractDaemonTest {
     String newProjectName = "newProject";
     adminSshSession.exec(
         "gerrit create-project --branch master --owner " + wrongGroupName + " " + newProjectName);
-    assertWithMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isTrue();
+    adminSshSession.assertFailure();
     ProjectState projectState = projectCache.get(new Project.NameKey(newProjectName));
     assertThat(projectState).isNull();
   }

javatests/com/google/gerrit/acceptance/ssh/ElasticIndexIT.java

@@ -47,10 +47,15 @@ public class ElasticIndexIT extends AbstractIndexTests {
   }
 
   @ConfigSuite.Config
-  public static Config elasticsearchV6() {
+  public static Config elasticsearchV6_2() {
     return getConfig(ElasticVersion.V6_2);
   }
 
+  @ConfigSuite.Config
+  public static Config elasticsearchV6_3() {
+    return getConfig(ElasticVersion.V6_3);
+  }
+
   @Override
   public void configureIndex(Injector injector) throws Exception {
     ElasticTestUtils.createAllIndexes(injector);

javatests/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GcAssert;
@@ -56,7 +55,7 @@ public class GarbageCollectionIT extends AbstractDaemonTest {
   public void testGc() throws Exception {
     String response =
         adminSshSession.exec("gerrit gc \"" + project.get() + "\" \"" + project2.get() + "\"");
-    assertWithMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isFalse();
+    adminSshSession.assertSuccess();
     assertNoError(response);
     gcAssert.assertHasPackFile(project, project2);
     gcAssert.assertHasNoPackFile(allProjects, project3);
@@ -66,7 +65,7 @@ public class GarbageCollectionIT extends AbstractDaemonTest {
   @UseLocalDisk
   public void testGcAll() throws Exception {
     String response = adminSshSession.exec("gerrit gc --all");
-    assertWithMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isFalse();
+    adminSshSession.assertSuccess();
     assertNoError(response);
     gcAssert.assertHasPackFile(allProjects, project, project2, project3);
   }
@@ -74,7 +73,7 @@ public class GarbageCollectionIT extends AbstractDaemonTest {
   @Test
   public void gcWithoutCapability_Error() throws Exception {
     userSshSession.exec("gerrit gc --all");
-    assertThat(userSshSession.hasError()).isTrue();
+    userSshSession.assertFailure();
     String error = userSshSession.getError();
     assertThat(error).isNotNull();
     assertError("maintain server not permitted", error);

javatests/com/google/gerrit/acceptance/ssh/QueryIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.common.collect.Lists;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -149,8 +148,7 @@ public class QueryIT extends AbstractDaemonTest {
   public void shouldFailWithFilesWithoutPatchSetsOrCurrentPatchSetsOption() throws Exception {
     String changeId = createChange().getChangeId();
     adminSshSession.exec("gerrit query --files " + changeId);
-    assertThat(adminSshSession.hasError()).isTrue();
+    adminSshSession.assertFailure("needs --patch-sets or --current-patch-set");
-    assertThat(adminSshSession.getError()).contains("needs --patch-sets or --current-patch-set");
   }
 
   @Test
@@ -305,7 +303,7 @@ public class QueryIT extends AbstractDaemonTest {
   private List<ChangeAttribute> executeSuccessfulQuery(String params, SshSession session)
       throws Exception {
     String rawResponse = session.exec("gerrit query --format=JSON " + params);
-    assertWithMessage(session.getError()).that(session.hasError()).isFalse();
+    session.assertSuccess();
     return getChanges(rawResponse);
   }
 

javatests/com/google/gerrit/acceptance/ssh/SetReviewersIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -52,7 +51,7 @@ public class SetReviewersIT extends AbstractDaemonTest {
   private void setReviewer(boolean add, String id) throws Exception {
     adminSshSession.exec(
         String.format("gerrit set-reviewers -%s %s %s", add ? "a" : "r", user.email, id));
-    assert_().withMessage(adminSshSession.getError()).that(adminSshSession.hasError()).isFalse();
+    adminSshSession.assertSuccess();
     ImmutableSet<Id> reviewers = change.getChange().getReviewers().all();
     if (add) {
       assertThat(reviewers).contains(user.id);

javatests/com/google/gerrit/elasticsearch/ElasticContainer.java

@@ -45,9 +45,11 @@ public class ElasticContainer<SELF extends ElasticContainer<SELF>> extends Gener
       case V2_4:
         return "elasticsearch:2.4.6-alpine";
       case V5_6:
-        return "elasticsearch:5.6.9-alpine";
+        return "elasticsearch:5.6.10-alpine";
       case V6_2:
         return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.2.4";
+      case V6_3:
+        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.0";
     }
     throw new IllegalStateException("No tests for version: " + version.name());
   }

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java

@@ -41,7 +41,7 @@ public class ElasticV6QueryAccountsTest extends AbstractQueryAccountsTest {
       return;
     }
 
-    container = ElasticContainer.createAndStart(ElasticVersion.V6_2);
+    container = ElasticContainer.createAndStart(ElasticVersion.V6_3);
     nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
   }
 

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java

@@ -41,7 +41,7 @@ public class ElasticV6QueryChangesTest extends AbstractQueryChangesTest {
       return;
     }
 
-    container = ElasticContainer.createAndStart(ElasticVersion.V6_2);
+    container = ElasticContainer.createAndStart(ElasticVersion.V6_3);
     nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
   }
 

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java

@@ -41,7 +41,7 @@ public class ElasticV6QueryGroupsTest extends AbstractQueryGroupsTest {
       return;
     }
 
-    container = ElasticContainer.createAndStart(ElasticVersion.V6_2);
+    container = ElasticContainer.createAndStart(ElasticVersion.V6_3);
     nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
   }
 

javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java

@@ -30,9 +30,13 @@ public class ElasticVersionTest {
 
     assertThat(ElasticVersion.forVersion("5.6.0")).isEqualTo(ElasticVersion.V5_6);
     assertThat(ElasticVersion.forVersion("5.6.9")).isEqualTo(ElasticVersion.V5_6);
+    assertThat(ElasticVersion.forVersion("5.6.10")).isEqualTo(ElasticVersion.V5_6);
 
     assertThat(ElasticVersion.forVersion("6.2.0")).isEqualTo(ElasticVersion.V6_2);
     assertThat(ElasticVersion.forVersion("6.2.4")).isEqualTo(ElasticVersion.V6_2);
+
+    assertThat(ElasticVersion.forVersion("6.3.0")).isEqualTo(ElasticVersion.V6_3);
+    assertThat(ElasticVersion.forVersion("6.3.1")).isEqualTo(ElasticVersion.V6_3);
   }
 
   @Test