Merge "When adding issue occurrences, mark them additionally as 'added@'"
diff --git a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
index 017fbb9..57f51da 100644
--- a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
+++ b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractor.java
@@ -7,7 +7,10 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.hooks.its.ItsName;
@@ -25,13 +28,16 @@
private final Config gerritConfig;
private final String itsName;
private final CommitMessageFetcher commitMessageFetcher;
+ private final ReviewDb db;
@Inject
IssueExtractor(@GerritServerConfig Config gerritConfig,
- @ItsName String itsName, CommitMessageFetcher commitMessageFetcher) {
+ @ItsName String itsName, CommitMessageFetcher commitMessageFetcher,
+ ReviewDb db) {
this.gerritConfig = gerritConfig;
this.itsName = itsName;
this.commitMessageFetcher = commitMessageFetcher;
+ this.db = db;
}
/**
@@ -157,4 +163,62 @@
}
return ret;
}
+
+ /**
+ * Gets issues for a commit with new issue occurrences marked as "added".
+ * <p>
+ * Fetches the patch set's immediate ancestor and compares issue occurrences
+ * between them. Any new occurrence gets marked as "added." So if for
+ * example in patch sets 1, and 2 issue 23 occurs in the subject, while in
+ * patch set the issue occurs in the body, then patch set 2 has occurrences
+ * "somewhere", and "subject" for issue 23. Patch set 3 has occurrences
+ * "somewhere", "body", and "body-added" for issue 23.
+ *
+ * @param projectName The project to fetch {@code commitId} from.
+ * @param commitId The commit id to fetch issues for.
+ * @param patchSetId The patch set for the {@code commitId}. If it is null,
+ * no occurrence can be marked as "-added".
+ * @return A mapping, whose keys are issue ids and whose values is a set of
+ * places where the issue occurs. Each issue occurs at least in
+ * "somewhere". Issues from the first line get tagged with an occurrence
+ * "subject". Issues in the last block get tagged with "footer". Issues
+ * occurring between "subject" and "footer" get tagged with "body".
+ */
+ public Map<String,Set<String>> getIssueIds(String projectName,
+ String commitId, PatchSet.Id patchSetId) {
+ Map<String,Set<String>> current = getIssueIds(projectName, commitId);
+ if (patchSetId != null) {
+ Map<String,Set<String>> previous = Maps.newHashMap();
+ if (patchSetId.get() != 1) {
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(
+ patchSetId.getParentKey(), patchSetId.get() - 1);
+ try {
+ PatchSet previousPatchSet = db.patchSets().get(previousPatchSetId);
+ if (previousPatchSet != null) {
+ previous = getIssueIds(projectName,
+ previousPatchSet.getRevision().get());
+ }
+ } catch (OrmException e) {
+ // previous is still empty to indicate that there was no previous
+ // accessible patch set. We treat every occurrence as added.
+ }
+ }
+
+ for (String issue : current.keySet()) {
+ Set<String> currentOccurrences = current.get(issue);
+ Set<String> previousOccurrences = previous.get(issue);
+ Set<String> newOccurrences;
+ if (previousOccurrences == null || previousOccurrences.isEmpty()) {
+ newOccurrences = Sets.newHashSet(currentOccurrences);
+ } else {
+ newOccurrences = Sets.newHashSet(currentOccurrences);
+ newOccurrences.removeAll(previousOccurrences);
+ }
+ for (String occurrence : newOccurrences) {
+ currentOccurrences.add( "added@" + occurrence);
+ }
+ }
+ }
+ return current;
+ }
}
diff --git a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java
index 895ec33..e6f9968 100644
--- a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java
+++ b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractor.java
@@ -18,6 +18,8 @@
import java.util.Set;
import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.events.ChangeAbandonedEvent;
import com.google.gerrit.server.events.ChangeEvent;
import com.google.gerrit.server.events.ChangeMergedEvent;
@@ -47,6 +49,22 @@
this.propertyAttributeExtractor = propertyAttributeExtractor;
}
+ /**
+ * creates a patch id for change id string and patchset id string.
+ * @param changeId String representation of the patch sets {@code Change.Id@}
+ * @param patchId String representation of the patch sets {@code Patchset.Id@}
+ * @return PatchSet.Id for the specified patch set. If the String to int
+ * conversion fails for any of the parameters, null is returned.
+ */
+ private PatchSet.Id newPatchSetId(String changeId, String patchId) {
+ try {
+ return new PatchSet.Id(new Change.Id(Integer.parseInt(changeId)),
+ Integer.parseInt(patchId));
+ } catch (NumberFormatException e) {
+ return null;
+ }
+ }
+
private Map<String,Set<String>> extractFrom(ChangeAbandonedEvent event,
Set<Property> common) {
common.add(propertyFactory.create("event-type", event.type));
@@ -54,8 +72,10 @@
common.addAll(propertyAttributeExtractor.extractFrom(event.abandoner, "abandoner"));
common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
common.add(propertyFactory.create("reason", event.reason));
+ PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+ event.patchSet.number);
return issueExtractor.getIssueIds(event.change.project,
- event.patchSet.revision);
+ event.patchSet.revision, patchSetId);
}
private Map<String,Set<String>> extractFrom(ChangeMergedEvent event,
@@ -64,8 +84,10 @@
common.addAll(propertyAttributeExtractor.extractFrom(event.change));
common.addAll(propertyAttributeExtractor.extractFrom(event.submitter, "submitter"));
common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
+ PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+ event.patchSet.number);
return issueExtractor.getIssueIds(event.change.project,
- event.patchSet.revision);
+ event.patchSet.revision, patchSetId);
}
private Map<String,Set<String>> extractFrom(ChangeRestoredEvent event,
@@ -75,8 +97,10 @@
common.addAll(propertyAttributeExtractor.extractFrom(event.restorer, "restorer"));
common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
common.add(propertyFactory.create("reason", event.reason));
+ PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+ event.patchSet.number);
return issueExtractor.getIssueIds(event.change.project,
- event.patchSet.revision);
+ event.patchSet.revision, patchSetId);
}
private Map<String,Set<String>> extractFrom(RefUpdatedEvent event,
@@ -94,8 +118,10 @@
common.addAll(propertyAttributeExtractor.extractFrom(event.change));
common.addAll(propertyAttributeExtractor.extractFrom(event.patchSet));
common.addAll(propertyAttributeExtractor.extractFrom(event.uploader, "uploader"));
+ PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+ event.patchSet.number);
return issueExtractor.getIssueIds(event.change.project,
- event.patchSet.revision);
+ event.patchSet.revision, patchSetId);
}
private Map<String,Set<String>> extractFrom(CommentAddedEvent event,
@@ -106,8 +132,10 @@
common.addAll(propertyAttributeExtractor.extractFrom(event.author, "commenter"));
common.add(propertyFactory.create("comment", event.comment));
//TODO approvals
+ PatchSet.Id patchSetId = newPatchSetId(event.change.number,
+ event.patchSet.number);
return issueExtractor.getIssueIds(event.change.project,
- event.patchSet.revision);
+ event.patchSet.revision, patchSetId);
}
/**
diff --git a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
index 323205f..53d69db 100644
--- a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
+++ b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/IssueExtractorTest.java
@@ -21,20 +21,32 @@
import java.util.Set;
import org.eclipse.jgit.lib.Config;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.server.PatchSetAccess;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.googlesource.gerrit.plugins.hooks.its.ItsName;
import com.googlesource.gerrit.plugins.hooks.testutil.LoggingMockingTestCase;
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({PatchSet.class,RevId.class})
public class IssueExtractorTest extends LoggingMockingTestCase {
private Injector injector;
private Config serverConfig;
private CommitMessageFetcher commitMessageFetcher;
+ private ReviewDb db;
public void testPatternNullMatch() {
IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
@@ -637,6 +649,431 @@
assertLogMessageContains("Matching");
}
+ public void testIssueIdsCommitWAddedEmptyFirst() {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn("");
+
+ replayMocks();
+
+ PatchSet.Id patchSetId = new PatchSet.Id(new Change.Id(4), 1);
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", patchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedSingleSubjectIssueFirst() {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(1).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("42", Sets.newHashSet("somewhere", "subject",
+ "added@somewhere", "added@subject"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondEmpty()
+ throws OrmException {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ // Call for current patch set
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ // Call for previous patch set
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+ RevId previousRevId = createMock(RevId.class);
+ expect(previousRevId.get())
+ .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+ PatchSet previousPatchSet = createMock(PatchSet.class);
+ expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+ PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+ expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "9876543211987654321298765432139876543214")).andReturn(
+ "subject\n" +
+ "\n" +
+ "Change-Id: I9876543211987654321298765432139876543214");
+
+ expect(db.patchSets()).andReturn(patchSetAccess);
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("42", Sets.newHashSet("somewhere", "subject",
+ "added@somewhere", "added@subject"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondSame()
+ throws OrmException {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ // Call for current patch set
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ // Call for previous patch set
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+ RevId previousRevId = createMock(RevId.class);
+ expect(previousRevId.get())
+ .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+ PatchSet previousPatchSet = createMock(PatchSet.class);
+ expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+ PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+ expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "9876543211987654321298765432139876543214")).andReturn(
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I9876543211987654321298765432139876543214");
+
+ expect(db.patchSets()).andReturn(patchSetAccess);
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("42", Sets.newHashSet("somewhere", "subject"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondBody()
+ throws OrmException {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ // Call for current patch set
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ // Call for previous patch set
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+ RevId previousRevId = createMock(RevId.class);
+ expect(previousRevId.get())
+ .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+ PatchSet previousPatchSet = createMock(PatchSet.class);
+ expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+ PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+ expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "9876543211987654321298765432139876543214")).andReturn(
+ "subject\n" +
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I9876543211987654321298765432139876543214");
+
+ expect(db.patchSets()).andReturn(patchSetAccess);
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedSingleSubjectIssueSecondFooter()
+ throws OrmException {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ // Call for current patch set
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "subject\n" +
+ "\n" +
+ "Bug: bug#42\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ // Call for previous patch set
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+ RevId previousRevId = createMock(RevId.class);
+ expect(previousRevId.get())
+ .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+ PatchSet previousPatchSet = createMock(PatchSet.class);
+ expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+ PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+ expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "9876543211987654321298765432139876543214")).andReturn(
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I9876543211987654321298765432139876543214");
+
+ expect(db.patchSets()).andReturn(patchSetAccess);
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("42", Sets.newHashSet("somewhere", "footer", "added@footer"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedSubjectFooter()
+ throws OrmException {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ // Call for current patch set
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "subject bug#42\n" +
+ "\n" +
+ "body bug#42\n" +
+ "\n" +
+ "Bug: bug#42\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ // Call for previous patch set
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+ RevId previousRevId = createMock(RevId.class);
+ expect(previousRevId.get())
+ .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+ PatchSet previousPatchSet = createMock(PatchSet.class);
+ expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+ PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+ expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "9876543211987654321298765432139876543214")).andReturn(
+ "subject\n" +
+ "bug#42\n" +
+ "\n" +
+ "Change-Id: I9876543211987654321298765432139876543214");
+
+ expect(db.patchSets()).andReturn(patchSetAccess);
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject",
+ "body", "footer", "added@footer"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
+ public void testIssueIdsCommitWAddedMultiple()
+ throws OrmException {
+ expect(serverConfig.getString("commentLink", "ItsTestName", "match"))
+ .andReturn("bug#(\\d+)").atLeastOnce();
+
+ Change.Id changeId = createMock(Change.Id.class);
+
+ // Call for current patch set
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "1234567891123456789212345678931234567894")).andReturn(
+ "subject bug#42\n" +
+ "\n" +
+ "body bug#42 bug#16\n" +
+ "\n" +
+ "Bug: bug#42\n" +
+ "Change-Id: I1234567891123456789212345678931234567894");
+
+ // Call for previous patch set
+ PatchSet.Id previousPatchSetId = new PatchSet.Id(changeId, 1);
+ RevId previousRevId = createMock(RevId.class);
+ expect(previousRevId.get())
+ .andReturn("9876543211987654321298765432139876543214").anyTimes();
+
+ PatchSet previousPatchSet = createMock(PatchSet.class);
+ expect(previousPatchSet.getRevision()).andReturn(previousRevId).anyTimes();
+
+ PatchSetAccess patchSetAccess = createMock(PatchSetAccess.class);
+ expect(patchSetAccess.get(previousPatchSetId)).andReturn(previousPatchSet);
+
+ expect(commitMessageFetcher.fetchGuarded("testProject",
+ "9876543211987654321298765432139876543214")).andReturn(
+ "subject\n" +
+ "bug#42 bug#4711\n" +
+ "\n" +
+ "Bug: bug#16\n" +
+ "Change-Id: I9876543211987654321298765432139876543214");
+
+ expect(db.patchSets()).andReturn(patchSetAccess);
+
+ PatchSet.Id currentPatchSetId = createMock(PatchSet.Id.class);
+ expect(currentPatchSetId.get()).andReturn(2).anyTimes();
+ expect(currentPatchSetId.getParentKey()).andReturn(changeId)
+ .anyTimes();
+
+ replayMocks();
+
+ IssueExtractor issueExtractor = injector.getInstance(IssueExtractor.class);
+ Map<String,Set<String>> actual = issueExtractor.getIssueIds("testProject",
+ "1234567891123456789212345678931234567894", currentPatchSetId);
+
+ Map<String,Set<String>> expected = Maps.newHashMap();
+ expected.put("16", Sets.newHashSet("somewhere", "body", "added@body"));
+ expected.put("42", Sets.newHashSet("somewhere", "subject", "added@subject",
+ "body", "footer", "added@footer"));
+ assertEquals("Extracted issues do not match", expected, actual);
+
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ assertLogMessageContains("Matching");
+ }
+
@Override
public void setUp() throws Exception {
super.setUp();
@@ -656,6 +1093,9 @@
commitMessageFetcher = createMock(CommitMessageFetcher.class);
bind(CommitMessageFetcher.class).toInstance(commitMessageFetcher);
+
+ db = createMock(ReviewDb.class);
+ bind(ReviewDb.class).toInstance(db);
}
}
}
\ No newline at end of file
diff --git a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java
index ef9270b..5c331a7 100644
--- a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java
+++ b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyExtractorTest.java
@@ -20,6 +20,8 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ChangeAttribute;
@@ -91,7 +93,9 @@
.andReturn(propertyReason);
changeAttribute.project = "testProject";
+ changeAttribute.number = "176";
patchSetAttribute.revision = "testRevision";
+ patchSetAttribute.number = "3";
Set<Property> common = Sets.newHashSet();
common.add(propertyChange);
@@ -99,7 +103,8 @@
common.add(propertyPatchSet);
common.add(propertyReason);
- eventHelper(event, "ChangeAbandonedEvent", "change-abandoned", common);
+ eventHelper(event, "ChangeAbandonedEvent", "change-abandoned", common,
+ true);
}
public void testChangeMergedEvent() {
@@ -124,14 +129,16 @@
.andReturn(Sets.newHashSet(propertyPatchSet));
changeAttribute.project = "testProject";
+ changeAttribute.number = "176";
patchSetAttribute.revision = "testRevision";
+ patchSetAttribute.number = "3";
Set<Property> common = Sets.newHashSet();
common.add(propertyChange);
common.add(propertySubmitter);
common.add(propertyPatchSet);
- eventHelper(event, "ChangeMergedEvent", "change-merged", common);
+ eventHelper(event, "ChangeMergedEvent", "change-merged", common, true);
}
public void testChangeRestoredEvent() {
@@ -161,7 +168,9 @@
.andReturn(propertyReason);
changeAttribute.project = "testProject";
+ changeAttribute.number = "176";
patchSetAttribute.revision = "testRevision";
+ patchSetAttribute.number = "3";
Set<Property> common = Sets.newHashSet();
common.add(propertyChange);
@@ -169,7 +178,7 @@
common.add(propertyPatchSet);
common.add(propertyReason);
- eventHelper(event, "ChangeRestoredEvent", "change-restored", common);
+ eventHelper(event, "ChangeRestoredEvent", "change-restored", common, true);
}
public void testCommentAddedEvent() {
@@ -199,7 +208,9 @@
.andReturn(propertyComment);
changeAttribute.project = "testProject";
+ changeAttribute.number = "176";
patchSetAttribute.revision = "testRevision";
+ patchSetAttribute.number = "3";
Set<Property> common = Sets.newHashSet();
common.add(propertyChange);
@@ -207,7 +218,7 @@
common.add(propertyPatchSet);
common.add(propertyComment);
- eventHelper(event, "CommentAddedEvent", "comment-added", common);
+ eventHelper(event, "CommentAddedEvent", "comment-added", common, true);
}
public void testPatchSetCreatedEvent() {
@@ -232,14 +243,17 @@
.andReturn(Sets.newHashSet(propertyPatchSet));
changeAttribute.project = "testProject";
+ changeAttribute.number = "176";
patchSetAttribute.revision = "testRevision";
+ patchSetAttribute.number = "3";
Set<Property> common = Sets.newHashSet();
common.add(propertyChange);
common.add(propertySubmitter);
common.add(propertyPatchSet);
- eventHelper(event, "PatchSetCreatedEvent", "patchset-created", common);
+ eventHelper(event, "PatchSetCreatedEvent", "patchset-created", common,
+ true);
}
public void testRefUpdatedEvent() {
@@ -265,11 +279,11 @@
common.add(propertySubmitter);
common.add(propertyRefUpdated);
- eventHelper(event, "RefUpdatedEvent", "ref-updated", common);
+ eventHelper(event, "RefUpdatedEvent", "ref-updated", common, false);
}
private void eventHelper(ChangeEvent event, String className, String type,
- Set<Property> common) {
+ Set<Property> common, boolean withRevision) {
PropertyExtractor propertyExtractor = injector.getInstance(
PropertyExtractor.class);
@@ -304,8 +318,14 @@
HashMap<String,Set<String>> issueMap = Maps.newHashMap();
issueMap.put("4711", Sets.newHashSet("body", "anywhere"));
issueMap.put("42", Sets.newHashSet("footer", "anywhere"));
- expect(issueExtractor.getIssueIds("testProject", "testRevision"))
- .andReturn(issueMap);
+ if (withRevision) {
+ PatchSet.Id patchSetId = new PatchSet.Id(new Change.Id(176), 3);
+ expect(issueExtractor.getIssueIds("testProject", "testRevision",
+ patchSetId)).andReturn(issueMap);
+ } else {
+ expect(issueExtractor.getIssueIds("testProject", "testRevision"))
+ .andReturn(issueMap);
+ }
replayMocks();
