From 61b835d906d938ccbea9702d2421fe8c9bdd300d Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 14 May 2019 11:24:14 +0200 Subject: [PATCH] Add ConfigSubject over JGit Configs The general pattern for check methods is Config#getFoo becomes ConfigSubject#fooValues, with the same argument list. A more fluent API using intermediate types may be possible, but the goal to start with is to keep the API close to the underlying Config APIs. Change-Id: I09e6e04a5a77fd440ff34d3d284b1d69f8440695 --- java/com/google/gerrit/acceptance/BUILD | 1 + java/com/google/gerrit/truth/BUILD | 1 + .../google/gerrit/truth/ConfigSubject.java | 129 ++++++++++++++++++ javatests/com/google/gerrit/acceptance/BUILD | 1 + .../MergeableFileBasedConfigTest.java | 3 +- .../acceptance/api/accounts/AccountIT.java | 8 +- .../rest/change/ConfigChangeIT.java | 5 +- .../acceptance/rest/project/AccessIT.java | 5 +- .../project/ProjectOperationsImplTest.java | 10 +- .../gerrit/server/config/ConfigUtilTest.java | 23 ++-- .../server/schema/AllProjectsCreatorTest.java | 5 +- 11 files changed, 166 insertions(+), 25 deletions(-) create mode 100644 java/com/google/gerrit/truth/ConfigSubject.java diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD index 0714d22dd6..ada2fb67a9 100644 --- a/java/com/google/gerrit/acceptance/BUILD +++ b/java/com/google/gerrit/acceptance/BUILD @@ -35,6 +35,7 @@ java_library( "//java/com/google/gerrit/server/restapi", "//java/com/google/gerrit/sshd", "//java/com/google/gerrit/testing:gerrit-test-util", + "//java/com/google/gerrit/truth", "//lib:args4j", "//lib:gson", "//lib:guava-retrying", diff --git a/java/com/google/gerrit/truth/BUILD b/java/com/google/gerrit/truth/BUILD index 6f958b12ca..4727da16a6 100644 --- a/java/com/google/gerrit/truth/BUILD +++ b/java/com/google/gerrit/truth/BUILD @@ -6,6 +6,7 @@ java_library( deps = [ "//java/com/google/gerrit/common:annotations", "//lib:guava", + "//lib/jgit/org.eclipse.jgit:jgit", "//lib/truth", ], ) diff --git a/java/com/google/gerrit/truth/ConfigSubject.java b/java/com/google/gerrit/truth/ConfigSubject.java new file mode 100644 index 0000000000..2a991514fd --- /dev/null +++ b/java/com/google/gerrit/truth/ConfigSubject.java @@ -0,0 +1,129 @@ +// Copyright (C) 2019 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.truth; + +import static com.google.common.truth.Truth.assertAbout; +import static java.util.Objects.requireNonNull; + +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.truth.BooleanSubject; +import com.google.common.truth.FailureMetadata; +import com.google.common.truth.IntegerSubject; +import com.google.common.truth.IterableSubject; +import com.google.common.truth.ListMultimapSubject; +import com.google.common.truth.LongSubject; +import com.google.common.truth.StringSubject; +import com.google.common.truth.Subject; +import com.google.gerrit.common.Nullable; +import java.util.Arrays; +import org.eclipse.jgit.lib.Config; + +public class ConfigSubject extends Subject { + public static ConfigSubject assertThat(Config config) { + return assertAbout(ConfigSubject::new).that(config); + } + + private final Config config; + + private ConfigSubject(FailureMetadata metadata, Config actual) { + super(metadata, actual); + this.config = actual; + } + + public IterableSubject sections() { + isNotNull(); + return check("getSections()").that(config.getSections()); + } + + public IterableSubject subsections(String section) { + requireNonNull(section); + isNotNull(); + return check("getSubsections(%s)", section).that(config.getSubsections(section)); + } + + public ListMultimapSubject sectionValues(String section) { + requireNonNull(section); + return sectionValuesImpl(section, null); + } + + public ListMultimapSubject subsectionValues(String section, String subsection) { + requireNonNull(section); + requireNonNull(subsection); + return sectionValuesImpl(section, subsection); + } + + private ListMultimapSubject sectionValuesImpl(String section, @Nullable String subsection) { + isNotNull(); + ImmutableListMultimap.Builder b = ImmutableListMultimap.builder(); + config + .getNames(section, subsection, true) + .forEach( + n -> + Arrays.stream(config.getStringList(section, subsection, n)) + .forEach(v -> b.put(n, v))); + return check("getSection(%s, %s)", section, subsection).that(b.build()); + } + + public void isEmpty() { + sections().isEmpty(); + } + + public StringSubject text() { + isNotNull(); + return check("toText()").that(config.toText()); + } + + public IterableSubject stringValues(String section, @Nullable String subsection, String name) { + requireNonNull(section); + requireNonNull(name); + isNotNull(); + return check("getStringList(%s, %s, %s)", section, subsection, name) + .that(Arrays.asList(config.getStringList(section, subsection, name))); + } + + public StringSubject stringValue(String section, @Nullable String subsection, String name) { + requireNonNull(section); + requireNonNull(name); + isNotNull(); + return check("getString(%s, %s, %s)", section, subsection, name) + .that(config.getString(section, subsection, name)); + } + + public IntegerSubject intValue( + String section, @Nullable String subsection, String name, int defaultValue) { + requireNonNull(section); + requireNonNull(name); + isNotNull(); + return check("getInt(%s, %s, %s, %s)", section, subsection, name, defaultValue) + .that(config.getInt(section, subsection, name, defaultValue)); + } + + public LongSubject longValue(String section, String subsection, String name, long defaultValue) { + requireNonNull(section); + requireNonNull(name); + isNotNull(); + return check("getLong(%s, %s, %s, %s)", section, subsection, name, defaultValue) + .that(config.getLong(section, subsection, name, defaultValue)); + } + + public BooleanSubject booleanValue( + String section, String subsection, String name, boolean defaultValue) { + requireNonNull(section); + requireNonNull(name); + isNotNull(); + return check("getBoolean(%s, %s, %s, %s)", section, subsection, name, defaultValue) + .that(config.getBoolean(section, subsection, name, defaultValue)); + } +} diff --git a/javatests/com/google/gerrit/acceptance/BUILD b/javatests/com/google/gerrit/acceptance/BUILD index 54b3626b50..405610b274 100644 --- a/javatests/com/google/gerrit/acceptance/BUILD +++ b/javatests/com/google/gerrit/acceptance/BUILD @@ -7,6 +7,7 @@ junit_tests( "//java/com/google/gerrit/acceptance:lib", "//java/com/google/gerrit/server/util/time", "//java/com/google/gerrit/testing:gerrit-test-util", + "//java/com/google/gerrit/truth", "//lib:guava", "//lib/jgit/org.eclipse.jgit:jgit", "//lib/truth", diff --git a/javatests/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java b/javatests/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java index 49c23e3652..3d17de06c9 100644 --- a/javatests/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java +++ b/javatests/com/google/gerrit/acceptance/MergeableFileBasedConfigTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; @@ -111,7 +112,7 @@ public class MergeableFileBasedConfigTest { } private void assertConfig(MergeableFileBasedConfig cfg, String expected) throws Exception { - assertThat(cfg.toText()).isEqualTo(expected); + assertThat(cfg).text().isEqualTo(expected); cfg.save(); assertThat(new String(Files.readAllBytes(cfg.getFile().toPath()), UTF_8)).isEqualTo(expected); } diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index 6e35c2d17d..f1f32cac61 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -33,6 +33,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPG import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; @@ -477,10 +478,11 @@ public class AccountIT extends AbstractDaemonTest { assertThat(tw).isNotNull(); Config cfg = new Config(); cfg.fromText(new String(or.open(tw.getObjectId(0), OBJ_BLOB).getBytes(), UTF_8)); - assertThat( - cfg.getString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_FULL_NAME)) + assertThat(cfg) + .stringValue(AccountProperties.ACCOUNT, null, AccountProperties.KEY_FULL_NAME) .isEqualTo(name); - assertThat(cfg.getString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS)) + assertThat(cfg) + .stringValue(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS) .isEqualTo(status); } else { // No account properties were set, hence an 'account.config' file was not created. diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ConfigChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ConfigChangeIT.java index 4ce3fc9a6e..a4ca7a35f0 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/ConfigChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/ConfigChangeIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; @@ -71,7 +72,7 @@ public class ConfigChangeIT extends AbstractDaemonTest { private String testUpdateProjectConfig() throws Exception { Config cfg = projectOperations.project(project).getConfig(); - assertThat(cfg.getString("project", null, "description")).isNull(); + assertThat(cfg).stringValue("project", null, "description").isNull(); String desc = "new project description"; cfg.setString("project", null, "description", desc); @@ -108,7 +109,7 @@ public class ConfigChangeIT extends AbstractDaemonTest { requestScopeOperations.setApiUser(user.id()); Config cfg = projectOperations.project(project).getConfig(); - assertThat(cfg.getString("access", null, "inheritFrom")).isAnyOf(null, allProjects.get()); + assertThat(cfg).stringValue("access", null, "inheritFrom").isAnyOf(null, allProjects.get()); cfg.setString("access", null, "inheritFrom", parent.name); PushOneCommit.Result r = createConfigChange(cfg); diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java index 8d62765123..503ebcca88 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; @@ -568,7 +569,7 @@ public class AccessIT extends AbstractDaemonTest { .file(ProjectConfig.PROJECT_CONFIG) .asString(); cfg.fromText(config); - assertThat(cfg.getString(access, refsFor, unknownPermission)).isEqualTo(registeredUsers); + assertThat(cfg).stringValue(access, refsFor, unknownPermission).isEqualTo(registeredUsers); // Make permission change through API ProjectAccessInput accessInput = newProjectAccessInput(); @@ -587,7 +588,7 @@ public class AccessIT extends AbstractDaemonTest { .file(ProjectConfig.PROJECT_CONFIG) .asString(); cfg.fromText(config); - assertThat(cfg.getString(access, refsFor, unknownPermission)).isEqualTo(registeredUsers); + assertThat(cfg).stringValue(access, refsFor, unknownPermission).isEqualTo(registeredUsers); } @Test diff --git a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java index 8d90a1f1ab..7713ab5dee 100644 --- a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java +++ b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.testsuite.project; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import static java.util.stream.Collectors.toList; import com.google.common.collect.ImmutableList; @@ -108,7 +109,7 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest { Project.NameKey key = projectOperations.newProject().create(); Config config = projectOperations.project(key).getConfig(); assertThat(config).isNotInstanceOf(StoredConfig.class); - assertThat(config.toText()).isEmpty(); + assertThat(config).text().isEmpty(); ConfigInput input = new ConfigInput(); input.description = "my fancy project"; @@ -116,7 +117,9 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest { config = projectOperations.project(key).getConfig(); assertThat(config).isNotInstanceOf(StoredConfig.class); - assertThat(config.toText()).isEqualTo("[project]\n\tdescription = my fancy project\n"); + assertThat(config).sections().containsExactly("project"); + assertThat(config).subsections("project").isEmpty(); + assertThat(config).sectionValues("project").containsExactly("description", "my fancy project"); } @Test @@ -125,9 +128,8 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest { deleteRefsMetaConfig(key); Config config = projectOperations.project(key).getConfig(); - assertThat(config).isNotNull(); assertThat(config).isNotInstanceOf(StoredConfig.class); - assertThat(config.toText()).isEmpty(); + assertThat(config).isEmpty(); } private void deleteRefsMetaConfig(Project.NameKey key) throws Exception { diff --git a/javatests/com/google/gerrit/server/config/ConfigUtilTest.java b/javatests/com/google/gerrit/server/config/ConfigUtilTest.java index 231b5845ed..865bda6e95 100644 --- a/javatests/com/google/gerrit/server/config/ConfigUtilTest.java +++ b/javatests/com/google/gerrit/server/config/ConfigUtilTest.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.config; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -90,17 +91,17 @@ public class ConfigUtilTest { Config cfg = new Config(); ConfigUtil.storeSection(cfg, SECT, SUB, in, d); - assertThat(cfg.getString(SECT, SUB, "CONSTANT")).isNull(); - assertThat(cfg.getString(SECT, SUB, "missing")).isNull(); - assertThat(cfg.getBoolean(SECT, SUB, "b", false)).isEqualTo(in.b); - assertThat(cfg.getBoolean(SECT, SUB, "bb", false)).isEqualTo(in.bb); - assertThat(cfg.getInt(SECT, SUB, "i", 0)).isEqualTo(0); - assertThat(cfg.getInt(SECT, SUB, "ii", 0)).isEqualTo(in.ii); - assertThat(cfg.getLong(SECT, SUB, "l", 0L)).isEqualTo(0L); - assertThat(cfg.getLong(SECT, SUB, "ll", 0L)).isEqualTo(in.ll); - assertThat(cfg.getString(SECT, SUB, "s")).isEqualTo(in.s); - assertThat(cfg.getString(SECT, SUB, "sd")).isNull(); - assertThat(cfg.getString(SECT, SUB, "nd")).isNull(); + assertThat(cfg).stringValue(SECT, SUB, "CONSTANT").isNull(); + assertThat(cfg).stringValue(SECT, SUB, "missing").isNull(); + assertThat(cfg).booleanValue(SECT, SUB, "b", false).isEqualTo(in.b); + assertThat(cfg).booleanValue(SECT, SUB, "bb", false).isEqualTo(in.bb); + assertThat(cfg).intValue(SECT, SUB, "i", 0).isEqualTo(0); + assertThat(cfg).intValue(SECT, SUB, "ii", 0).isEqualTo(in.ii); + assertThat(cfg).longValue(SECT, SUB, "l", 0L).isEqualTo(0L); + assertThat(cfg).longValue(SECT, SUB, "ll", 0L).isEqualTo(in.ll); + assertThat(cfg).stringValue(SECT, SUB, "s").isEqualTo(in.s); + assertThat(cfg).stringValue(SECT, SUB, "sd").isNull(); + assertThat(cfg).stringValue(SECT, SUB, "nd").isNull(); SectionInfo out = new SectionInfo(); ConfigUtil.loadSection(cfg, SECT, SUB, out, d, null); diff --git a/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java b/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java index 5f13489848..4c384e0727 100644 --- a/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java +++ b/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java @@ -21,6 +21,7 @@ import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil.getAllProjectsWithoutDefaultAcls; import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil.getDefaultAllProjectsWithAllDefaultSections; import static com.google.gerrit.server.schema.testing.AllProjectsCreatorTestUtil.readAllProjectsConfig; +import static com.google.gerrit.truth.ConfigSubject.assertThat; import com.google.common.collect.ImmutableList; import com.google.gerrit.common.data.GroupReference; @@ -126,7 +127,7 @@ public class AllProjectsCreatorTest { allProjectsCreator.create(allProjectsInput); Config config = readAllProjectsConfig(repoManager, allProjectsName); - assertThat(config.getString("project", null, "description")).isEqualTo(testDescription); + assertThat(config).stringValue("project", null, "description").isEqualTo(testDescription); } @Test @@ -142,7 +143,7 @@ public class AllProjectsCreatorTest { allProjectsCreator.create(allProjectsInput); Config config = readAllProjectsConfig(repoManager, allProjectsName); - assertThat(config.getBoolean("submit", null, "rejectEmptyCommit", false)).isTrue(); + assertThat(config).booleanValue("submit", null, "rejectEmptyCommit", false).isTrue(); } @Test