Highlight matching atoms of copy conditions in change messages
When a new patch set is created and approvals are copied to the new
patch set or if approvals are dropped (because they cannot be copied to
the new patch set) Gerrit posts a change message to inform about the
copied and outdated votes. For each copied/outdated vote the change
message includes the corresponding copy condition, so that users can
check why a vote matched or didn't match the copy condition.
Understanding why a vote matched or didn't match the copy condition
could be easier though if the passing and failing atoms in the copy
condition would be highlighted. E.g. for expressions of submit
requirements we already have such highlighting (passing atoms are
underlined in green, failing atoms are underlined in red).
It would be nice to have the same highlighting for copy conditions, but
it's not easily doable. For the expressions of the submit requirements
the highlighting is done in the web frontend. This requires the web
frontend to know about the expression and the passing and failing atoms,
which means that the backend must have an API to return this data.
Adding such an API for copy conditions is not easy, since they are
embedded in change messages which are plain strings. In addition the
backend has only the current copy condition available and in restrospect
it can't tell how the copy condition looked like at the moment when the
copy evaluation for a certain approval was done.
That's why for highlighting atoms in copy conditions we take a different
approach. The change message with the copy conditions is generated at
the moment when the copy evaluation is done. At this moment we can check
which atoms of the copy conditions are passing/failing. We can then use
this information to highlight the passing atoms in the change message.
The change message is a plain string, but Markdown formatting is
supported. We make use of this and use Markdown syntax to print passing
atoms in bold. This is not the same formatting as for expressions of
submit requirements, but it's the best we can do with Markdown. Markdown
does neither support underlining text nor colored text (this is only
possible with HTML snippets but those are not supported in change
messages). This approach can be implemented with little effort and still
allows us to highlight which atoms of a copy condition are passing.
To implement this we compute the passing/failing atoms of the copy
condition at the moment when the copy evaluation in ApprovalCopier is
done. To pass this information to the caller we change the type for the
copied/outdated approvals in ApprovalCopier.Result from PatchSetApproval
to a new ApprovalData type that contains the PatchSetApproval and the
passing and failing atoms (for consistency the method names for getting
the passing/failing atoms are kept the same as for
SubmitRequirementExpressionResult).
ApprovalsUtil gets the copied/outdated approvals as input to format the
change message with copied/dropped votes. In addition to the
copied/outdated approvals it now has the passing and failing atoms
available for each approval, so that it can format the passing atoms in
bold (using Markdown syntax).
So far approvals with the same label had been grouped as one line in the
change message (since they have the same copy condition), e.g.:
Copied Votes:
* Code-Review-2, Code-Review+2 (copy condition: "is:MIN OR is:MAX")
Now we need to split those into multiple lines if the passing atoms
differ, so that in every line we can higlight different atoms:
Copied Votes:
* Code-Review-2 (copy condition: "**is:MIN** OR is:MAX")
* Code-Review+2 (copy condition: "is:MIN OR **is:MAX**")
If approvals for the same label have the same passing atoms they are
still grouped together as one line, e.g.:
Copied Votes:
* Code-Review-1, Code-Review+1 (copy condition: "is:MIN OR **changekind:TRIVIAL_REBASE**")
Bug: Google b/240377462
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I7b42f350f9d79b1dc3cac62aeac7d7c7f8699456
Release-Notes: Matching atoms in copy conditions that are posted as change messages are highlighted in bold now
diff --git a/java/com/google/gerrit/server/approval/ApprovalCopier.java b/java/com/google/gerrit/server/approval/ApprovalCopier.java
index 059445e..cd2d79e 100644
--- a/java/com/google/gerrit/server/approval/ApprovalCopier.java
+++ b/java/com/google/gerrit/server/approval/ApprovalCopier.java
@@ -32,6 +32,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.ChangeKind;
+import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeKindCache;
@@ -85,7 +86,7 @@
* <li>the approval is not overridden by a current approval on the patch set
* </ul>
*/
- public abstract ImmutableSet<PatchSetApproval> copiedApprovals();
+ public abstract ImmutableSet<PatchSetApprovalData> copiedApprovals();
/**
* Approvals on the previous patch set that have not been copied to the patch set.
@@ -96,7 +97,7 @@
* <p>Only returns non-copied approvals of the previous patch set. Approvals from earlier patch
* sets that were outdated before are not included.
*/
- public abstract ImmutableSet<PatchSetApproval> outdatedApprovals();
+ public abstract ImmutableSet<PatchSetApprovalData> outdatedApprovals();
static Result empty() {
return create(
@@ -105,10 +106,68 @@
@VisibleForTesting
public static Result create(
- ImmutableSet<PatchSetApproval> copiedApprovals,
- ImmutableSet<PatchSetApproval> outdatedApprovals) {
+ ImmutableSet<PatchSetApprovalData> copiedApprovals,
+ ImmutableSet<PatchSetApprovalData> outdatedApprovals) {
return new AutoValue_ApprovalCopier_Result(copiedApprovals, outdatedApprovals);
}
+
+ /**
+ * A {@link PatchSetApproval} with information about which atoms of the copy condition are
+ * passing/failing.
+ */
+ @AutoValue
+ public abstract static class PatchSetApprovalData {
+ /** The approval. */
+ public abstract PatchSetApproval patchSetApproval();
+
+ /**
+ * Lists the leaf predicates of the copy condition that are fulfilled.
+ *
+ * <p>Example: The expression
+ *
+ * <pre>
+ * changekind:TRIVIAL_REBASE OR is:MIN
+ * </pre>
+ *
+ * has two leaf predicates:
+ *
+ * <ul>
+ * <li>changekind:TRIVIAL_REBASE
+ * <li>is:MIN
+ * </ul>
+ *
+ * This method will return the leaf predicates that are fulfilled, for example if only the
+ * first predicate is fulfilled, the returned list will be equal to
+ * ["changekind:TRIVIAL_REBASE"].
+ *
+ * <p>Empty if the label type is missing, if there is no copy condition or if the copy
+ * condition is not parseable.
+ */
+ public abstract ImmutableSet<String> passingAtoms();
+
+ /**
+ * Lists the leaf predicates of the copy condition that are not fulfilled. See {@link
+ * #passingAtoms()} for more details.
+ *
+ * <p>Empty if the label type is missing, if there is no copy condition or if the copy
+ * condition is not parseable.
+ */
+ public abstract ImmutableSet<String> failingAtoms();
+
+ @VisibleForTesting
+ public static PatchSetApprovalData create(
+ PatchSetApproval approval,
+ ImmutableSet<String> passingAtoms,
+ ImmutableSet<String> failingAtoms) {
+ return new AutoValue_ApprovalCopier_Result_PatchSetApprovalData(
+ approval, passingAtoms, failingAtoms);
+ }
+
+ private static PatchSetApprovalData createForMissingLabelType(PatchSetApproval approval) {
+ return new AutoValue_ApprovalCopier_Result_PatchSetApprovalData(
+ approval, ImmutableSet.of(), ImmutableSet.of());
+ }
+ }
}
private final GitRepositoryManager repoManager;
@@ -227,17 +286,18 @@
followUpPatchSet.commitId());
boolean isMerge = isMerge(changeNotes.getProjectName(), revWalk, followUpPatchSet);
- if (canCopy(
- changeNotes,
- priorPatchSet.id(),
- followUpPatchSet,
- approverId,
- labelType.get(),
- approvalValue,
- changeKind,
- isMerge,
- revWalk,
- repo.getConfig())) {
+ if (computeCopyResult(
+ changeNotes,
+ priorPatchSet.id(),
+ followUpPatchSet,
+ approverId,
+ labelType.get(),
+ approvalValue,
+ changeKind,
+ isMerge,
+ revWalk,
+ repo.getConfig())
+ .canCopy()) {
targetPatchSetsBuilder.add(followUpPatchSetId);
} else {
// The approval is not copyable to this follow-up patch set.
@@ -251,7 +311,14 @@
return targetPatchSetsBuilder.build();
}
- private boolean canCopy(
+ /**
+ * Checks whether a given approval can be copied from the given source patch set to the given
+ * target patch set.
+ *
+ * <p>The returned result also informs about which atoms of the copy condition are
+ * passing/failing.
+ */
+ private ApprovalCopyResult computeCopyResult(
ChangeNotes changeNotes,
PatchSet.Id sourcePatchSetId,
PatchSet targetPatchSet,
@@ -263,7 +330,7 @@
RevWalk revWalk,
Config repoConfig) {
if (!labelType.getCopyCondition().isPresent()) {
- return false;
+ return ApprovalCopyResult.createForMissingCopyCondition();
}
ApprovalContext ctx =
ApprovalContext.create(
@@ -283,15 +350,18 @@
// request (e.g. a group used in this query might not be visible to the person sending this
// request).
try (ManualRequestContext ignored = requestContext.open()) {
- return approvalQueryBuilder
- .parse(labelType.getCopyCondition().get())
- .asMatchable()
- .match(ctx);
+ Predicate<ApprovalContext> copyConditionPredicate =
+ approvalQueryBuilder.parse(labelType.getCopyCondition().get());
+ boolean canCopy = copyConditionPredicate.asMatchable().match(ctx);
+ ImmutableSet.Builder<String> passingAtoms = ImmutableSet.builder();
+ ImmutableSet.Builder<String> failingAtoms = ImmutableSet.builder();
+ evaluateAtoms(copyConditionPredicate, ctx, passingAtoms, failingAtoms);
+ return ApprovalCopyResult.create(canCopy, passingAtoms.build(), failingAtoms.build());
}
} catch (QueryParseException e) {
logger.atWarning().withCause(e).log(
"Unable to copy label because config is invalid. This should have been caught before.");
- return false;
+ return ApprovalCopyResult.createForNonParseableCopyCondition();
}
}
@@ -321,8 +391,10 @@
nonCopiedApprovalsForGivenPatchSet.forEach(
psa -> currentApprovalsByUser.put(psa.label(), psa.accountId(), psa));
- Table<String, Account.Id, PatchSetApproval> copiedApprovalsByUser = HashBasedTable.create();
- ImmutableSet.Builder<PatchSetApproval> outdatedApprovalsBuilder = ImmutableSet.builder();
+ Table<String, Account.Id, Result.PatchSetApprovalData> copiedApprovalsByUser =
+ HashBasedTable.create();
+ ImmutableSet.Builder<Result.PatchSetApprovalData> outdatedApprovalsBuilder =
+ ImmutableSet.builder();
ImmutableList<PatchSetApproval> priorApprovals =
notes.load().getApprovals().all().get(priorPatchSet.getKey());
@@ -362,35 +434,55 @@
priorPsa.key().patchSetId().changeId().get(),
targetPsId.get(),
projectName);
- outdatedApprovalsBuilder.add(priorPsa);
+ outdatedApprovalsBuilder.add(
+ Result.PatchSetApprovalData.createForMissingLabelType(priorPsa));
continue;
}
- if (canCopy(
- notes,
- priorPsa.patchSetId(),
- targetPatchSet,
- priorPsa.accountId(),
- labelType.get(),
- priorPsa.value(),
- changeKind,
- isMerge,
- rw,
- repoConfig)) {
+ ApprovalCopyResult approvalCopyResult =
+ computeCopyResult(
+ notes,
+ priorPsa.patchSetId(),
+ targetPatchSet,
+ priorPsa.accountId(),
+ labelType.get(),
+ priorPsa.value(),
+ changeKind,
+ isMerge,
+ rw,
+ repoConfig);
+ if (approvalCopyResult.canCopy()) {
if (!currentApprovalsByUser.contains(priorPsa.label(), priorPsa.accountId())) {
+ PatchSetApproval copiedApproval = priorPsa.copyWithPatchSet(targetPatchSet.id());
+
+ // Normalize the copied approval.
+ Optional<PatchSetApproval> copiedApprovalNormalized =
+ labelNormalizer.normalize(notes, copiedApproval);
+ logger.atFine().log(
+ "Copied approval %s has been normalized to %s",
+ copiedApproval,
+ copiedApprovalNormalized.map(PatchSetApproval::toString).orElse("n/a"));
+ if (!copiedApprovalNormalized.isPresent()) {
+ continue;
+ }
+
copiedApprovalsByUser.put(
priorPsa.label(),
priorPsa.accountId(),
- priorPsa.copyWithPatchSet(targetPatchSet.id()));
+ Result.PatchSetApprovalData.create(
+ copiedApprovalNormalized.get(),
+ approvalCopyResult.passingAtoms(),
+ approvalCopyResult.failingAtoms()));
}
} else {
- outdatedApprovalsBuilder.add(priorPsa);
+ outdatedApprovalsBuilder.add(
+ Result.PatchSetApprovalData.create(
+ priorPsa, approvalCopyResult.passingAtoms(), approvalCopyResult.failingAtoms()));
continue;
}
}
- ImmutableSet<PatchSetApproval> copiedApprovals =
- labelNormalizer.normalize(notes, copiedApprovalsByUser.values()).getNormalized();
- return Result.create(copiedApprovals, outdatedApprovalsBuilder.build());
+ return Result.create(
+ ImmutableSet.copyOf(copiedApprovalsByUser.values()), outdatedApprovalsBuilder.build());
}
private boolean isMerge(Project.NameKey project, RevWalk rw, PatchSet patchSet) {
@@ -404,4 +496,72 @@
e);
}
}
+
+ /**
+ * Evaluates a predicate of the copy condition and adds its passing and failing atoms to the given
+ * builders.
+ *
+ * @param predicate a predicate of the copy condition that should be evaluated
+ * @param approvalContext the approval context against which the predicate should be evaluated
+ * @param passingAtoms a builder to which passing atoms should be added
+ * @param failingAtoms a builder to which failing atoms should be added
+ */
+ private static void evaluateAtoms(
+ Predicate<ApprovalContext> predicate,
+ ApprovalContext approvalContext,
+ ImmutableSet.Builder<String> passingAtoms,
+ ImmutableSet.Builder<String> failingAtoms) {
+ if (predicate.isLeaf()) {
+ boolean isPassing = predicate.asMatchable().match(approvalContext);
+ (isPassing ? passingAtoms : failingAtoms).add(predicate.getPredicateString());
+ return;
+ }
+ predicate
+ .getChildren()
+ .forEach(
+ childPredicate ->
+ evaluateAtoms(childPredicate, approvalContext, passingAtoms, failingAtoms));
+ }
+
+ /** Result for checking if an approval can be copied to the next patch set. */
+ @AutoValue
+ abstract static class ApprovalCopyResult {
+ /** Whether the approval can be copied to the next patch set. */
+ abstract boolean canCopy();
+
+ /**
+ * Lists the leaf predicates of the copy condition that are fulfilled. See {@link
+ * Result.PatchSetApprovalData#passingAtoms()} for more details.
+ *
+ * <p>Empty if there is no copy condition or if the copy condition is not parseable.
+ */
+ abstract ImmutableSet<String> passingAtoms();
+
+ /**
+ * Lists the leaf predicates of the copy condition that are not fulfilled. See {@link
+ * Result.PatchSetApprovalData#passingAtoms()} for more details.
+ *
+ * <p>Empty if there is no copy condition or if the copy condition is not parseable.
+ */
+ abstract ImmutableSet<String> failingAtoms();
+
+ private static ApprovalCopyResult create(
+ boolean canCopy, ImmutableSet<String> passingAtoms, ImmutableSet<String> failingAtoms) {
+ return new AutoValue_ApprovalCopier_ApprovalCopyResult(canCopy, passingAtoms, failingAtoms);
+ }
+
+ private static ApprovalCopyResult createForMissingCopyCondition() {
+ return new AutoValue_ApprovalCopier_ApprovalCopyResult(
+ /* canCopy= */ false,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of());
+ }
+
+ private static ApprovalCopyResult createForNonParseableCopyCondition() {
+ return new AutoValue_ApprovalCopier_ApprovalCopyResult(
+ /* canCopy= */ false,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of());
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/approval/ApprovalsUtil.java b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
index bd31356f..09820b1 100644
--- a/java/com/google/gerrit/server/approval/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
@@ -15,8 +15,10 @@
package com.google.gerrit.server.approval;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
@@ -25,6 +27,7 @@
import static java.util.stream.Collectors.joining;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
@@ -85,6 +88,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.StringTokenizer;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -399,12 +403,17 @@
ChangeUpdate changeUpdate) {
ApprovalCopier.Result approvalCopierResult =
approvalCopier.forPatchSet(notes, patchSet, revWalk, repoConfig);
- approvalCopierResult.copiedApprovals().forEach(a -> changeUpdate.putCopiedApproval(a));
+ approvalCopierResult
+ .copiedApprovals()
+ .forEach(approvalData -> changeUpdate.putCopiedApproval(approvalData.patchSetApproval()));
if (!notes.getChange().isWorkInProgress()) {
// The attention set should not be updated when the change is work-in-progress.
addAttentionSetUpdatesForOutdatedApprovals(
- changeUpdate, approvalCopierResult.outdatedApprovals());
+ changeUpdate,
+ approvalCopierResult.outdatedApprovals().stream()
+ .map(ApprovalCopier.Result.PatchSetApprovalData::patchSetApproval)
+ .collect(toImmutableSet()));
}
return approvalCopierResult;
@@ -511,31 +520,40 @@
* "is:FOO")}
* </ul>
*
- * @param approvals the approvals that should be formatted
+ * @param approvalDatas the approvals that should be formatted, with approval meta data
* @param labelTypes the label types
* @return bullet list with the formatted approvals
*/
private String formatApprovalListWithCopyCondition(
- ImmutableSet<PatchSetApproval> approvals, LabelTypes labelTypes) {
+ ImmutableSet<ApprovalCopier.Result.PatchSetApprovalData> approvalDatas,
+ LabelTypes labelTypes) {
StringBuilder message = new StringBuilder();
// sort approvals by label vote so that we list them in a deterministic order
- ImmutableList<PatchSetApproval> approvalsSortedByLabelVote =
- approvals.stream()
- .sorted(comparing(psa -> LabelVote.create(psa.label(), psa.value()).format()))
+ ImmutableList<ApprovalCopier.Result.PatchSetApprovalData> approvalsSortedByLabelVote =
+ approvalDatas.stream()
+ .sorted(
+ comparing(
+ approvalData ->
+ LabelVote.create(
+ approvalData.patchSetApproval().label(),
+ approvalData.patchSetApproval().value())
+ .format()))
.collect(toImmutableList());
- ImmutableListMultimap<String, PatchSetApproval> approvalsByLabel =
- Multimaps.index(approvalsSortedByLabelVote, PatchSetApproval::label);
+ ImmutableListMultimap<String, ApprovalCopier.Result.PatchSetApprovalData> approvalsByLabel =
+ Multimaps.index(
+ approvalsSortedByLabelVote, approvalData -> approvalData.patchSetApproval().label());
- for (Map.Entry<String, Collection<PatchSetApproval>> approvalsByLabelEntry :
- approvalsByLabel.asMap().entrySet()) {
+ for (Map.Entry<String, Collection<ApprovalCopier.Result.PatchSetApprovalData>>
+ approvalsByLabelEntry : approvalsByLabel.asMap().entrySet()) {
String label = approvalsByLabelEntry.getKey();
- Collection<PatchSetApproval> approvalsForSameLabel = approvalsByLabelEntry.getValue();
+ Collection<ApprovalCopier.Result.PatchSetApprovalData> approvalsForSameLabel =
+ approvalsByLabelEntry.getValue();
- message.append("* ");
if (!labelTypes.byLabel(label).isPresent()) {
message
+ .append("* ")
.append(formatApprovalsAsLabelVotesList(approvalsForSameLabel))
.append(" (label type is missing)\n");
continue;
@@ -543,22 +561,65 @@
LabelType labelType = labelTypes.byLabel(label).get();
if (!labelType.getCopyCondition().isPresent()) {
- message.append(formatApprovalsAsLabelVotesList(approvalsForSameLabel)).append("\n");
+ message
+ .append("* ")
+ .append(formatApprovalsAsLabelVotesList(approvalsForSameLabel))
+ .append("\n");
continue;
}
- message
- .append(
- formatApprovalsWithCopyCondition(
- approvalsForSameLabel, labelType.getCopyCondition().get()))
- .append("\n");
+ // Group the approvals that have the same label by the passing atoms. If approvals have the
+ // same label, but have different passing atoms, we need to list them in separate lines
+ // (because in each line we will highlight different passing atoms that matched). Approvals
+ // with the same label and the same passing atoms are formatted as a single line.
+ ImmutableListMultimap<ImmutableSet<String>, ApprovalCopier.Result.PatchSetApprovalData>
+ approvalsForSameLabelByPassingAndFailingAtoms =
+ Multimaps.index(
+ approvalsForSameLabel, ApprovalCopier.Result.PatchSetApprovalData::passingAtoms);
+
+ // Approvals with the same label that have the same passing atoms should have the same failing
+ // atoms (since the label is the same they have the same copy condition).
+ approvalsForSameLabelByPassingAndFailingAtoms
+ .asMap()
+ .values()
+ .forEach(
+ approvalsForSameLabelAndSamePassingAtoms ->
+ checkThatPropertyIsTheSameForAllApprovals(
+ approvalsForSameLabelAndSamePassingAtoms,
+ "failing atoms",
+ approvalData -> approvalData.failingAtoms()));
+
+ // The order in which we add lines for approvals with the same label but different passing
+ // atoms needs to be deterministic for tests. Just sort them by the string representation of
+ // the passing atoms.
+ for (Collection<ApprovalCopier.Result.PatchSetApprovalData>
+ approvalsForSameLabelWithSamePassingAndFailingAtoms :
+ approvalsForSameLabelByPassingAndFailingAtoms.asMap().entrySet().stream()
+ .sorted(
+ comparing(
+ (Map.Entry<
+ ImmutableSet<String>,
+ Collection<ApprovalCopier.Result.PatchSetApprovalData>>
+ e) -> e.getKey().toString()))
+ .map(Map.Entry::getValue)
+ .collect(toImmutableList())) {
+ message
+ .append("* ")
+ .append(
+ formatApprovalsWithCopyCondition(
+ approvalsForSameLabelWithSamePassingAndFailingAtoms,
+ labelType.getCopyCondition().get()))
+ .append("\n");
+ }
}
return message.toString();
}
/**
- * Formats the given approvals of the same label with the given copy condition.
+ * Formats the given approvals with the given copy condition.
+ *
+ * <p>The given approvals must have the same label and the same passing and failing atoms.
*
* <p>E.g.: {Code-Review+1, Code-Review+2 (copy condition: "is:MIN")}
*
@@ -578,12 +639,29 @@
* "is:FOO")}
* </ul>
*
- * @param approvalsForSameLabel the approvals that should be formatted, must be for the same label
+ * @param approvalsWithSameLabelAndSamePassingAndFailingAtoms the approvals that should be
+ * formatted, must be for the same label
* @param copyCondition the copy condition of the label
* @return the formatted approvals
*/
private String formatApprovalsWithCopyCondition(
- Collection<PatchSetApproval> approvalsForSameLabel, String copyCondition) {
+ Collection<ApprovalCopier.Result.PatchSetApprovalData>
+ approvalsWithSameLabelAndSamePassingAndFailingAtoms,
+ String copyCondition) {
+ // Check that all given approvals have the same label and the same passing and failing atoms.
+ checkThatPropertyIsTheSameForAllApprovals(
+ approvalsWithSameLabelAndSamePassingAndFailingAtoms,
+ "label",
+ approvalData -> approvalData.patchSetApproval().label());
+ checkThatPropertyIsTheSameForAllApprovals(
+ approvalsWithSameLabelAndSamePassingAndFailingAtoms,
+ "passing atoms",
+ approvalData -> approvalData.passingAtoms());
+ checkThatPropertyIsTheSameForAllApprovals(
+ approvalsWithSameLabelAndSamePassingAndFailingAtoms,
+ "failing atoms",
+ approvalData -> approvalData.failingAtoms());
+
StringBuilder message = new StringBuilder();
boolean containsUserInPredicate;
@@ -591,7 +669,8 @@
containsUserInPredicate = containsUserInPredicate(copyCondition);
} catch (QueryParseException e) {
logger.atWarning().withCause(e).log("Non-parsable query condition");
- message.append(formatApprovalsAsLabelVotesList(approvalsForSameLabel));
+ message.append(
+ formatApprovalsAsLabelVotesList(approvalsWithSameLabelAndSamePassingAndFailingAtoms));
message.append(String.format(" (non-parseable copy condition: \"%s\")", copyCondition));
return message.toString();
}
@@ -618,26 +697,35 @@
// sort the approvals by their approvers name-email so that the approvers always appear in a
// deterministic order
- ImmutableList<PatchSetApproval> approvalsSortedByLabelVoteAndApprover =
- approvalsForSameLabel.stream()
- .sorted(
- comparing(
- (PatchSetApproval psa) ->
- LabelVote.create(psa.label(), psa.value()).format())
- .thenComparing(
- psa ->
- accountCache
- .getEvenIfMissing(psa.accountId())
- .account()
- .getNameEmail(anonymousCowardName)))
- .collect(toImmutableList());
+ ImmutableList<ApprovalCopier.Result.PatchSetApprovalData>
+ approvalsSortedByLabelVoteAndApprover =
+ approvalsWithSameLabelAndSamePassingAndFailingAtoms.stream()
+ .sorted(
+ comparing(
+ (ApprovalCopier.Result.PatchSetApprovalData approvalData) ->
+ LabelVote.create(
+ approvalData.patchSetApproval().label(),
+ approvalData.patchSetApproval().value())
+ .format())
+ .thenComparing(
+ approvalData ->
+ accountCache
+ .getEvenIfMissing(approvalData.patchSetApproval().accountId())
+ .account()
+ .getNameEmail(anonymousCowardName)))
+ .collect(toImmutableList());
ImmutableListMultimap<LabelVote, Account.Id> approversByLabelVote =
Multimaps.index(
approvalsSortedByLabelVoteAndApprover,
- psa -> LabelVote.create(psa.label(), psa.value()))
+ approvalData ->
+ LabelVote.create(
+ approvalData.patchSetApproval().label(),
+ approvalData.patchSetApproval().value()))
.entries().stream()
- .collect(toImmutableListMultimap(e -> e.getKey(), e -> e.getValue().accountId()));
+ .collect(
+ toImmutableListMultimap(
+ e -> e.getKey(), e -> e.getValue().patchSetApproval().accountId()));
message.append(
approversByLabelVote.asMap().entrySet().stream()
.map(
@@ -647,12 +735,64 @@
.collect(joining(", ")));
} else {
// copy condition doesn't contain a UserInPredicate
- message.append(formatApprovalsAsLabelVotesList(approvalsForSameLabel));
+ message.append(
+ formatApprovalsAsLabelVotesList(approvalsWithSameLabelAndSamePassingAndFailingAtoms));
}
- message.append(String.format(" (copy condition: \"%s\")", copyCondition));
+ ImmutableSet<String> passingAtoms =
+ !approvalsWithSameLabelAndSamePassingAndFailingAtoms.isEmpty()
+ ? approvalsWithSameLabelAndSamePassingAndFailingAtoms.iterator().next().passingAtoms()
+ : ImmutableSet.of();
+ message.append(
+ String.format(
+ " (copy condition: \"%s\")",
+ formatCopyConditionAsMarkdown(copyCondition, passingAtoms)));
return message.toString();
}
+ /** Checks that all given approvals have the same value for a given property. */
+ private void checkThatPropertyIsTheSameForAllApprovals(
+ Collection<ApprovalCopier.Result.PatchSetApprovalData> approvals,
+ String propertyName,
+ Function<ApprovalCopier.Result.PatchSetApprovalData, ?> propertyExtractor) {
+ if (approvals.isEmpty()) {
+ return;
+ }
+
+ Object propertyOfFirstEntry = propertyExtractor.apply(approvals.iterator().next());
+ approvals.forEach(
+ approvalData ->
+ checkState(
+ propertyExtractor.apply(approvalData).equals(propertyOfFirstEntry),
+ "property %s of approval %s does not match, expected value: %s",
+ propertyName,
+ approvalData,
+ propertyOfFirstEntry));
+ }
+
+ /**
+ * Formats the given copy condition as a Markdown string.
+ *
+ * <p>Passing atoms are formatted as bold.
+ *
+ * @param copyCondition the copy condition that should be formatted
+ * @param passingAtoms atoms of the copy conditions which are passing/matching
+ * @return the formatted copy condition as a Markdown string
+ */
+ private String formatCopyConditionAsMarkdown(
+ String copyCondition, ImmutableSet<String> passingAtoms) {
+ StringBuilder formattedCopyCondition = new StringBuilder();
+ StringTokenizer tokenizer = new StringTokenizer(copyCondition, " ()", /* returnDelims= */ true);
+ while (tokenizer.hasMoreTokens()) {
+ String token = tokenizer.nextToken();
+ if (passingAtoms.contains(token)) {
+ formattedCopyCondition.append("**" + token.replace("*", "\\*") + "**");
+ } else {
+ formattedCopyCondition.append(token);
+ }
+ }
+ return formattedCopyCondition.toString();
+ }
+
private boolean containsUserInPredicate(String copyCondition) throws QueryParseException {
// Use a request context to run checks as an internal user with expanded visibility. This is
// so that the output of the copy condition does not depend on who is running the current
@@ -675,8 +815,9 @@
* @return the given approvals as a comma-separated list of label votes
*/
private String formatApprovalsAsLabelVotesList(
- Collection<PatchSetApproval> sortedApprovalsForSameLabel) {
+ Collection<ApprovalCopier.Result.PatchSetApprovalData> sortedApprovalsForSameLabel) {
return sortedApprovalsForSameLabel.stream()
+ .map(ApprovalCopier.Result.PatchSetApprovalData::patchSetApproval)
.map(psa -> LabelVote.create(psa.label(), psa.value()))
.distinct()
.map(LabelVote::format)
diff --git a/java/com/google/gerrit/server/change/LabelNormalizer.java b/java/com/google/gerrit/server/change/LabelNormalizer.java
index fb6b177..b1fcf48 100644
--- a/java/com/google/gerrit/server/change/LabelNormalizer.java
+++ b/java/com/google/gerrit/server/change/LabelNormalizer.java
@@ -21,6 +21,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelType;
@@ -131,6 +132,20 @@
return Result.create(unchanged, updated, deleted);
}
+ /**
+ * Returns a copy of the given approval normalized to the defined ranges for the label type. If
+ * the approval is for an unknown label {@link Optional#empty()} is returned
+ *
+ * @param notes change notes containing the given approval
+ * @param approval approval that should be normalized
+ */
+ public Optional<PatchSetApproval> normalize(ChangeNotes notes, PatchSetApproval approval) {
+ Result result = normalize(notes, ImmutableSet.of(approval));
+ return Optional.ofNullable(
+ Iterables.getFirst(
+ result.unchanged(), Iterables.getFirst(result.updated(), /* defaultValue= */ null)));
+ }
+
private PatchSetApproval applyTypeFloor(LabelType lt, PatchSetApproval a) {
PatchSetApproval.Builder b = a.toBuilder();
LabelValue atMin = lt.getMin();
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index f7bec1c0..1fe67af 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
@@ -368,7 +369,9 @@
ctx,
patchSet,
mailMessage,
- approvalCopierResult.outdatedApprovals(),
+ approvalCopierResult.outdatedApprovals().stream()
+ .map(ApprovalCopier.Result.PatchSetApprovalData::patchSetApproval)
+ .collect(toImmutableSet()),
oldReviewers.byState(REVIEWER),
oldReviewers.byState(CC),
changeKind,
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 6e5cfff..0e17342 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -510,7 +510,9 @@
ctx,
newPatchSet,
mailMessage,
- approvalCopierResult.outdatedApprovals(),
+ approvalCopierResult.outdatedApprovals().stream()
+ .map(ApprovalCopier.Result.PatchSetApprovalData::patchSetApproval)
+ .collect(toImmutableSet()),
Streams.concat(
oldRecipients.getReviewers().stream(),
reviewerAdditions.flattenResults(ReviewerOp.Result::addedReviewers).stream()
@@ -595,6 +597,7 @@
return Optional.of(
"The following approvals got outdated and were removed:\n"
+ approvalCopierResult.outdatedApprovals().stream()
+ .map(ApprovalCopier.Result.PatchSetApprovalData::patchSetApproval)
.map(
outdatedApproval ->
String.format(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/CopiedApprovalsInChangeMessageIT.java b/javatests/com/google/gerrit/acceptance/api/change/CopiedApprovalsInChangeMessageIT.java
index f8cf5fd..2b1bef0 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/CopiedApprovalsInChangeMessageIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/CopiedApprovalsInChangeMessageIT.java
@@ -45,12 +45,29 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.server.approval.ApprovalCopier;
import com.google.gerrit.server.approval.ApprovalsUtil;
+import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.util.AccountTemplateUtil;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import org.junit.Test;
+/**
+ * Tests to verify that copied/outdated approvals are included into the change message that is
+ * posted on patch set creation. Includes verifying that the copied/outdated approvals in the change
+ * message are correctly formatted.
+ *
+ * <p>Some of the tests only verify the correct formatting of the copied/outdated approvals in the
+ * change message that is done by {@link
+ * ApprovalsUtil#formatApprovalCopierResult(com.google.gerrit.server.approval.ApprovalCopier.Result,
+ * LabelTypes)}. This method does the formatting based on the inputs that it gets, but it doesn't do
+ * any verification of these inputs. This means it's possible to provide inputs that are
+ * inconsistent with the approval copying logic in {@link ApprovalCopier}. E.g. it's possible to
+ * provide "is:MAX" as a passing atom for a "Code-Review-1" vote and have "is:MAX" highlighted as
+ * passing in the message although the "Code-Review-1" vote doesn't match with "is:MAX". For easier
+ * readability the formatting tests avoid using such inconsistent input data, but it's not
+ * impossible that in some cases we made a mistake and the input data is inconsistent.
+ */
public class CopiedApprovalsInChangeMessageIT extends AbstractDaemonTest {
@Inject private ApprovalsUtil approvalsUtil;
@Inject private ProjectOperations projectOperations;
@@ -98,7 +115,11 @@
PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1 (label type is missing)\n");
@@ -111,7 +132,11 @@
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())));
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Outdated Votes:\n* Code-Review+1 (label type is missing)\n");
}
@@ -125,7 +150,11 @@
PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1\n");
@@ -141,7 +170,11 @@
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())));
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Outdated Votes:\n* Code-Review+1\n");
}
@@ -153,13 +186,17 @@
ImmutableList.of(
createLabelType(
/* labelName= */ "Code-Review", /* copyCondition= */ "is:MIN OR is:MAX")));
- PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
+ PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", -2);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of("is:MIN"),
+ /* failingAtoms= */ ImmutableSet.of("is:MAX"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
- .hasValue("Copied Votes:\n* Code-Review+1 (copy condition: \"is:MIN OR is:MAX\")\n");
+ .hasValue("Copied Votes:\n* Code-Review-2 (copy condition: \"**is:MIN** OR is:MAX\")\n");
}
@Test
@@ -168,14 +205,21 @@
new LabelTypes(
ImmutableList.of(
createLabelType(
- /* labelName= */ "Code-Review", /* copyCondition= */ "is:MIN OR is:MAX")));
- PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
+ /* labelName= */ "Code-Review",
+ /* copyCondition= */ "changekind:TRIVIAL_REBASE is:MAX")));
+ PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 2);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of("is:MAX"),
+ /* failingAtoms= */ ImmutableSet.of("changekind:TRIVIAL_REBASE"))));
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
- .hasValue("Outdated Votes:\n* Code-Review+1 (copy condition: \"is:MIN OR is:MAX\")\n");
+ .hasValue(
+ "Outdated Votes:\n* Code-Review+2 (copy condition:"
+ + " \"changekind:TRIVIAL_REBASE **is:MAX**\")\n");
}
@Test
@@ -189,7 +233,11 @@
PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -208,7 +256,11 @@
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())));
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
"Outdated Votes:\n* Code-Review+1 (non-parseable copy condition: \"foo bar baz\")\n");
@@ -225,17 +277,22 @@
/* labelName= */ "Code-Review",
/* copyCondition= */ String.format(
"is:MIN OR (is:MAX approverin:%s)", groupUuid))));
- PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
+ PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 2);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
"Copied Votes:\n"
- + "* Code-Review+1 by %s"
- + " (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + "* Code-Review+2 by %s"
+ + " (copy condition: \"is:MIN OR (**is:MAX** **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(admin.id()), groupUuid));
}
@@ -254,12 +311,17 @@
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN", "is:MAX"))));
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
"Outdated Votes:\n"
- + "* Code-Review+1 by %s (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + "* Code-Review+1 by %s (copy condition: \"is:MIN"
+ + " OR (is:MAX **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(admin.id()), groupUuid));
}
@@ -275,10 +337,15 @@
/* labelName= */ "Code-Review",
/* copyCondition= */ String.format(
"is:MIN OR (is:MAX approverin:%s)", groupUuid))));
- PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
+ PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 2);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
// Set 'user' as the current user in the request scope.
@@ -291,8 +358,8 @@
.hasValue(
String.format(
"Copied Votes:\n"
- + "* Code-Review+1 by %s"
- + " (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + "* Code-Review+2 by %s"
+ + " (copy condition: \"is:MIN OR (**is:MAX** **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(admin.id()), groupUuid));
}
@@ -313,7 +380,11 @@
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN", "is:MAX"))));
// Set 'user' as the current user in the request scope.
// 'user' cannot see the Administrators group that is used in the copy condition.
@@ -325,7 +396,8 @@
.hasValue(
String.format(
"Outdated Votes:\n"
- + "* Code-Review+1 by %s (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + "* Code-Review+1 by %s (copy condition: \"is:MIN"
+ + " OR (is:MAX **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(admin.id()), groupUuid));
}
@@ -344,7 +416,11 @@
PatchSetApproval patchSetApproval = createPatchSetApproval(admin, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -371,7 +447,11 @@
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(),
- /* outdatedApprovals= */ ImmutableSet.of(patchSetApproval));
+ /* outdatedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())));
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
@@ -388,7 +468,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1 (label type is missing)\n");
@@ -401,7 +489,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Verified", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -417,7 +513,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1, Code-Review+2 (label type is missing)\n");
@@ -433,7 +537,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1\n");
@@ -450,7 +562,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Verified", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1\n* Verified+1\n");
@@ -466,7 +586,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue("Copied Votes:\n* Code-Review+1, Code-Review+2\n");
@@ -480,14 +608,22 @@
ImmutableList.of(
createLabelType(
/* labelName= */ "Code-Review", /* copyCondition= */ "is:MIN OR is:MAX")));
- PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 1);
- PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
+ PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 2);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of("is:MAX"),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of("is:MAX"),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
- .hasValue("Copied Votes:\n* Code-Review+1 (copy condition: \"is:MIN OR is:MAX\")\n");
+ .hasValue("Copied Votes:\n* Code-Review+2 (copy condition: \"is:MIN OR **is:MAX**\")\n");
}
@Test
@@ -498,39 +634,92 @@
ImmutableList.of(
createLabelType(
/* labelName= */ "Code-Review", /* copyCondition= */ "is:MIN OR is:MAX"),
- createLabelType(
- /* labelName= */ "Verified", /* copyCondition= */ "is:MIN OR is:MAX")));
- PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 1);
+ LabelType.builder(
+ "Verified",
+ ImmutableList.of(
+ LabelValue.create((short) -1, "Fails"),
+ LabelValue.create((short) 0, "No Vote"),
+ LabelValue.create((short) 1, "Succeeds")))
+ .setCopyCondition("is:MIN OR is:MAX")
+ .build()));
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Verified", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of("is:MAX"),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of("is:MAX"),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
"Copied Votes:\n"
- + "* Code-Review+1 (copy condition: \"is:MIN OR is:MAX\")\n"
- + "* Verified+1 (copy condition: \"is:MIN OR is:MAX\")\n");
+ + "* Code-Review+2 (copy condition: \"is:MIN OR **is:MAX**\")\n"
+ + "* Verified+1 (copy condition: \"is:MIN OR **is:MAX**\")\n");
}
@Test
- public void formatMultipleApprovals_differentValue_withCopyCondition_noUserInPredicate()
- throws Exception {
+ public void
+ formatMultipleApprovals_differentValue_withCopyCondition_noUserInPredicate_samePassingAtoms()
+ throws Exception {
LabelTypes labelTypes =
new LabelTypes(
ImmutableList.of(
createLabelType(
- /* labelName= */ "Code-Review", /* copyCondition= */ "is:MIN OR is:MAX")));
+ /* labelName= */ "Code-Review", /* copyCondition= */ "changekind:REWORK")));
PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of("changekind:REWORK"),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of("changekind:REWORK"),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
"Copied Votes:\n"
- + "* Code-Review+1, Code-Review+2 (copy condition: \"is:MIN OR is:MAX\")\n");
+ + "* Code-Review+1, Code-Review+2 (copy condition: \"**changekind:REWORK**\")\n");
+ }
+
+ @Test
+ public void
+ formatMultipleApprovals_differentValue_withCopyCondition_noUserInPredicate_differentPassingAtoms()
+ throws Exception {
+ LabelTypes labelTypes =
+ new LabelTypes(
+ ImmutableList.of(
+ createLabelType(
+ /* labelName= */ "Code-Review", /* copyCondition= */ "is:1 OR is:2")));
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
+ PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
+ ApprovalCopier.Result approvalCopierResult =
+ ApprovalCopier.Result.create(
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of("is:2"),
+ /* failingAtoms= */ ImmutableSet.of("is:1")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of("is:1"),
+ /* failingAtoms= */ ImmutableSet.of("is:2"))),
+ /* outdatedApprovals= */ ImmutableSet.of());
+ assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
+ .hasValue(
+ "Copied Votes:\n"
+ + "* Code-Review+1 (copy condition: \"**is:1** OR is:2\")\n"
+ + "* Code-Review+2 (copy condition: \"is:1 OR **is:2**\")\n");
}
@Test
@@ -545,7 +734,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -565,7 +762,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Verified", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -587,7 +792,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -597,8 +810,9 @@
}
@Test
- public void formatMultipleApprovals_sameVote_withCopyCondition_withUserInPredicate()
- throws Exception {
+ public void
+ formatMultipleApprovals_sameVote_withCopyCondition_withUserInPredicate_samePassingAtoms()
+ throws Exception {
String groupUuid =
groupCache.get(AccountGroup.nameKey("Administrators")).get().getGroupUUID().get();
LabelTypes labelTypes =
@@ -608,24 +822,86 @@
/* labelName= */ "Code-Review",
/* copyCondition= */ String.format(
"is:MIN OR (is:MAX approverin:%s)", groupUuid))));
- PatchSetApproval patchSetApproval1 = createPatchSetApproval(user, "Code-Review", 1);
- PatchSetApproval patchSetApproval2 = createPatchSetApproval(admin, "Code-Review", 1);
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(user, "Code-Review", 2);
+ PatchSetApproval patchSetApproval2 = createPatchSetApproval(admin, "Code-Review", 2);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
"Copied Votes:\n"
- + "* Code-Review+1 by %s, %s"
- + " (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + "* Code-Review+2 by %s, %s"
+ + " (copy condition: \"is:MIN OR (**is:MAX** **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(admin.id()),
AccountTemplateUtil.getAccountTemplate(user.id()),
groupUuid));
}
@Test
+ public void
+ formatMultipleApprovals_sameVote_withCopyCondition_withUserInPredicate_differentPassingAtoms()
+ throws Exception {
+ String administratorsGroupUuid =
+ groupCache.get(AccountGroup.nameKey("Administrators")).get().getGroupUUID().get();
+ String registeredUsersGroupUuid = SystemGroupBackend.REGISTERED_USERS.get();
+ LabelTypes labelTypes =
+ new LabelTypes(
+ ImmutableList.of(
+ createLabelType(
+ /* labelName= */ "Code-Review",
+ /* copyCondition= */ String.format(
+ "is:MIN OR (is:MAX approverin:%s) OR (is:MAX approverin:%s)",
+ administratorsGroupUuid, registeredUsersGroupUuid))));
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(user, "Code-Review", 2);
+ PatchSetApproval patchSetApproval2 = createPatchSetApproval(admin, "Code-Review", 2);
+ ApprovalCopier.Result approvalCopierResult =
+ ApprovalCopier.Result.create(
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", registeredUsersGroupUuid)),
+ /* failingAtoms= */ ImmutableSet.of(
+ "is:MIN", String.format("approverin:%s", administratorsGroupUuid))),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX",
+ String.format("approverin:%s", administratorsGroupUuid),
+ String.format("approverin:%s", registeredUsersGroupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
+ /* outdatedApprovals= */ ImmutableSet.of());
+ assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
+ .hasValue(
+ String.format(
+ "Copied Votes:\n"
+ + "* Code-Review+2 by %s"
+ + " (copy condition: \"is:MIN OR (**is:MAX** **approverin:%s**)"
+ + " OR (**is:MAX** **approverin:%s**)\")\n"
+ + "* Code-Review+2 by %s"
+ + " (copy condition: \"is:MIN OR (**is:MAX** approverin:%s)"
+ + " OR (**is:MAX** **approverin:%s**)\")\n",
+ AccountTemplateUtil.getAccountTemplate(admin.id()),
+ administratorsGroupUuid,
+ registeredUsersGroupUuid,
+ AccountTemplateUtil.getAccountTemplate(user.id()),
+ administratorsGroupUuid,
+ registeredUsersGroupUuid));
+ }
+
+ @Test
public void formatMultipleApprovals_differentLabel_withCopyCondition_withUserInPredicate()
throws Exception {
String groupUuid =
@@ -641,18 +917,30 @@
/* labelName= */ "Verified",
/* copyCondition= */ String.format(
"is:MIN OR (is:MAX approverin:%s)", groupUuid))));
- PatchSetApproval patchSetApproval1 = createPatchSetApproval(user, "Code-Review", 1);
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(user, "Code-Review", -2);
PatchSetApproval patchSetApproval2 = createPatchSetApproval(admin, "Verified", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of("is:MIN"),
+ /* failingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", groupUuid))),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:MAX", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
"Copied Votes:\n"
- + "* Code-Review+1 by %s (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n"
- + "* Verified+1 by %s (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + "* Code-Review-2 by %s (copy condition: \"**is:MIN**"
+ + " OR (is:MAX approverin:%s)\")\n"
+ + "* Verified+1 by %s (copy condition: \"is:MIN"
+ + " OR (**is:MAX** **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(user.id()),
groupUuid,
AccountTemplateUtil.getAccountTemplate(admin.id()),
@@ -660,8 +948,9 @@
}
@Test
- public void formatMultipleApprovals_differentValue_withCopyCondition_withUserInPredicate()
- throws Exception {
+ public void
+ formatMultipleApprovals_differentValue_withCopyCondition_withUserInPredicate_samePassingAtoms()
+ throws Exception {
String groupUuid =
groupCache.get(AccountGroup.nameKey("Administrators")).get().getGroupUUID().get();
LabelTypes labelTypes =
@@ -670,51 +959,122 @@
createLabelType(
/* labelName= */ "Code-Review",
/* copyCondition= */ String.format(
- "is:MIN OR (is:MAX approverin:%s)", groupUuid))));
+ "is:MIN OR (is:ANY approverin:%s)", groupUuid))));
PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:ANY", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:ANY", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
"Copied Votes:\n"
+ "* Code-Review+1 by %s, Code-Review+2 by %s"
- + " (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + " (copy condition: \"is:MIN OR (**is:ANY** **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(user.id()),
AccountTemplateUtil.getAccountTemplate(admin.id()),
groupUuid));
}
@Test
+ public void
+ formatMultipleApprovals_differentValue_withCopyCondition_withUserInPredicate_differentPassingAtoms()
+ throws Exception {
+ String groupUuid =
+ groupCache.get(AccountGroup.nameKey("Administrators")).get().getGroupUUID().get();
+ LabelTypes labelTypes =
+ new LabelTypes(
+ ImmutableList.of(
+ createLabelType(
+ /* labelName= */ "Code-Review",
+ /* copyCondition= */ String.format(
+ "is:MIN OR (is:1 approverin:%s) OR (is:2 approverin:%s)",
+ groupUuid, groupUuid))));
+ PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
+ PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
+ ApprovalCopier.Result approvalCopierResult =
+ ApprovalCopier.Result.create(
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:2", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN", "is:1")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:1", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN", "is:2"))),
+ /* outdatedApprovals= */ ImmutableSet.of());
+ assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
+ .hasValue(
+ String.format(
+ "Copied Votes:\n"
+ + "* Code-Review+1 by %s"
+ + " (copy condition: \"is:MIN OR (**is:1** **approverin:%s**)"
+ + " OR (is:2 **approverin:%s**)\")\n"
+ + "* Code-Review+2 by %s"
+ + " (copy condition: \"is:MIN OR (is:1 **approverin:%s**)"
+ + " OR (**is:2** **approverin:%s**)\")\n",
+ AccountTemplateUtil.getAccountTemplate(user.id()),
+ groupUuid,
+ groupUuid,
+ AccountTemplateUtil.getAccountTemplate(admin.id()),
+ groupUuid,
+ groupUuid));
+ }
+
+ @Test
public void formatMultipleApprovals_differentAndSameValue_withCopyCondition_withUserInPredicate()
throws Exception {
TestAccount user2 = accountCreator.user2();
- String groupUuid =
- groupCache.get(AccountGroup.nameKey("Administrators")).get().getGroupUUID().get();
+ String groupUuid = SystemGroupBackend.REGISTERED_USERS.get();
LabelTypes labelTypes =
new LabelTypes(
ImmutableList.of(
createLabelType(
/* labelName= */ "Code-Review",
/* copyCondition= */ String.format(
- "is:MIN OR (is:MAX approverin:%s)", groupUuid))));
+ "is:MIN OR (is:ANY approverin:%s)", groupUuid))));
PatchSetApproval patchSetApproval1 = createPatchSetApproval(admin, "Code-Review", 2);
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user2, "Code-Review", 1);
PatchSetApproval patchSetApproval3 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
/* copiedApprovals= */ ImmutableSet.of(
- patchSetApproval1, patchSetApproval2, patchSetApproval3),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:ANY", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:ANY", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN")),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval3,
+ /* passingAtoms= */ ImmutableSet.of(
+ "is:ANY", String.format("approverin:%s", groupUuid)),
+ /* failingAtoms= */ ImmutableSet.of("is:MIN"))),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
String.format(
"Copied Votes:\n"
+ "* Code-Review+1 by %s, %s, Code-Review+2 by %s"
- + " (copy condition: \"is:MIN OR (is:MAX approverin:%s)\")\n",
+ + " (copy condition: \"is:MIN OR (**is:ANY** **approverin:%s**)\")\n",
AccountTemplateUtil.getAccountTemplate(user.id()),
AccountTemplateUtil.getAccountTemplate(user2.id()),
AccountTemplateUtil.getAccountTemplate(admin.id()),
@@ -737,7 +1097,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -769,7 +1137,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Verified", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -799,7 +1175,15 @@
PatchSetApproval patchSetApproval2 = createPatchSetApproval(user, "Code-Review", 1);
ApprovalCopier.Result approvalCopierResult =
ApprovalCopier.Result.create(
- /* copiedApprovals= */ ImmutableSet.of(patchSetApproval1, patchSetApproval2),
+ /* copiedApprovals= */ ImmutableSet.of(
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval1,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of()),
+ ApprovalCopier.Result.PatchSetApprovalData.create(
+ patchSetApproval2,
+ /* passingAtoms= */ ImmutableSet.of(),
+ /* failingAtoms= */ ImmutableSet.of())),
/* outdatedApprovals= */ ImmutableSet.of());
assertThat(approvalsUtil.formatApprovalCopierResult(approvalCopierResult, labelTypes))
.hasValue(
@@ -849,7 +1233,7 @@
+ "\n"
+ "Copied Votes:\n"
+ "* Code-Review-2 (copy condition: \"changekind:NO_CHANGE"
- + " OR changekind:TRIVIAL_REBASE OR is:MIN\")\n"
+ + " OR changekind:TRIVIAL_REBASE OR **is:MIN**\")\n"
+ "\n"
+ "Outdated Votes:\n"
+ "* Verified+1\n");
@@ -900,7 +1284,7 @@
+ "\n"
+ "Copied Votes:\n"
+ "* Code-Review-2 (copy condition: \"changekind:NO_CHANGE"
- + " OR changekind:TRIVIAL_REBASE OR is:MIN\")\n"
+ + " OR changekind:TRIVIAL_REBASE OR **is:MIN**\")\n"
+ "\n"
+ "Outdated Votes:\n"
+ "* Verified+1\n");
@@ -946,7 +1330,7 @@
+ "\n"
+ "Copied Votes:\n"
+ "* Code-Review-2 (copy condition: \"changekind:NO_CHANGE"
- + " OR changekind:TRIVIAL_REBASE OR is:MIN\")\n"
+ + " OR changekind:TRIVIAL_REBASE OR **is:MIN**\")\n"
+ "\n"
+ "Outdated Votes:\n"
+ "* Verified+1\n");
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java b/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java
index 0d06946..379a712 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java
@@ -17,18 +17,23 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertAbout;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.server.change.ApprovalCopierIT.PatchSetApprovalSubject.assertThatList;
-import static com.google.gerrit.acceptance.server.change.ApprovalCopierIT.PatchSetApprovalSubject.hasTestId;
+import static com.google.gerrit.acceptance.server.change.ApprovalCopierIT.ApprovalDataSubject.assertThat;
+import static com.google.gerrit.acceptance.server.change.ApprovalCopierIT.ApprovalDataSubject.assertThatList;
+import static com.google.gerrit.acceptance.server.change.ApprovalCopierIT.ApprovalDataSubject.hasTestId;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
import static com.google.gerrit.server.project.testing.TestLabels.value;
+import static com.google.gerrit.truth.ListSubject.elements;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;
import com.google.common.truth.Correspondence;
import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.StandardSubjectBuilder;
+import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
+import com.google.common.truth.Truth8;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -43,6 +48,7 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.approval.ApprovalCopier;
import com.google.gerrit.server.query.change.ChangeData;
@@ -50,6 +56,7 @@
import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
import java.io.IOException;
+import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import org.eclipse.jgit.lib.Repository;
@@ -71,6 +78,24 @@
@Before
public void setup() throws Exception {
+ // Overwrite "Code-Review" label that is inherited from All-Projects.
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ LabelType.Builder codeReview =
+ labelBuilder(
+ LabelId.CODE_REVIEW,
+ value(2, "Looks good to me, approved"),
+ value(1, "Looks good to me, but someone else must approve"),
+ value(0, "No score"),
+ value(-1, "I would prefer this is not submitted as is"),
+ value(-2, "This shall not be submitted"))
+ .setCopyCondition(
+ String.format(
+ "changekind:%s OR changekind:%s OR is:MIN",
+ ChangeKind.NO_CHANGE, ChangeKind.TRIVIAL_REBASE.name()));
+ u.getConfig().upsertLabelType(codeReview.build());
+ u.save();
+ }
+
// Add Verified label.
try (ProjectConfigUpdate u = updateProject(project)) {
LabelType.Builder verified =
@@ -153,6 +178,18 @@
.containsExactly(
PatchSetApprovalTestId.create(patchSet1Id, admin.id(), LabelId.CODE_REVIEW, 2),
PatchSetApprovalTestId.create(patchSet1Id, user.id(), LabelId.VERIFIED, 1));
+
+ ApprovalDataSubject codeReviewApprovalSubject =
+ assertThat(approvalCopierResult.outdatedApprovals(), LabelId.CODE_REVIEW, admin.id());
+ codeReviewApprovalSubject.hasPassingAtomsThat().isEmpty();
+ codeReviewApprovalSubject
+ .hasFailingAtomsThat()
+ .containsExactly("changekind:NO_CHANGE", "changekind:TRIVIAL_REBASE", "is:MIN");
+
+ ApprovalDataSubject verifiedApprovalSubject =
+ assertThat(approvalCopierResult.outdatedApprovals(), LabelId.VERIFIED, user.id());
+ verifiedApprovalSubject.hasPassingAtomsThat().isEmpty();
+ verifiedApprovalSubject.hasFailingAtomsThat().containsExactly("is:MIN");
}
@Test
@@ -176,6 +213,18 @@
PatchSetApprovalTestId.create(patchSet2Id, admin.id(), LabelId.CODE_REVIEW, -2),
PatchSetApprovalTestId.create(patchSet2Id, user.id(), LabelId.VERIFIED, -1));
assertThatList(approvalCopierResult.outdatedApprovals()).isEmpty();
+
+ ApprovalDataSubject codeReviewApprovalSubject =
+ assertThat(approvalCopierResult.copiedApprovals(), LabelId.CODE_REVIEW, admin.id());
+ codeReviewApprovalSubject.hasPassingAtomsThat().containsExactly("is:MIN");
+ codeReviewApprovalSubject
+ .hasFailingAtomsThat()
+ .containsExactly("changekind:NO_CHANGE", "changekind:TRIVIAL_REBASE");
+
+ ApprovalDataSubject verifiedApprovalSubject =
+ assertThat(approvalCopierResult.copiedApprovals(), LabelId.VERIFIED, user.id());
+ verifiedApprovalSubject.hasPassingAtomsThat().containsExactly("is:MIN");
+ verifiedApprovalSubject.hasFailingAtomsThat().isEmpty();
}
@Test
@@ -230,6 +279,30 @@
.containsExactly(
PatchSetApprovalTestId.create(patchSet1Id, user.id(), LabelId.CODE_REVIEW, 1),
PatchSetApprovalTestId.create(patchSet1Id, admin.id(), LabelId.VERIFIED, 1));
+
+ ApprovalDataSubject copiedCodeReviewApprovalSubject =
+ assertThat(approvalCopierResult.copiedApprovals(), LabelId.CODE_REVIEW, admin.id());
+ copiedCodeReviewApprovalSubject.hasPassingAtomsThat().containsExactly("is:MIN");
+ copiedCodeReviewApprovalSubject
+ .hasFailingAtomsThat()
+ .containsExactly("changekind:NO_CHANGE", "changekind:TRIVIAL_REBASE");
+
+ ApprovalDataSubject copiedVerifiedApprovalSubject =
+ assertThat(approvalCopierResult.copiedApprovals(), LabelId.VERIFIED, user.id());
+ copiedVerifiedApprovalSubject.hasPassingAtomsThat().containsExactly("is:MIN");
+ copiedVerifiedApprovalSubject.hasFailingAtomsThat().isEmpty();
+
+ ApprovalDataSubject outdatedCodeReviewApprovalSubject1 =
+ assertThat(approvalCopierResult.outdatedApprovals(), LabelId.CODE_REVIEW, user.id());
+ outdatedCodeReviewApprovalSubject1.hasPassingAtomsThat().isEmpty();
+ outdatedCodeReviewApprovalSubject1
+ .hasFailingAtomsThat()
+ .containsExactly("changekind:NO_CHANGE", "changekind:TRIVIAL_REBASE", "is:MIN");
+
+ ApprovalDataSubject outdatedVerifiedApprovalSubject1 =
+ assertThat(approvalCopierResult.outdatedApprovals(), LabelId.VERIFIED, admin.id());
+ outdatedVerifiedApprovalSubject1.hasPassingAtomsThat().isEmpty();
+ outdatedVerifiedApprovalSubject1.hasFailingAtomsThat().containsExactly("is:MIN");
}
@Test
@@ -275,6 +348,11 @@
.comparingElementsUsing(hasTestId())
.containsExactly(
PatchSetApprovalTestId.create(patchSet1Id, admin.id(), LabelId.CODE_REVIEW, -2));
+
+ ApprovalDataSubject codeReviewApprovalSubject1 =
+ assertThat(approvalCopierResult.outdatedApprovals(), LabelId.CODE_REVIEW, admin.id());
+ codeReviewApprovalSubject1.hasPassingAtomsThat().isEmpty();
+ codeReviewApprovalSubject1.hasFailingAtomsThat().isEmpty();
}
@Test
@@ -347,12 +425,14 @@
ApprovalCopier.Result approvalCopierResult =
invokeApprovalCopierForCurrentPatchSet(
r.getChange().getId(), /* expectedCurrentPatchSetNum= */ 2);
- ImmutableSet<PatchSetApproval> copiedApprovals = approvalCopierResult.copiedApprovals();
- assertThatList(filter(copiedApprovals, PatchSetApproval::copied))
+ ImmutableSet<ApprovalCopier.Result.PatchSetApprovalData> copiedApprovals =
+ approvalCopierResult.copiedApprovals();
+ assertThatList(filter(copiedApprovals, approval -> approval.patchSetApproval().copied()))
.comparingElementsUsing(hasTestId())
.containsExactly(
PatchSetApprovalTestId.create(patchSet2Id, user.id(), LabelId.VERIFIED, -1));
- assertThatList(filter(copiedApprovals, psa -> !psa.copied())).isEmpty();
+ assertThatList(filter(copiedApprovals, approval -> !approval.patchSetApproval().copied()))
+ .isEmpty();
}
private void vote(String changeId, TestAccount testAccount, String label, int value)
@@ -362,8 +442,9 @@
requestScopeOperations.setApiUser(admin.id());
}
- private ImmutableSet<PatchSetApproval> filter(
- Set<PatchSetApproval> approvals, Predicate<PatchSetApproval> filter) {
+ private ImmutableSet<ApprovalCopier.Result.PatchSetApprovalData> filter(
+ Set<ApprovalCopier.Result.PatchSetApprovalData> approvals,
+ Predicate<ApprovalCopier.Result.PatchSetApprovalData> filter) {
return approvals.stream().filter(filter).collect(toImmutableSet());
}
@@ -378,20 +459,75 @@
}
}
- public static class PatchSetApprovalSubject extends Subject {
- public static Correspondence<PatchSetApproval, PatchSetApprovalTestId> hasTestId() {
- return NullAwareCorrespondence.transforming(PatchSetApprovalTestId::create, "has test ID");
+ public static class ApprovalDataSubject extends Subject {
+ public static Correspondence<ApprovalCopier.Result.PatchSetApprovalData, PatchSetApprovalTestId>
+ hasTestId() {
+ return NullAwareCorrespondence.transforming(
+ approvalData -> PatchSetApprovalTestId.create(approvalData.patchSetApproval()),
+ "has test ID");
}
+ public static ApprovalDataSubject assertThat(
+ ApprovalCopier.Result.PatchSetApprovalData approvalData) {
+ return assertAbout(approvalDatas()).that(approvalData);
+ }
+
+ public static ApprovalDataSubject assertThat(
+ ImmutableSet<ApprovalCopier.Result.PatchSetApprovalData> approvalDatas,
+ String labelId,
+ Account.Id accountId) {
+ Optional<ApprovalCopier.Result.PatchSetApprovalData> approvalDataForLabelAndAccount =
+ approvalDatas.stream()
+ .filter(
+ approvalData ->
+ approvalData.patchSetApproval().label().equals(labelId)
+ && approvalData.patchSetApproval().accountId().equals(accountId))
+ .findAny();
+ Truth8.assertThat(approvalDataForLabelAndAccount).isPresent();
+ return assertAbout(approvalDatas()).that(approvalDataForLabelAndAccount.get());
+ }
+
+ public static ListSubject<ApprovalDataSubject, ApprovalCopier.Result.PatchSetApprovalData>
+ assertThatList(ImmutableSet<ApprovalCopier.Result.PatchSetApprovalData> approvalDatas) {
+ return ListSubject.assertThat(approvalDatas.asList(), approvalDatas());
+ }
+
+ private static Factory<ApprovalDataSubject, ApprovalCopier.Result.PatchSetApprovalData>
+ approvalDatas() {
+ return ApprovalDataSubject::new;
+ }
+
+ private final ApprovalCopier.Result.PatchSetApprovalData approvalData;
+
+ private ApprovalDataSubject(
+ FailureMetadata metadata, ApprovalCopier.Result.PatchSetApprovalData approvalData) {
+ super(metadata, approvalData);
+ this.approvalData = approvalData;
+ }
+
+ public ListSubject<StringSubject, String> hasPassingAtomsThat() {
+ return check("passingAtoms()")
+ .about(elements())
+ .that(approvalData().passingAtoms().asList(), StandardSubjectBuilder::that);
+ }
+
+ public ListSubject<StringSubject, String> hasFailingAtomsThat() {
+ return check("failingAtoms()")
+ .about(elements())
+ .that(approvalData().failingAtoms().asList(), StandardSubjectBuilder::that);
+ }
+
+ private ApprovalCopier.Result.PatchSetApprovalData approvalData() {
+ isNotNull();
+ return approvalData;
+ }
+ }
+
+ public static class PatchSetApprovalSubject extends Subject {
public static PatchSetApprovalSubject assertThat(PatchSetApproval patchSetApproval) {
return assertAbout(patchSetApprovals()).that(patchSetApproval);
}
- public static ListSubject<PatchSetApprovalSubject, PatchSetApproval> assertThatList(
- ImmutableSet<PatchSetApproval> patchSetApprovals) {
- return ListSubject.assertThat(patchSetApprovals.asList(), patchSetApprovals());
- }
-
private static Factory<PatchSetApprovalSubject, PatchSetApproval> patchSetApprovals() {
return PatchSetApprovalSubject::new;
}
