Add noOneElse() to FakeEmailSenderSubject
Adding this assertion to the end of every chain of assertions on
FakeEmailSender gives us a couple of advantages. We can get rid of
explicitly configured project watches, and just watch all watches all
the time, and use this to automatically verify that notifications don't
inadvertently leak to some unexpected watch type. We can also get rid of
notTo(...) assertions (but we don't have to).
Change-Id: Id31a0a3fdd54239c4e7761caf1ed1a1204076211
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
index a962c70..5539f00 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
@@ -39,11 +39,13 @@
import com.google.gerrit.testutil.FakeEmailSender;
import com.google.gerrit.testutil.FakeEmailSender.Message;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.function.Function;
import java.util.stream.Collectors;
import org.eclipse.jgit.junit.TestRepository;
import org.junit.After;
@@ -92,6 +94,7 @@
private Message message;
private StagedUsers users;
private Map<RecipientType, List<String>> recipients = new HashMap<>();
+ private Set<String> accountedFor = new HashSet<>();
FakeEmailSenderSubject(FailureStrategy failureStrategy, FakeEmailSender target) {
super(failureStrategy, target);
@@ -131,6 +134,11 @@
// Return a named subject that displays a human-readable table of
// recipients.
+ return named(recipientMapToString(recipients, e -> users.emailToName(e)));
+ }
+
+ private static String recipientMapToString(
+ Map<RecipientType, List<String>> recipients, Function<String, String> emailToName) {
StringBuilder buf = new StringBuilder();
buf.append('[');
for (RecipientType type : ImmutableList.of(TO, CC, BCC)) {
@@ -140,12 +148,12 @@
String delim = " ";
for (String r : recipients.get(type)) {
buf.append(delim);
- buf.append(users.emailToName(r));
+ buf.append(emailToName.apply(r));
delim = ", ";
}
}
buf.append("\n]");
- return named(buf.toString());
+ return buf.toString();
}
List<String> parseAddresses(Message msg, String headerName) {
@@ -189,6 +197,35 @@
expected ? "notifies" : "doesn't notify",
"]\n" + type + ": " + users.emailToName(email) + "\n]");
}
+ if (expected) {
+ accountedFor.add(email);
+ }
+ }
+
+ public FakeEmailSenderSubject noOneElse() {
+ for (Map.Entry<NotifyType, TestAccount> watchEntry : users.watchers.entrySet()) {
+ if (!accountedFor.contains(watchEntry.getValue().email)) {
+ notTo(watchEntry.getKey());
+ }
+ }
+
+ Map<RecipientType, List<String>> unaccountedFor = new HashMap<>();
+ boolean ok = true;
+ for (Map.Entry<RecipientType, List<String>> entry : recipients.entrySet()) {
+ unaccountedFor.put(entry.getKey(), new ArrayList<>());
+ for (String address : entry.getValue()) {
+ if (!accountedFor.contains(address)) {
+ unaccountedFor.get(entry.getKey()).add(address);
+ ok = false;
+ }
+ }
+ }
+ if (!ok) {
+ fail(
+ "was fully tested, missing assertions for: "
+ + recipientMapToString(unaccountedFor, e -> users.emailToName(e)));
+ }
+ return this;
}
public FakeEmailSenderSubject notTo(String... emails) {
@@ -291,7 +328,7 @@
return account;
}
- public StagedUsers(List<NotifyType> watches) throws Exception {
+ public StagedUsers() throws Exception {
synchronized (stagedUsers) {
if (stagedUsers.containsKey(usersCacheKey())) {
StagedUsers existing = stagedUsers.get(usersCacheKey());
@@ -319,7 +356,10 @@
setApiUser(watchingProjectOwner);
watch(allProjects.get(), pwi -> pwi.notifyNewChanges = true);
- for (NotifyType watch : watches) {
+ for (NotifyType watch : NotifyType.values()) {
+ if (watch == NotifyType.ALL) {
+ continue;
+ }
TestAccount watcher = testAccount(watch.toString());
setApiUser(watcher);
watch(
@@ -400,14 +440,13 @@
protected final PushOneCommit.Result result;
public final String changeId;
- StagedPreChange(String ref, List<NotifyType> watches) throws Exception {
- this(ref, null, watches);
+ StagedPreChange(String ref) throws Exception {
+ this(ref, null);
}
- StagedPreChange(
- String ref, @Nullable PushOptionGenerator pushOptionGenerator, List<NotifyType> watches)
+ StagedPreChange(String ref, @Nullable PushOptionGenerator pushOptionGenerator)
throws Exception {
- super(watches);
+ super();
List<String> pushOptions = null;
if (pushOptionGenerator != null) {
pushOptions = pushOptionGenerator.pushOptions(this);
@@ -424,19 +463,18 @@
}
}
- protected StagedPreChange stagePreChange(String ref, NotifyType... watches) throws Exception {
- return new StagedPreChange(ref, ImmutableList.copyOf(watches));
+ protected StagedPreChange stagePreChange(String ref) throws Exception {
+ return new StagedPreChange(ref);
}
protected StagedPreChange stagePreChange(
- String ref, @Nullable PushOptionGenerator pushOptionGenerator, NotifyType... watches)
- throws Exception {
- return new StagedPreChange(ref, pushOptionGenerator, ImmutableList.copyOf(watches));
+ String ref, @Nullable PushOptionGenerator pushOptionGenerator) throws Exception {
+ return new StagedPreChange(ref, pushOptionGenerator);
}
protected class StagedChange extends StagedPreChange {
- StagedChange(String ref, List<NotifyType> watches) throws Exception {
- super(ref, watches);
+ StagedChange(String ref) throws Exception {
+ super(ref);
setApiUser(starrer);
gApi.accounts().self().starChange(result.getChangeId());
@@ -447,39 +485,39 @@
}
}
- protected StagedChange stageReviewableChange(NotifyType... watches) throws Exception {
- return new StagedChange("refs/for/master", ImmutableList.copyOf(watches));
+ protected StagedChange stageReviewableChange() throws Exception {
+ return new StagedChange("refs/for/master");
}
- protected StagedChange stageWipChange(NotifyType... watches) throws Exception {
- return new StagedChange("refs/for/master%wip", ImmutableList.copyOf(watches));
+ protected StagedChange stageWipChange() throws Exception {
+ return new StagedChange("refs/for/master%wip");
}
- protected StagedChange stageReviewableWipChange(NotifyType... watches) throws Exception {
- StagedChange sc = stageReviewableChange(watches);
+ protected StagedChange stageReviewableWipChange() throws Exception {
+ StagedChange sc = stageReviewableChange();
setApiUser(sc.owner);
gApi.changes().id(sc.changeId).setWorkInProgress();
return sc;
}
- protected StagedChange stageAbandonedReviewableChange(NotifyType... watches) throws Exception {
- StagedChange sc = stageReviewableChange(watches);
+ protected StagedChange stageAbandonedReviewableChange() throws Exception {
+ StagedChange sc = stageReviewableChange();
setApiUser(sc.owner);
gApi.changes().id(sc.changeId).abandon();
sender.clear();
return sc;
}
- protected StagedChange stageAbandonedReviewableWipChange(NotifyType... watches) throws Exception {
- StagedChange sc = stageReviewableWipChange(watches);
+ protected StagedChange stageAbandonedReviewableWipChange() throws Exception {
+ StagedChange sc = stageReviewableWipChange();
setApiUser(sc.owner);
gApi.changes().id(sc.changeId).abandon();
sender.clear();
return sc;
}
- protected StagedChange stageAbandonedWipChange(NotifyType... watches) throws Exception {
- StagedChange sc = stageWipChange(watches);
+ protected StagedChange stageAbandonedWipChange() throws Exception {
+ StagedChange sc = stageWipChange();
setApiUser(sc.owner);
gApi.changes().id(sc.changeId).abandon();
sender.clear();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbandonedSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbandonedSenderIT.java
index 35e59c9..dd42a75 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbandonedSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbandonedSenderIT.java
@@ -41,20 +41,20 @@
@Test
public void abandonReviewableChangeByOwner() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner);
assertThat(sender)
.sent("abandon", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ABANDONED_CHANGES);
+ .bcc(ABANDONED_CHANGES)
+ .noOneElse();
}
@Test
public void abandonReviewableChangeByOwnerCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("abandon", sc)
@@ -62,27 +62,28 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ABANDONED_CHANGES);
+ .bcc(ABANDONED_CHANGES)
+ .noOneElse();
}
@Test
public void abandonReviewableChangeByOther() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
TestAccount other = accountCreator.create("other", "other@example.com", "other");
abandon(sc.changeId, other);
assertThat(sender)
.sent("abandon", sc)
- .notTo(other)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ABANDONED_CHANGES);
+ .bcc(ABANDONED_CHANGES)
+ .noOneElse();
}
@Test
public void abandonReviewableChangeByOtherCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
TestAccount other = accountCreator.create("other", "other@example.com", "other");
abandon(sc.changeId, other, CC_ON_OWN_COMMENTS);
assertThat(sender)
@@ -91,31 +92,31 @@
.cc(sc.reviewer, sc.ccer, other)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ABANDONED_CHANGES);
+ .bcc(ABANDONED_CHANGES)
+ .noOneElse();
}
@Test
public void abandonReviewableChangeNotifyOwnersReviewers() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner, OWNER_REVIEWERS);
assertThat(sender)
.sent("abandon", sc)
- .notTo(sc.owner, sc.starrer)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(ABANDONED_CHANGES);
+ .noOneElse();
}
@Test
public void abandonReviewableChangeNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner, OWNER);
assertThat(sender).notSent();
}
@Test
public void abandonReviewableChangeNotifyOwnerCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner, CC_ON_OWN_COMMENTS, OWNER);
// Self-CC applies *after* need for sending notification is determined.
// Since there are no recipients before including the user taking action,
@@ -125,63 +126,57 @@
@Test
public void abandonReviewableChangeByOtherCcingSelfNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
TestAccount other = accountCreator.create("other", "other@example.com", "other");
abandon(sc.changeId, other, CC_ON_OWN_COMMENTS, OWNER);
- assertThat(sender)
- .sent("abandon", sc)
- .to(sc.owner)
- .cc(other)
- .notTo(sc.reviewer, sc.ccer, sc.starrer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(ABANDONED_CHANGES);
+ assertThat(sender).sent("abandon", sc).to(sc.owner).cc(other).noOneElse();
}
@Test
public void abandonReviewableChangeNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner, NONE);
assertThat(sender).notSent();
}
@Test
public void abandonReviewableChangeNotifyNoneCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableChange();
abandon(sc.changeId, sc.owner, CC_ON_OWN_COMMENTS, NONE);
assertThat(sender).notSent();
}
@Test
public void abandonReviewableWipChange() throws Exception {
- StagedChange sc = stageReviewableWipChange(ABANDONED_CHANGES);
+ StagedChange sc = stageReviewableWipChange();
abandon(sc.changeId, sc.owner);
assertThat(sender)
.sent("abandon", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ABANDONED_CHANGES);
+ .bcc(ABANDONED_CHANGES)
+ .noOneElse();
}
@Test
public void abandonWipChange() throws Exception {
- StagedChange sc = stageWipChange(ABANDONED_CHANGES);
+ StagedChange sc = stageWipChange();
abandon(sc.changeId, sc.owner);
assertThat(sender).notSent();
}
@Test
public void abandonWipChangeNotifyAll() throws Exception {
- StagedChange sc = stageWipChange(ABANDONED_CHANGES);
+ StagedChange sc = stageWipChange();
abandon(sc.changeId, sc.owner, ALL);
assertThat(sender)
.sent("abandon", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ABANDONED_CHANGES);
+ .bcc(ABANDONED_CHANGES)
+ .noOneElse();
}
private void abandon(String changeId, TestAccount by) throws Exception {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AddReviewerSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AddReviewerSenderIT.java
index 449835d..2625b0e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AddReviewerSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AddReviewerSenderIT.java
@@ -81,7 +81,7 @@
.to(reviewer)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer);
+ .noOneElse();
});
}
@@ -99,7 +99,7 @@
.to(reviewer)
.cc(sc.reviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer);
+ .noOneElse();
});
}
@@ -117,7 +117,7 @@
.to(reviewer)
.cc(sc.owner, sc.reviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer);
+ .noOneElse();
});
}
@@ -136,7 +136,7 @@
.to(reviewer)
.cc(sc.owner, sc.reviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer, other);
+ .noOneElse();
});
}
@@ -155,7 +155,7 @@
.to(reviewer)
.cc(sc.owner, sc.reviewer, other)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer);
+ .noOneElse();
});
}
@@ -185,7 +185,7 @@
.to(email)
.cc(sc.reviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer);
+ .noOneElse();
});
}
@@ -225,7 +225,7 @@
.to(reviewer)
.cc(sc.reviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer);
+ .noOneElse();
});
}
@@ -242,7 +242,7 @@
.to(reviewer)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer);
+ .noOneElse();
});
}
@@ -260,7 +260,7 @@
.to(reviewer)
.cc(sc.reviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer);
+ .noOneElse();
});
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java
index e82c294..ca441cf 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CommentSenderIT.java
@@ -33,34 +33,34 @@
public class CommentSenderIT extends AbstractNotificationTest {
@Test
public void commentOnReviewableChangeByOwner() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.owner, sc.changeId, ENABLED);
assertThat(sender)
.sent("comment", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByReviewer() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.reviewer, sc.changeId, ENABLED);
assertThat(sender)
.sent("comment", sc)
- .notTo(sc.reviewer)
.to(sc.owner)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByOwnerCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.owner, sc.changeId, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("comment", sc)
@@ -68,12 +68,13 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByReviewerCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.reviewer, sc.changeId, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("comment", sc)
@@ -81,28 +82,29 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByOther() throws Exception {
TestAccount other = accountCreator.create("other", "other@example.com", "other");
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(other, sc.changeId, ENABLED);
assertThat(sender)
.sent("comment", sc)
- .notTo(other)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByOtherCcingSelf() throws Exception {
TestAccount other = accountCreator.create("other", "other@example.com", "other");
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(other, sc.changeId, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("comment", sc)
@@ -110,31 +112,31 @@
.cc(sc.reviewer, sc.ccer, other)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByOwnerNotifyOwnerReviewers() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.owner, sc.changeId, ENABLED, OWNER_REVIEWERS);
assertThat(sender)
.sent("comment", sc)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.owner, sc.starrer)
- .notTo(ALL_COMMENTS);
+ .noOneElse();
}
@Test
public void commentOnReviewableChangeByOwnerNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.owner, sc.changeId, ENABLED, OWNER);
assertThat(sender).notSent();
}
@Test
public void commentOnReviewableChangeByOwnerCcingSelfNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
setEmailStrategy(sc.owner, CC_ON_OWN_COMMENTS);
review(sc.owner, sc.changeId, ENABLED, OWNER);
assertThat(sender).notSent(); // TODO(logan): Why not send to owner?
@@ -142,14 +144,14 @@
@Test
public void commentOnReviewableChangeByOwnerNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
review(sc.owner, sc.changeId, ENABLED, NONE);
assertThat(sender).notSent();
}
@Test
public void commentOnReviewableChangeByOwnerCcingSelfNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
setEmailStrategy(sc.owner, CC_ON_OWN_COMMENTS);
review(sc.owner, sc.changeId, ENABLED, NONE);
assertThat(sender).notSent(); // TODO(logan): Why not send to owner?
@@ -157,62 +159,50 @@
@Test
public void commentOnWipChangeByOwner() throws Exception {
- StagedChange sc = stageWipChange(ALL_COMMENTS);
+ StagedChange sc = stageWipChange();
review(sc.owner, sc.changeId, ENABLED);
assertThat(sender).notSent();
}
@Test
public void commentOnWipChangeByOwnerCcingSelf() throws Exception {
- StagedChange sc = stageWipChange(ALL_COMMENTS);
+ StagedChange sc = stageWipChange();
review(sc.owner, sc.changeId, CC_ON_OWN_COMMENTS);
assertThat(sender).notSent();
}
@Test
public void commentOnWipChangeByOwnerNotifyAll() throws Exception {
- StagedChange sc = stageWipChange(ALL_COMMENTS);
+ StagedChange sc = stageWipChange();
review(sc.owner, sc.changeId, ENABLED, ALL);
assertThat(sender)
.sent("comment", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnWipChangeByBot() throws Exception {
- StagedChange sc = stageWipChange(ALL_COMMENTS);
+ StagedChange sc = stageWipChange();
TestAccount bot = sc.testAccount("bot");
review(bot, sc.changeId, ENABLED, null, "tag");
- assertThat(sender)
- .sent("comment", sc)
- .to(sc.owner)
- .notTo(sc.reviewer, sc.ccer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("comment", sc).to(sc.owner).noOneElse();
}
@Test
public void commentOnReviewableWipChangeByBot() throws Exception {
- StagedChange sc = stageReviewableWipChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableWipChange();
TestAccount bot = sc.testAccount("bot");
review(bot, sc.changeId, ENABLED, null, "tag");
- assertThat(sender)
- .sent("comment", sc)
- .to(sc.owner)
- .notTo(sc.reviewer, sc.ccer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("comment", sc).to(sc.owner).noOneElse();
}
@Test
public void commentOnReviewableWipChangeByBotNotifyAll() throws Exception {
- StagedChange sc = stageWipChange(ALL_COMMENTS);
+ StagedChange sc = stageWipChange();
TestAccount bot = sc.testAccount("bot");
review(bot, sc.changeId, ENABLED, ALL, "tag");
assertThat(sender)
@@ -221,20 +211,21 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void commentOnReviewableWipChangeByOwner() throws Exception {
- StagedChange sc = stageReviewableWipChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableWipChange();
review(sc.owner, sc.changeId, ENABLED);
assertThat(sender)
.sent("comment", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
private void review(TestAccount account, String changeId, EmailStrategy strategy)
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CreateChangeSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CreateChangeSenderIT.java
index 36dcb5d..66f75a2 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CreateChangeSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/CreateChangeSenderIT.java
@@ -24,47 +24,46 @@
public class CreateChangeSenderIT extends AbstractNotificationTest {
@Test
public void createReviewableChange() throws Exception {
- StagedPreChange spc = stagePreChange("refs/for/master", NEW_CHANGES, NEW_PATCHSETS);
+ StagedPreChange spc = stagePreChange("refs/for/master");
assertThat(sender)
.sent("newchange", spc)
- .notTo(spc.owner)
.to(spc.watchingProjectOwner)
- .bcc(NEW_CHANGES, NEW_PATCHSETS);
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void createWipChange() throws Exception {
- StagedPreChange spc = stagePreChange("refs/for/master%wip", NEW_CHANGES, NEW_PATCHSETS);
+ StagedPreChange spc = stagePreChange("refs/for/master%wip");
assertThat(sender).notSent();
}
@Test
public void createReviewableChangeWithNotifyOwnerReviewers() throws Exception {
- stagePreChange("refs/for/master%notify=OWNER_REVIEWERS", NEW_CHANGES, NEW_PATCHSETS);
+ stagePreChange("refs/for/master%notify=OWNER_REVIEWERS");
assertThat(sender).notSent();
}
@Test
public void createReviewableChangeWithNotifyOwner() throws Exception {
- stagePreChange("refs/for/master%notify=OWNER", NEW_CHANGES, NEW_PATCHSETS);
+ stagePreChange("refs/for/master%notify=OWNER");
assertThat(sender).notSent();
}
@Test
public void createReviewableChangeWithNotifyNone() throws Exception {
- stagePreChange("refs/for/master%notify=OWNER", NEW_CHANGES, NEW_PATCHSETS);
+ stagePreChange("refs/for/master%notify=OWNER");
assertThat(sender).notSent();
}
@Test
public void createWipChangeWithNotifyAll() throws Exception {
- StagedPreChange spc =
- stagePreChange("refs/for/master%wip,notify=ALL", NEW_CHANGES, NEW_PATCHSETS);
+ StagedPreChange spc = stagePreChange("refs/for/master%wip,notify=ALL");
assertThat(sender)
.sent("newchange", spc)
- .notTo(spc.owner)
.to(spc.watchingProjectOwner)
- .bcc(NEW_CHANGES, NEW_PATCHSETS);
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse();
}
@Test
@@ -73,14 +72,12 @@
StagedPreChange spc =
stagePreChange(
"refs/for/master",
- users -> ImmutableList.of("r=" + users.reviewer.username, "cc=" + users.ccer.username),
- NEW_CHANGES,
- NEW_PATCHSETS);
+ users -> ImmutableList.of("r=" + users.reviewer.username, "cc=" + users.ccer.username));
assertThat(sender)
.sent("newchange", spc)
- .notTo(spc.owner)
.to(spc.reviewer, spc.watchingProjectOwner)
.cc(spc.ccer)
- .bcc(NEW_CHANGES, NEW_PATCHSETS);
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse();
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteReviewerSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteReviewerSenderIT.java
index 398907f..e28e52c 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteReviewerSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteReviewerSenderIT.java
@@ -24,7 +24,6 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.client.ReviewerState;
-import com.google.gerrit.server.account.WatchConfig.NotifyType;
import org.junit.Before;
import org.junit.Test;
@@ -41,21 +40,21 @@
@Test
public void deleteReviewerFromReviewableChange() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer);
assertThat(sender)
.sent("deleteReviewer", sc)
- .notTo(sc.owner)
.to(extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeByOwnerCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
setEmailStrategy(sc.owner, EmailStrategy.CC_ON_OWN_COMMENTS);
removeReviewer(sc, extraReviewer);
assertThat(sender)
@@ -64,12 +63,13 @@
.cc(extraCcer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeByAdmin() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
setApiUser(admin);
removeReviewer(sc, extraReviewer);
assertThat(sender)
@@ -78,12 +78,13 @@
.cc(extraCcer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeByAdminCcingSelf() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
setEmailStrategy(admin, EmailStrategy.CC_ON_OWN_COMMENTS);
setApiUser(admin);
removeReviewer(sc, extraReviewer);
@@ -93,74 +94,61 @@
.cc(admin, extraCcer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void deleteCcerFromReviewableChange() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
removeReviewer(sc, extraCcer);
assertThat(sender)
.sent("deleteReviewer", sc)
- .notTo(sc.owner)
.to(extraCcer)
.cc(extraReviewer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeNotifyOwnerReviewers() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer, NotifyHandling.OWNER_REVIEWERS);
assertThat(sender)
.sent("deleteReviewer", sc)
- .notTo(sc.owner)
.to(extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ .noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer, NotifyHandling.OWNER);
- assertThat(sender)
- .sent("deleteReviewer", sc)
- .to(extraReviewer)
- .notTo(extraCcer, sc.owner, sc.reviewer, sc.ccer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("deleteReviewer", sc).to(extraReviewer).noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeByOwnerCcingSelfNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
setEmailStrategy(sc.owner, EmailStrategy.CC_ON_OWN_COMMENTS);
removeReviewer(sc, extraReviewer, NotifyHandling.OWNER);
- assertThat(sender)
- .sent("deleteReviewer", sc)
- .to(sc.owner, extraReviewer)
- .notTo(extraCcer, sc.reviewer, sc.ccer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("deleteReviewer", sc).to(sc.owner, extraReviewer).noOneElse();
}
@Test
public void deleteReviewerFromReviewableChangeNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer, NotifyHandling.NONE);
assertThat(sender).notSent();
}
@Test
public void deleteReviewerFromReviewableChangeByOwnerCcingSelfNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange();
+ StagedChange sc = stageReviewableChangeWithExtraReviewer();
setEmailStrategy(sc.owner, EmailStrategy.CC_ON_OWN_COMMENTS);
removeReviewer(sc, extraReviewer, NotifyHandling.NONE);
assertThat(sender).notSent();
@@ -168,73 +156,68 @@
@Test
public void deleteReviewerFromReviewableWipChange() throws Exception {
- StagedChange sc = stageReviewableWipChange();
+ StagedChange sc = stageReviewableWipChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer);
assertThat(sender).notSent();
}
@Test
public void deleteReviewerFromWipChange() throws Exception {
- StagedChange sc = stageWipChange();
+ StagedChange sc = stageWipChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer);
assertThat(sender).notSent();
}
@Test
public void deleteReviewerFromWipChangeNotifyAll() throws Exception {
- StagedChange sc = stageWipChange();
+ StagedChange sc = stageWipChangeWithExtraReviewer();
removeReviewer(sc, extraReviewer, NotifyHandling.ALL);
assertThat(sender)
.sent("deleteReviewer", sc)
- .notTo(sc.owner)
.to(extraReviewer)
.cc(extraCcer, sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void deleteReviewerWithApprovalFromWipChange() throws Exception {
- StagedChange sc = stageWipChange();
+ StagedChange sc = stageWipChangeWithExtraReviewer();
setApiUser(extraReviewer);
gApi.changes().id(sc.changeId).revision("current").review(ReviewInput.recommend());
sender.clear();
setApiUser(sc.owner);
removeReviewer(sc, extraReviewer);
- assertThat(sender)
- .sent("deleteReviewer", sc)
- .to(extraReviewer)
- .notTo(sc.owner, sc.ccer, sc.starrer, extraCcer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("deleteReviewer", sc).to(extraReviewer).noOneElse();
}
@Test
public void deleteReviewerWithApprovalFromWipChangeNotifyOwner() throws Exception {
- StagedChange sc = stageWipChange();
+ StagedChange sc = stageWipChangeWithExtraReviewer();
setApiUser(extraReviewer);
gApi.changes().id(sc.changeId).revision("current").review(ReviewInput.recommend());
sender.clear();
setApiUser(sc.owner);
removeReviewer(sc, extraReviewer, NotifyHandling.OWNER);
- assertThat(sender).sent("deleteReviewer", sc).to(extraReviewer);
+ assertThat(sender).sent("deleteReviewer", sc).to(extraReviewer).noOneElse();
}
@Test
public void deleteReviewerByEmailFromWipChangeInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageWipChange();
+ StagedChange sc = stageWipChangeWithExtraReviewer();
gApi.changes().id(sc.changeId).reviewer(sc.reviewerByEmail).remove();
assertThat(sender).notSent();
}
private interface Stager {
- StagedChange stage(NotifyType... watches) throws Exception;
+ StagedChange stage() throws Exception;
}
- private StagedChange stageChange(Stager stager) throws Exception {
- StagedChange sc = stager.stage(ALL_COMMENTS);
+ private StagedChange stageChangeWithExtraReviewer(Stager stager) throws Exception {
+ StagedChange sc = stager.stage();
ReviewInput in =
ReviewInput.noScore()
.reviewer(extraReviewer.email)
@@ -243,16 +226,16 @@
return sc;
}
- private StagedChange stageReviewableChange() throws Exception {
- return stageChange(this::stageReviewableChange);
+ private StagedChange stageReviewableChangeWithExtraReviewer() throws Exception {
+ return stageChangeWithExtraReviewer(this::stageReviewableChange);
}
- private StagedChange stageReviewableWipChange() throws Exception {
- return stageChange(this::stageReviewableWipChange);
+ private StagedChange stageReviewableWipChangeWithExtraReviewer() throws Exception {
+ return stageChangeWithExtraReviewer(this::stageReviewableWipChange);
}
- private StagedChange stageWipChange() throws Exception {
- return stageChange(this::stageWipChange);
+ private StagedChange stageWipChangeWithExtraReviewer() throws Exception {
+ return stageChangeWithExtraReviewer(this::stageWipChange);
}
private void removeReviewer(StagedChange sc, TestAccount account) throws Exception {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteVoteSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteVoteSenderIT.java
index b3db552..e308e17 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteVoteSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/DeleteVoteSenderIT.java
@@ -22,7 +22,6 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
-import com.google.gerrit.server.account.WatchConfig.NotifyType;
import org.junit.Before;
import org.junit.Test;
@@ -41,12 +40,12 @@
deleteVote(sc, voter);
assertThat(sender)
.sent("deleteVote", sc)
- .notTo(voter)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
@@ -60,7 +59,8 @@
.cc(sc.reviewer, sc.ccer, voter)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
@@ -74,7 +74,8 @@
.cc(sc.reviewer, sc.ccer, voter)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
@@ -89,7 +90,8 @@
.cc(sc.reviewer, sc.ccer, admin, voter)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
@@ -98,12 +100,10 @@
deleteVote(sc, voter, NotifyHandling.OWNER_REVIEWERS);
assertThat(sender)
.sent("deleteVote", sc)
- .notTo(voter)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ .noOneElse();
}
@Test
@@ -116,22 +116,14 @@
.to(sc.owner)
.cc(sc.reviewer, sc.ccer, voter)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ .noOneElse();
}
@Test
public void deleteVoteFromReviewableChangeNotifyOwner() throws Exception {
StagedChange sc = stageReviewableChange();
deleteVote(sc, voter, NotifyHandling.OWNER);
- assertThat(sender)
- .sent("deleteVote", sc)
- .notTo(voter)
- .to(sc.owner)
- .notTo(sc.reviewer, sc.ccer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("deleteVote", sc).to(sc.owner).noOneElse();
}
@Test
@@ -139,14 +131,7 @@
StagedChange sc = stageReviewableChange();
selfCc();
deleteVote(sc, voter, NotifyHandling.OWNER);
- assertThat(sender)
- .sent("deleteVote", sc)
- .to(sc.owner)
- .cc(voter)
- .notTo(sc.reviewer, sc.ccer)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS);
+ assertThat(sender).sent("deleteVote", sc).to(sc.owner).cc(voter).noOneElse();
}
@Test
@@ -170,12 +155,12 @@
deleteVote(sc, voter);
assertThat(sender)
.sent("deleteVote", sc)
- .notTo(voter)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
@@ -184,35 +169,35 @@
deleteVote(sc, voter);
assertThat(sender)
.sent("deleteVote", sc)
- .notTo(voter)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
private interface Stager {
- StagedChange stage(NotifyType... watches) throws Exception;
+ StagedChange stage() throws Exception;
}
private StagedChange stageChange(Stager stager) throws Exception {
- StagedChange sc = stager.stage(ALL_COMMENTS);
+ StagedChange sc = stager.stage();
setApiUser(voter);
gApi.changes().id(sc.changeId).revision("current").review(ReviewInput.recommend());
return sc;
}
- private StagedChange stageReviewableChange() throws Exception {
- return stageChange(this::stageReviewableChange);
+ protected StagedChange stageReviewableChange() throws Exception {
+ return stageChange(() -> super.stageReviewableChange());
}
- private StagedChange stageReviewableWipChange() throws Exception {
- return stageChange(this::stageReviewableWipChange);
+ protected StagedChange stageReviewableWipChange() throws Exception {
+ return stageChange(() -> super.stageReviewableWipChange());
}
- private StagedChange stageWipChange() throws Exception {
- return stageChange(this::stageWipChange);
+ protected StagedChange stageWipChange() throws Exception {
+ return stageChange(() -> super.stageWipChange());
}
private void deleteVote(StagedChange sc, TestAccount account) throws Exception {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MergedSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MergedSenderIT.java
index e284564..c78b37e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MergedSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MergedSenderIT.java
@@ -28,7 +28,6 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
-import com.google.gerrit.server.account.WatchConfig;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.Util;
import org.junit.Before;
@@ -51,20 +50,20 @@
@Test
public void mergeByOwner() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, sc.owner);
assertThat(sender)
.sent("merged", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS, SUBMITTED_CHANGES);
+ .bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
+ .noOneElse();
}
@Test
public void mergeByOwnerCcingSelf() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, sc.owner, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("merged", sc)
@@ -72,26 +71,27 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS, SUBMITTED_CHANGES);
+ .bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
+ .noOneElse();
}
@Test
public void mergeByReviewer() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, sc.reviewer);
assertThat(sender)
.sent("merged", sc)
- .notTo(sc.reviewer)
.to(sc.owner)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS, SUBMITTED_CHANGES);
+ .bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
+ .noOneElse();
}
@Test
public void mergeByReviewerCcingSelf() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, sc.reviewer, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("merged", sc)
@@ -99,56 +99,47 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS, SUBMITTED_CHANGES);
+ .bcc(ALL_COMMENTS, SUBMITTED_CHANGES)
+ .noOneElse();
}
@Test
public void mergeByOtherNotifyOwnerReviewers() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, other, OWNER_REVIEWERS);
assertThat(sender)
.sent("merged", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(sc.starrer)
- .notTo(ALL_COMMENTS, SUBMITTED_CHANGES);
+ .noOneElse();
}
@Test
public void mergeByOtherNotifyOwner() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, other, OWNER);
- assertThat(sender)
- .sent("merged", sc)
- .notTo(sc.reviewer, sc.ccer, sc.starrer, other)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(ALL_COMMENTS, SUBMITTED_CHANGES);
+ assertThat(sender).sent("merged", sc).to(sc.owner).noOneElse();
}
@Test
public void mergeByOtherCcingSelfNotifyOwner() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
setEmailStrategy(other, EmailStrategy.CC_ON_OWN_COMMENTS);
merge(sc.changeId, other, OWNER);
- assertThat(sender)
- .sent("merged", sc)
- .to(sc.owner)
- .notTo(sc.reviewer, sc.ccer, sc.starrer, other)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(ALL_COMMENTS, SUBMITTED_CHANGES);
+ assertThat(sender).sent("merged", sc).to(sc.owner).noOneElse();
}
@Test
public void mergeByOtherNotifyNone() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
merge(sc.changeId, other, NONE);
assertThat(sender).notSent();
}
@Test
public void mergeByOtherCcingSelfNotifyNone() throws Exception {
- StagedChange sc = stageChange(ALL_COMMENTS, SUBMITTED_CHANGES);
+ StagedChange sc = stageChange();
setEmailStrategy(other, EmailStrategy.CC_ON_OWN_COMMENTS);
merge(sc.changeId, other, NONE);
assertThat(sender).notSent();
@@ -179,8 +170,8 @@
gApi.changes().id(changeId).revision("current").submit(in);
}
- private StagedChange stageChange(WatchConfig.NotifyType... watches) throws Exception {
- StagedChange sc = stageReviewableChange(watches);
+ private StagedChange stageChange() throws Exception {
+ StagedChange sc = stageReviewableChange();
setApiUser(sc.reviewer);
gApi.changes().id(sc.changeId).revision("current").review(ReviewInput.approve());
sender.clear();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ReplacePatchSetSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ReplacePatchSetSenderIT.java
index 46cf41b..c5f02cc 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ReplacePatchSetSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ReplacePatchSetSenderIT.java
@@ -37,51 +37,52 @@
@Test
public void newPatchSetByOwnerOnReviewableChangeInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetByOwnerOnReviewableChangeInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
.to(sc.reviewer, sc.ccer)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", other);
+ // TODO(logan): This should include owner but currently doesn't because
+ // it's sent *from* the owner.
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner) // TODO(logan): This email shouldn't come from the owner.
.to(sc.reviewer, other)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", other);
assertThat(sender)
.sent("newpatchset", sc)
@@ -89,13 +90,14 @@
.to(sc.reviewer, sc.ccer, other)
.notTo(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeOwnerSelfCcInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newpatchset", sc)
@@ -103,91 +105,82 @@
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeOwnerSelfCcInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
.to(sc.owner, sc.reviewer, sc.ccer, other)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeNotifyOwnerReviewersInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other);
+ // TODO(logan): This should include owner but currently doesn't because
+ // it's sent *from* the owner.
assertThat(sender)
.sent("newpatchset", sc)
- // TODO(logan): This email shouldn't come from the owner.
- .notTo(sc.owner, sc.starrer, other)
.to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(NEW_PATCHSETS);
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeNotifyOwnerReviewersInReviewDb()
throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other);
- assertThat(sender)
- .sent("newpatchset", sc)
- .to(sc.reviewer, sc.ccer)
- .notTo(sc.starrer, other)
- .notTo(sc.owner) // TODO(logan): This email shouldn't come from the owner.
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(NEW_PATCHSETS);
+ // TODO(logan): This should include owner but currently doesn't because
+ // it's sent *from* the owner.
+ assertThat(sender).sent("newpatchset", sc).to(sc.reviewer, sc.ccer).noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeOwnerSelfCcNotifyOwnerReviewersInNoteDb()
throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.starrer, other)
.to(sc.owner, sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
- .notTo(NEW_PATCHSETS);
+ .noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeOwnerSelfCcNotifyOwnerReviewersInReviewDb()
throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other, EmailStrategy.CC_ON_OWN_COMMENTS);
- assertThat(sender)
- .sent("newpatchset", sc)
- .notTo(sc.starrer, other)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
- .to(sc.owner, sc.reviewer, sc.ccer)
- .notTo(NEW_PATCHSETS);
+ assertThat(sender).sent("newpatchset", sc).to(sc.owner, sc.reviewer, sc.ccer).noOneElse();
}
@Test
public void newPatchSetByOtherOnReviewableChangeNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER", other);
assertThat(sender).notSent();
}
@Test
public void newPatchSetByOtherOnReviewableChangeOwnerSelfCcNotifyOwner() throws Exception {
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER", other, EmailStrategy.CC_ON_OWN_COMMENTS);
// TODO(logan): This email shouldn't come from the owner, and that's why
// no email is currently sent (owner isn't CCing self).
@@ -196,7 +189,7 @@
@Test
public void newPatchSetByOtherOnReviewableChangeNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=NONE", other);
// TODO(logan): This email shouldn't come from the owner, and that's why
// no email is currently sent (owner isn't CCing self).
@@ -205,21 +198,21 @@
@Test
public void newPatchSetByOtherOnReviewableChangeOwnerSelfCcNotifyNone() throws Exception {
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=NONE", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender).notSent();
}
@Test
public void newPatchSetByOwnerOnReviewableChangeToWip() throws Exception {
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%wip", sc.owner);
assertThat(sender).notSent();
}
@Test
public void newPatchSetOnWipChange() throws Exception {
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%wip", sc.owner);
assertThat(sender).notSent();
}
@@ -227,64 +220,62 @@
@Test
public void newPatchSetOnWipChangeNotifyAllInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%wip,notify=ALL", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetOnWipChangeNotifyAllInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%wip,notify=ALL", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
.to(sc.reviewer, sc.ccer)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetOnWipChangeToReadyInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%ready", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetOnWipChangeToReadyInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%ready", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
.to(sc.reviewer, sc.ccer)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
}
@Test
public void newPatchSetOnReviewableWipChange() throws Exception {
- StagedChange sc = stageReviewableWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableWipChange();
pushTo(sc, "refs/for/master%wip", sc.owner);
assertThat(sender).notSent();
}
@@ -292,39 +283,39 @@
@Test
public void newPatchSetOnReviewableChangeAddingReviewerInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
TestAccount newReviewer = sc.testAccount("newReviewer");
pushTo(sc, "refs/for/master%r=" + newReviewer.username, sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer, newReviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
assertThat(sender).notSent();
}
@Test
public void newPatchSetOnReviewableChangeAddingReviewerInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageReviewableChange(NEW_PATCHSETS);
+ StagedChange sc = stageReviewableChange();
TestAccount newReviewer = sc.testAccount("newReviewer");
pushTo(sc, "refs/for/master%r=" + newReviewer.username, sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer, sc.ccer, newReviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
assertThat(sender).notSent();
}
@Test
public void newPatchSetOnWipChangeAddingReviewer() throws Exception {
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
TestAccount newReviewer = sc.testAccount("newReviewer");
pushTo(sc, "refs/for/master%r=" + newReviewer.username, sc.owner);
assertThat(sender).notSent();
@@ -333,64 +324,64 @@
@Test
public void newPatchSetOnWipChangeAddingReviewerNotifyAllInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
TestAccount newReviewer = sc.testAccount("newReviewer");
pushTo(sc, "refs/for/master%notify=ALL,r=" + newReviewer.username, sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer, newReviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
assertThat(sender).notSent();
}
@Test
public void newPatchSetOnWipChangeAddingReviewerNotifyAllInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
TestAccount newReviewer = sc.testAccount("newReviewer");
pushTo(sc, "refs/for/master%notify=ALL,r=" + newReviewer.username, sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer, sc.ccer, newReviewer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
assertThat(sender).notSent();
}
@Test
public void newPatchSetOnWipChangeSettingReadyInNoteDb() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%ready", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
assertThat(sender).notSent();
}
@Test
public void newPatchSetOnWipChangeSettingReadyInReviewDb() throws Exception {
assume().that(notesMigration.readChanges()).isFalse();
- StagedChange sc = stageWipChange(NEW_PATCHSETS);
+ StagedChange sc = stageWipChange();
pushTo(sc, "refs/for/master%ready", sc.owner);
assertThat(sender)
.sent("newpatchset", sc)
- .notTo(sc.owner)
.to(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(NEW_PATCHSETS);
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
assertThat(sender).notSent();
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RestoredSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RestoredSenderIT.java
index ca9330d..4f6963c 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RestoredSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RestoredSenderIT.java
@@ -33,60 +33,60 @@
@Test
public void restoreReviewableChange() throws Exception {
- StagedChange sc = stageAbandonedReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageAbandonedReviewableChange();
restore(sc.changeId, sc.owner);
assertThat(sender)
.sent("restore", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void restoreReviewableWipChange() throws Exception {
- StagedChange sc = stageAbandonedReviewableWipChange(ALL_COMMENTS);
+ StagedChange sc = stageAbandonedReviewableWipChange();
restore(sc.changeId, sc.owner);
assertThat(sender)
.sent("restore", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void restoreWipChange() throws Exception {
- StagedChange sc = stageAbandonedWipChange(ALL_COMMENTS);
+ StagedChange sc = stageAbandonedWipChange();
restore(sc.changeId, sc.owner);
assertThat(sender)
.sent("restore", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void restoreReviewableChangeByAdmin() throws Exception {
- StagedChange sc = stageAbandonedReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageAbandonedReviewableChange();
restore(sc.changeId, admin);
assertThat(sender)
.sent("restore", sc)
- .notTo(admin)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void restoreReviewableChangeByOwnerCcingSelf() throws Exception {
- StagedChange sc = stageAbandonedReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageAbandonedReviewableChange();
restore(sc.changeId, sc.owner, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("restore", sc)
@@ -94,12 +94,13 @@
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
@Test
public void restoreReviewableChangeByAdminCcingSelf() throws Exception {
- StagedChange sc = stageAbandonedReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageAbandonedReviewableChange();
restore(sc.changeId, admin, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("restore", sc)
@@ -107,7 +108,8 @@
.cc(sc.reviewer, sc.ccer, admin)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
- .bcc(ALL_COMMENTS);
+ .bcc(ALL_COMMENTS)
+ .noOneElse();
}
private void restore(String changeId, TestAccount by) throws Exception {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RevertedSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RevertedSenderIT.java
index b97a3b8..4e0daaf 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RevertedSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/RevertedSenderIT.java
@@ -17,6 +17,8 @@
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS;
import static com.google.gerrit.server.account.WatchConfig.NotifyType.ALL_COMMENTS;
+import static com.google.gerrit.server.account.WatchConfig.NotifyType.NEW_CHANGES;
+import static com.google.gerrit.server.account.WatchConfig.NotifyType.NEW_PATCHSETS;
import com.google.gerrit.acceptance.AbstractNotificationTest;
import com.google.gerrit.acceptance.TestAccount;
@@ -40,19 +42,16 @@
revert(sc, sc.owner);
assertThat(sender)
.sent("newchange", sc)
- .notTo(sc.owner)
- .to(sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .to(sc.reviewer, sc.ccer, sc.watchingProjectOwner, admin)
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
.notTo(sc.owner)
- .cc(sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -62,20 +61,16 @@
revert(sc, sc.owner);
assertThat(sender)
.sent("newchange", sc)
- .notTo(sc.owner)
- .to(sc.reviewer)
+ .to(sc.reviewer, sc.watchingProjectOwner, admin)
.cc(sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
- .notTo(sc.owner)
- .cc(sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -85,19 +80,17 @@
revert(sc, sc.owner, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newchange", sc)
- .to(sc.reviewer, sc.ccer)
+ .to(sc.reviewer, sc.ccer, sc.watchingProjectOwner, admin)
.cc(sc.owner)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
.to(sc.owner)
- .cc(sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -107,19 +100,17 @@
revert(sc, sc.owner, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newchange", sc)
- .to(sc.reviewer)
+ .to(sc.reviewer, sc.watchingProjectOwner, admin)
.cc(sc.owner, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
.to(sc.owner)
- .cc(sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -129,18 +120,15 @@
revert(sc, other);
assertThat(sender)
.sent("newchange", sc)
- .to(sc.owner, sc.reviewer, sc.ccer)
- .notTo(other)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .to(sc.owner, sc.reviewer, sc.ccer, sc.watchingProjectOwner, admin)
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
- .cc(sc.owner, sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.owner, sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -150,19 +138,16 @@
revert(sc, other);
assertThat(sender)
.sent("newchange", sc)
- .to(sc.owner, sc.reviewer)
+ .to(sc.owner, sc.reviewer, sc.watchingProjectOwner, admin)
.cc(sc.ccer)
- .notTo(other)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
- .cc(sc.owner, sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.owner, sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -172,19 +157,17 @@
revert(sc, other, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newchange", sc)
- .to(sc.owner, sc.reviewer, sc.ccer)
+ .to(sc.owner, sc.reviewer, sc.ccer, sc.watchingProjectOwner, admin)
.cc(other)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
.to(other)
- .cc(sc.owner, sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.owner, sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
@Test
@@ -194,23 +177,21 @@
revert(sc, other, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newchange", sc)
- .to(sc.owner, sc.reviewer)
+ .to(sc.owner, sc.reviewer, sc.watchingProjectOwner, admin)
.cc(sc.ccer, other)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .notTo(ALL_COMMENTS); // TODO(logan): Why not?
+ .bcc(NEW_CHANGES, NEW_PATCHSETS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
assertThat(sender)
.sent("revert", sc)
.to(other)
- .cc(sc.owner, sc.reviewer, sc.ccer)
- .notTo(sc.starrer) // TODO(logan): Why not?
- .notTo(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): Why not?
- .bcc(ALL_COMMENTS);
+ .cc(sc.owner, sc.reviewer, sc.ccer, admin)
+ .bcc(ALL_COMMENTS)
+ .noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
}
private StagedChange stageChange() throws Exception {
- StagedChange sc = stageReviewableChange(ALL_COMMENTS);
+ StagedChange sc = stageReviewableChange();
setApiUser(admin);
gApi.changes().id(sc.changeId).revision("current").review(ReviewInput.approve());
gApi.changes().id(sc.changeId).revision("current").submit();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java
index 1801cb9..96e7248 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/SetAssigneeSenderIT.java
@@ -38,9 +38,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
@Test
@@ -49,10 +49,10 @@
assign(sc, sc.owner, sc.assignee, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.reviewer, sc.ccer, sc.starrer)
.cc(sc.owner)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
@Test
@@ -61,9 +61,9 @@
assign(sc, admin, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, admin)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
@Test
@@ -72,10 +72,10 @@
assign(sc, admin, sc.assignee, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer)
.cc(admin)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
@Test
@@ -85,8 +85,8 @@
assign(sc, sc.owner, sc.owner);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, sc.assignee)
- .cc(sc.reviewerByEmail, sc.ccerByEmail); // TODO(logan): This is probably not intended!
+ .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .noOneElse();
}
@Test
@@ -106,9 +106,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, other)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
@Test
@@ -120,8 +120,8 @@
assign(sc, sc.owner, sc.owner);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer, sc.assignee)
- .cc(sc.reviewerByEmail, sc.ccerByEmail); // TODO(logan): This is probably not intended!
+ .cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
+ .noOneElse();
}
@Test
@@ -140,9 +140,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
@Test
@@ -151,9 +151,9 @@
assign(sc, sc.owner, sc.assignee);
assertThat(sender)
.sent("setassignee", sc)
- .notTo(sc.owner, sc.reviewer, sc.ccer, sc.starrer)
.cc(sc.reviewerByEmail, sc.ccerByEmail) // TODO(logan): This is probably not intended!
- .to(sc.assignee);
+ .to(sc.assignee)
+ .noOneElse();
}
private void assign(StagedChange sc, TestAccount by, TestAccount to) throws Exception {
