Merge branch 'stable-2.14' into stable-2.15

* stable-2.14:
  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

Change-Id: I6b1c93d4dd7b3dbe6e17e6c6e323fc2c7458a39e
This commit is contained in:
David Pursehouse 2018-06-20 08:44:03 +09:00
commit a09a747e06
20 changed files with 189 additions and 48 deletions

View File

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

View File

@ -261,7 +261,9 @@ The following values are currently supported for the group name:
* annotation
* api
* docker
* edit
* elastic
* git
* notedb
* pgm
@ -272,11 +274,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 any.
If Docker is not available, the Elasticsearch tests will be skipped.
Note that Bazel currently does not show
link:https://github.com/bazelbuild/bazel/issues/3476[the skipped tests].
== Dependencies

View File

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

View File

@ -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;
}

View File

@ -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.jcraft.jsch.ChannelExec;
import com.jcraft.jsch.JSch;
@ -73,7 +75,7 @@ public class SshSession {
return exec(command, null);
}
public boolean hasError() {
private boolean hasError() {
return error != null;
}
@ -81,6 +83,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();

View File

@ -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.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@ -62,7 +61,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");
}

View File

@ -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);
Iterable<Integer> ids = ids(result);
List<Integer> ids = query(q).stream().map(c -> c._number).collect(toList());
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);
}
}

View File

@ -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 =

View File

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

View File

@ -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);

View File

@ -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);

View File

@ -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();
assertThat(adminSshSession.getError()).contains("needs --patch-sets or --current-patch-set");
adminSshSession.assertFailure("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);
}

View File

@ -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);

View File

@ -31,13 +31,13 @@ public class ElasticQueryAdapter {
ElasticQueryAdapter(ElasticVersion version) {
this.ignoreUnmapped = version == ElasticVersion.V2_4;
this.usePostV5Type = version == ElasticVersion.V6_2;
this.versionDiscoveryUrl = version == ElasticVersion.V6_2 ? "%s*" : "%s*/_aliases";
this.usePostV5Type = isV6(version);
this.versionDiscoveryUrl = isV6(version) ? "%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";
@ -55,6 +55,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);

View File

@ -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;

View File

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

View File

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

View File

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

View File

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

View File

@ -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