Split out commit message extraction to make things better testable
RevCommit has final fields, and RevWalk etc is a pain to mock out. So we
split that part out to make writing tests easier.
CommitMessageFetcher and its test come from the its-base plugin.
Change-Id: I5a24ae94a5e6418bba5e04464719e27ac3b19368
diff --git a/BUILD b/BUILD
index 4fd31b4..5368f19 100644
--- a/BUILD
+++ b/BUILD
@@ -1,4 +1,11 @@
-load("//tools/bzl:plugin.bzl", "gerrit_plugin")
+load("@rules_java//java:defs.bzl", "java_library")
+load("//tools/bzl:junit.bzl", "junit_tests")
+load(
+ "//tools/bzl:plugin.bzl",
+ "PLUGIN_DEPS",
+ "PLUGIN_TEST_DEPS",
+ "gerrit_plugin",
+)
gerrit_plugin(
name = "zuul",
@@ -9,3 +16,23 @@
"Gerrit-Module: com.googlesource.gerrit.plugins.zuul.Module",
],
)
+
+junit_tests(
+ name = "zuul_tests",
+ testonly = 1,
+ srcs = glob(["src/test/java/**/*.java"]),
+ tags = ["zuul"],
+ deps = [
+ ":zuul__plugin_test_deps",
+ ],
+)
+
+java_library(
+ name = "zuul__plugin_test_deps",
+ testonly = 1,
+ visibility = ["//visibility:public"],
+ exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":zuul__plugin",
+ ],
+)
+
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcher.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcher.java
new file mode 100644
index 0000000..961cad8
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcher.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.zuul;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.inject.Inject;
+import java.io.IOException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+public class CommitMessageFetcher {
+ private final GitRepositoryManager repoManager;
+
+ @Inject
+ CommitMessageFetcher(GitRepositoryManager repoManager) {
+ this.repoManager = repoManager;
+ }
+
+ public String fetch(Project.NameKey p, String rev)
+ throws RepositoryNotFoundException, IOException {
+ try (Repository repo = repoManager.openRepository(p);
+ RevWalk rw = new RevWalk(repo)) {
+ RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
+ return commit.getFullMessage();
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
index 55a7123..6e8b1a7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
@@ -24,7 +24,6 @@
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.restapi.change.ChangesCollection;
import com.google.gerrit.server.restapi.change.QueryChanges;
@@ -36,22 +35,18 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
@Singleton
public class GetCrd implements RestReadView<RevisionResource> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ChangesCollection changes;
- private final GitRepositoryManager repoManager;
+ private final CommitMessageFetcher commitMessageFetcher;
@Inject
- GetCrd(ChangesCollection changes, GitRepositoryManager repoManager) {
+ GetCrd(ChangesCollection changes, CommitMessageFetcher commitMessageFetcher) {
this.changes = changes;
- this.repoManager = repoManager;
+ this.commitMessageFetcher = commitMessageFetcher;
}
@Override
@@ -67,18 +62,14 @@
// get depends on info
Project.NameKey p = rsrc.getChange().getProject();
- try (Repository repo = repoManager.openRepository(p);
- RevWalk rw = new RevWalk(repo)) {
- String rev = rsrc.getPatchSet().commitId().getName();
- RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
- String commitMsg = commit.getFullMessage();
- Pattern pattern = Pattern.compile("[Dd]epends-[Oo]n:? (I[0-9a-f]{8,40})", Pattern.DOTALL);
- Matcher matcher = pattern.matcher(commitMsg);
- while (matcher.find()) {
- String otherId = matcher.group(1);
- logger.atFinest().log("Change %s depends on change %s", thisId, otherId);
- out.dependsOn.add(otherId);
- }
+ String rev = rsrc.getPatchSet().commitId().getName();
+ String commitMsg = commitMessageFetcher.fetch(p, rev);
+ Pattern pattern = Pattern.compile("[Dd]epends-[Oo]n:? (I[0-9a-f]{8,40})", Pattern.DOTALL);
+ Matcher matcher = pattern.matcher(commitMsg);
+ while (matcher.find()) {
+ String otherId = matcher.group(1);
+ logger.atFinest().log("Change %s depends on change %s", thisId, otherId);
+ out.dependsOn.add(otherId);
}
// get needed by info
diff --git a/src/test/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcherTest.java b/src/test/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcherTest.java
new file mode 100644
index 0000000..edda09e
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcherTest.java
@@ -0,0 +1,123 @@
+// Copyright (C) 2020 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.googlesource.gerrit.plugins.zuul;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import java.io.IOException;
+import java.math.BigInteger;
+import java.util.HashSet;
+import java.util.Set;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CommitMessageFetcherTest {
+ private GitRepositoryManager repoManager;
+ private Repository repo;
+ private String objectIdBlob = "24c5735c3e8ce8fd18d312e9e58149a62236c01a";
+ private String objectIdTree = "3faaefce19558dfc8d9c976f09ae4897f45cb242";
+ private String objectIdCommit = "95aed53c03b6d3df0912bdd9bb1d0c6eaf619f58";
+ private String objectIdMissing = "0123456789012345678901234567890123456789";
+
+ private byte rawBlob[] = "def\n".getBytes();
+ private byte rawTree[] = sha1append("100644 abc\000", objectIdBlob);
+ private byte rawCommit[] =
+ ("tree\0003faaefce19558dfc8d9c976f09ae4897f45cb242\n"
+ + "author Author <author@example.org> 1592579853 +0200\n"
+ + "committer Committer <committer@example.org> 1592579853 +0200\n"
+ + "\n"
+ + "CommitMsg\n")
+ .getBytes();
+
+ private static byte[] sha1append(String left, String sha1sum) {
+ int leftLen = left.length();
+ byte[] right = (new BigInteger(sha1sum, 16)).toByteArray();
+ byte[] ret = new byte[leftLen + right.length];
+ System.arraycopy(left.getBytes(), 0, ret, 0, leftLen);
+ System.arraycopy(right, 0, ret, leftLen, right.length);
+ return ret;
+ }
+
+ @Test
+ public void testFetchBlob() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ assertThrows(
+ IOException.class, () -> fetcher.fetch(Project.nameKey("ProjectFoo"), objectIdBlob));
+ }
+
+ @Test
+ public void testFetchTree() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ assertThrows(
+ IOException.class, () -> fetcher.fetch(Project.nameKey("ProjectFoo"), objectIdTree));
+ }
+
+ @Test
+ public void testFetchCommit() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch(Project.nameKey("ProjectFoo"), objectIdCommit);
+
+ assertThat(commitMessage).isEqualTo("CommitMsg\n");
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ ObjectLoader objectLoaderBlob = mock(ObjectLoader.class);
+ when(objectLoaderBlob.getCachedBytes(anyInt())).thenReturn(rawBlob);
+ when(objectLoaderBlob.getType()).thenReturn(Constants.OBJ_BLOB);
+
+ ObjectLoader objectLoaderTree = mock(ObjectLoader.class);
+ when(objectLoaderTree.getCachedBytes(anyInt())).thenReturn(rawTree);
+ when(objectLoaderTree.getType()).thenReturn(Constants.OBJ_TREE);
+
+ ObjectLoader objectLoaderCommit = mock(ObjectLoader.class);
+ when(objectLoaderCommit.getCachedBytes(anyInt())).thenReturn(rawCommit);
+ when(objectLoaderCommit.getType()).thenReturn(Constants.OBJ_COMMIT);
+
+ Set<ObjectId> shallowCommits = new HashSet<>();
+ shallowCommits.add(ObjectId.fromString(objectIdCommit));
+
+ ObjectReader objectReader = mock(ObjectReader.class);
+ when(objectReader.getShallowCommits()).thenReturn(shallowCommits);
+ when(objectReader.open(ObjectId.fromString(objectIdBlob))).thenReturn(objectLoaderBlob);
+ when(objectReader.open(ObjectId.fromString(objectIdTree))).thenReturn(objectLoaderTree);
+ when(objectReader.open(ObjectId.fromString(objectIdCommit))).thenReturn(objectLoaderCommit);
+ when(objectReader.open(ObjectId.fromString(objectIdMissing)))
+ .thenThrow(
+ new MissingObjectException(ObjectId.fromString(objectIdMissing), Constants.OBJ_COMMIT));
+
+ repo = mock(Repository.class);
+ when(repo.newObjectReader()).thenReturn(objectReader);
+
+ repoManager = mock(GitRepositoryManager.class);
+ when(repoManager.openRepository(eq(Project.nameKey("ProjectFoo")))).thenReturn(repo);
+ }
+
+ private CommitMessageFetcher createCommitMessageFetcher() {
+ return new CommitMessageFetcher(repoManager);
+ }
+}
