ChangeEmail: Fix calculation of insertions and change buckets
When computing insertions and change buckets ChangeEmail also counted
lines of the commit message, so that changes never had the change bucket
NoOp or XS (a 1-line commit message is a 10-line /COMMIT_MSG file).
ChangeEmail#getChangeDetail() which formats number of files, insertions
and deletions didn't count the commit message for number of files, but
for insertions the lines from the commit message were counted, which was
inconsistent.
Release-Notes: Fixed calculation of insertions and change buckets for change emails
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I28d3796bfab98c881e38ca5d0a22745b252c92de
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 7bbee2a..ff811a0 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -242,7 +242,9 @@
}
private int getInsertionsCount() {
- return listModifiedFiles().values().stream()
+ return listModifiedFiles().entrySet().stream()
+ .filter(e -> !Patch.COMMIT_MSG.equals(e.getKey()))
+ .map(Map.Entry::getValue)
.map(FileDiffOutput::insertions)
.reduce(0, Integer::sum);
}
@@ -323,8 +325,8 @@
+ "{1,choice,0#0 insertions|1#1 insertion|1<{1} insertions}(+), " //
+ "{2,choice,0#0 deletions|1#1 deletion|1<{2} deletions}(-)" //
+ "\n",
- modifiedFiles.size() - 1, //
- getInsertionsCount(), //
+ modifiedFiles.size() - 1, // -1 to account for the commit message
+ getInsertionsCount(),
getDeletionsCount()));
detail.append("\n");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index e3d69e1..21fc4b4 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -101,6 +101,7 @@
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.EmailHeader.StringEmailHeader;
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.LabelType;
@@ -196,6 +197,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
@@ -4551,6 +4553,47 @@
.contains(String.format("%s has removed %s", admin.fullName(), reviewerInput.reviewer));
}
+ @Test
+ public void emailSubjectContainsChangeSizeBucket() throws Exception {
+ testEmailSubjectContainsChangeSizeBucket(0, "NoOp");
+ testEmailSubjectContainsChangeSizeBucket(1, "XS");
+ testEmailSubjectContainsChangeSizeBucket(9, "XS");
+ testEmailSubjectContainsChangeSizeBucket(10, "S");
+ testEmailSubjectContainsChangeSizeBucket(49, "S");
+ testEmailSubjectContainsChangeSizeBucket(50, "M");
+ testEmailSubjectContainsChangeSizeBucket(249, "M");
+ testEmailSubjectContainsChangeSizeBucket(250, "L");
+ testEmailSubjectContainsChangeSizeBucket(999, "L");
+ testEmailSubjectContainsChangeSizeBucket(1000, "XL");
+ }
+
+ private void testEmailSubjectContainsChangeSizeBucket(
+ int numberOfLines, String expectedSizeBucket) throws Exception {
+ String change;
+ if (numberOfLines == 0) {
+ // create empty change
+ ChangeInput in = new ChangeInput();
+ in.branch = Constants.MASTER;
+ in.subject = "Create a change from the API";
+ in.project = project.get();
+ ChangeInfo info = gApi.changes().create(in).get();
+ change = info.changeId;
+ } else {
+ change =
+ createChange(
+ "subject",
+ expectedSizeBucket + "-file-with-" + numberOfLines + "lines.txt",
+ Collections.nCopies(numberOfLines, "line").stream().collect(joining("\n")))
+ .getChangeId();
+ }
+ sender.clear();
+ gApi.changes().id(change).addReviewer(user.email());
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ assertThat(((StringEmailHeader) messages.get(0).headers().get("Subject")).getString())
+ .contains("[" + expectedSizeBucket + "]");
+ }
+
private PushOneCommit.Result createWorkInProgressChange() throws Exception {
return pushTo("refs/for/master%wip");
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index e44bfcf..cced47f 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -984,7 +984,7 @@
StagedPreChange spc = stagePreChange("refs/for/master");
assertThat(sender)
.sent("newchange", spc)
- .title(String.format("[S] Change in %s[master]: test commit", project));
+ .title(String.format("[XS] Change in %s[master]: test commit", project));
assertThat(sender).didNotSend();
}
