Merge "Update javadoc for ProjectState"
diff --git a/Documentation/linux-quickstart.txt b/Documentation/linux-quickstart.txt
index 29bb409..e34071f 100644
--- a/Documentation/linux-quickstart.txt
+++ b/Documentation/linux-quickstart.txt
@@ -19,8 +19,7 @@
. A Unix-based server, including any Linux flavor, MacOS, or Berkeley Software
Distribution (BSD).
-. Java SE Runtime Environment version 1.8. Gerrit is not compatible with Java
- 9 or newer yet.
+. Java SE Runtime Environment version 11 and up.
== Download Gerrit
diff --git a/WORKSPACE b/WORKSPACE
index 0caf8c3..428a6a4 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -991,5 +991,5 @@
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "6b778a270b7529fcb9b7a6f62f3ae9d38544ce2f",
+ sha1 = "595e3a50f59cd3c1d281ca6c1bc4037e277a1353",
)
diff --git a/java/com/google/gerrit/acceptance/PushOneCommit.java b/java/com/google/gerrit/acceptance/PushOneCommit.java
index a613588..d46fb78 100644
--- a/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -282,6 +282,11 @@
return this;
}
+ public PushOneCommit noParent() throws Exception {
+ commitBuilder.noParents();
+ return this;
+ }
+
public PushOneCommit addSymlink(String path, String target) throws Exception {
RevBlob blobId = testRepo.blob(target);
commitBuilder.edit(
diff --git a/java/com/google/gerrit/entities/SubmitRequirement.java b/java/com/google/gerrit/entities/SubmitRequirement.java
index 36f7b53..df03fd5 100644
--- a/java/com/google/gerrit/entities/SubmitRequirement.java
+++ b/java/com/google/gerrit/entities/SubmitRequirement.java
@@ -29,23 +29,23 @@
/**
* Expression of the condition that makes the requirement applicable. The expression should be
* evaluated for a specific {@link Change} and if it returns false, the requirement becomes
- * irrelevant for the change (i.e. {@link #blockingExpression()} and {@link #overrideExpression()}
- * become irrelevant).
+ * irrelevant for the change (i.e. {@link #submittabilityExpression()} and {@link
+ * #overrideExpression()} become irrelevant).
*
* <p>An empty {@link Optional} indicates that the requirement is applicable for any change.
*/
public abstract Optional<SubmitRequirementExpression> applicabilityExpression();
/**
- * Expression of the condition that blocks the submission of a change. The expression should be
- * evaluated for a specific {@link Change} and if it returns false, the requirement becomes
+ * Expression of the condition that allows the submission of a change. The expression should be
+ * evaluated for a specific {@link Change} and if it returns true, the requirement becomes
* fulfilled for the change.
*/
- public abstract SubmitRequirementExpression blockingExpression();
+ public abstract SubmitRequirementExpression submittabilityExpression();
/**
* Expression that, if evaluated to true, causes the submit requirement to be fulfilled,
- * regardless of the blocking expression. This expression should be evaluated for a specific
+ * regardless of the submittability expression. This expression should be evaluated for a specific
* {@link Change}.
*
* <p>An empty {@link Optional} indicates that the requirement is not overridable.
@@ -72,7 +72,8 @@
public abstract Builder setApplicabilityExpression(
Optional<SubmitRequirementExpression> applicabilityExpression);
- public abstract Builder setBlockingExpression(SubmitRequirementExpression blockingExpression);
+ public abstract Builder setSubmittabilityExpression(
+ SubmitRequirementExpression submittabilityExpression);
public abstract Builder setOverrideExpression(
Optional<SubmitRequirementExpression> overrideExpression);
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpression.java b/java/com/google/gerrit/entities/SubmitRequirementExpression.java
index 7b31304..c978347 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementExpression.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpression.java
@@ -19,12 +19,7 @@
import com.google.gerrit.common.Nullable;
import java.util.Optional;
-/**
- * Describe a applicability, blocking or override expression of a {@link SubmitRequirement}.
- *
- * <p>TODO: Store the tree representation of the parsed expression internally and throw an exception
- * upon creation if the expression syntax is invalid.
- */
+/** Describe a applicability, blocking or override expression of a {@link SubmitRequirement}. */
@AutoValue
public abstract class SubmitRequirementExpression {
@@ -45,5 +40,5 @@
}
/** Returns the underlying String representing this {@link SubmitRequirementExpression}. */
- public abstract String expression();
+ public abstract String expressionString();
}
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
new file mode 100644
index 0000000..94c0e91
--- /dev/null
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
@@ -0,0 +1,146 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.entities;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import java.util.Optional;
+
+/** Result of evaluating a submit requirement expression on a given Change. */
+@AutoValue
+public abstract class SubmitRequirementExpressionResult {
+
+ /**
+ * Entity detailing the result of evaluating a Submit requirement expression. Contains an empty
+ * {@link Optional} if {@link #status()} is equal to {@link Status#ERROR}.
+ */
+ public abstract Optional<PredicateResult> predicateResult();
+
+ public abstract Optional<String> errorMessage();
+
+ public Status status() {
+ if (predicateResult().isPresent()) {
+ return predicateResult().get().status() ? Status.PASS : Status.FAIL;
+ }
+ return Status.ERROR;
+ }
+
+ public static SubmitRequirementExpressionResult create(PredicateResult predicateResult) {
+ return new AutoValue_SubmitRequirementExpressionResult(
+ Optional.of(predicateResult), Optional.empty());
+ }
+
+ public static SubmitRequirementExpressionResult error(String errorMessage) {
+ return new AutoValue_SubmitRequirementExpressionResult(
+ Optional.empty(), Optional.of(errorMessage));
+ }
+
+ /**
+ * Returns a list of leaf predicate results whose {@link PredicateResult#status()} is true. If
+ * {@link #status()} is equal to {@link Status#ERROR}, an empty list is returned.
+ */
+ public ImmutableList<PredicateResult> getPassingAtoms() {
+ if (predicateResult().isPresent()) {
+ return predicateResult().get().getAtoms(/* status= */ true);
+ }
+ return ImmutableList.of();
+ }
+
+ /**
+ * Returns a list of leaf predicate results whose {@link PredicateResult#status()} is false. If
+ * {@link #status()} is equal to {@link Status#ERROR}, an empty list is returned.
+ */
+ public ImmutableList<PredicateResult> getFailingAtoms() {
+ if (predicateResult().isPresent()) {
+ return predicateResult().get().getAtoms(/* status= */ false);
+ }
+ return ImmutableList.of();
+ }
+
+ public enum Status {
+ /** Submit requirement expression is fulfilled for a given change. */
+ PASS,
+
+ /** Submit requirement expression is failing for a given change. */
+ FAIL,
+
+ /** Submit requirement expression contains invalid syntax and is not parsable. */
+ ERROR
+ }
+
+ /**
+ * Entity detailing the result of evaluating a predicate.
+ *
+ * <p>Example - branch:refs/heads/foo and has:unresolved
+ *
+ * <p>The above predicate is an "And" predicate having two child predicates:
+ *
+ * <ul>
+ * <li>branch:refs/heads/foo
+ * <li>has:unresolved
+ * </ul>
+ *
+ * <p>Each child predicate as well as the parent contains the result of its evaluation.
+ */
+ @AutoValue
+ public abstract static class PredicateResult {
+ abstract ImmutableList<PredicateResult> childPredicateResults();
+
+ abstract String predicateString();
+
+ /** true if the predicate is passing for a given change. */
+ abstract boolean status();
+
+ /**
+ * Returns the list of leaf {@link PredicateResult} whose {@link #status()} is equal to the
+ * {@code status} parameter.
+ */
+ ImmutableList<PredicateResult> getAtoms(boolean status) {
+ ImmutableList.Builder<PredicateResult> atomsList = ImmutableList.builder();
+ getAtomsRecursively(atomsList, status);
+ return atomsList.build();
+ }
+
+ private void getAtomsRecursively(ImmutableList.Builder<PredicateResult> list, boolean status) {
+ if (childPredicateResults().isEmpty() && status() == status) {
+ list.add(this);
+ return;
+ }
+ childPredicateResults().forEach(c -> c.getAtomsRecursively(list, status));
+ }
+
+ public static Builder builder() {
+ return new AutoValue_SubmitRequirementExpressionResult_PredicateResult.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder childPredicateResults(ImmutableList<PredicateResult> value);
+
+ protected abstract ImmutableList.Builder<PredicateResult> childPredicateResultsBuilder();
+
+ public abstract Builder predicateString(String value);
+
+ public abstract Builder status(boolean value);
+
+ public Builder addChildPredicateResult(PredicateResult result) {
+ childPredicateResultsBuilder().add(result);
+ return this;
+ }
+
+ public abstract PredicateResult build();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/entities/SubmitRequirementResult.java b/java/com/google/gerrit/entities/SubmitRequirementResult.java
new file mode 100644
index 0000000..7b4d609
--- /dev/null
+++ b/java/com/google/gerrit/entities/SubmitRequirementResult.java
@@ -0,0 +1,131 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.entities;
+
+import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
+import java.util.Optional;
+
+/** Result of evaluating a {@link SubmitRequirement} on a given Change. */
+@AutoValue
+public abstract class SubmitRequirementResult {
+ /** Result of evaluating a {@link SubmitRequirement#applicabilityExpression()} on a change. */
+ public abstract Optional<SubmitRequirementExpressionResult> applicabilityExpressionResult();
+
+ /**
+ * Result of evaluating a {@link SubmitRequirement#submittabilityExpression()} ()} on a change.
+ */
+ public abstract SubmitRequirementExpressionResult submittabilityExpressionResult();
+
+ /** Result of evaluating a {@link SubmitRequirement#overrideExpression()} ()} on a change. */
+ public abstract Optional<SubmitRequirementExpressionResult> overrideExpressionResult();
+
+ @Memoized
+ public Status status() {
+ if (assertError(submittabilityExpressionResult())
+ || assertError(applicabilityExpressionResult())
+ || assertError(overrideExpressionResult())) {
+ return Status.ERROR;
+ } else if (assertFail(applicabilityExpressionResult())) {
+ return Status.NOT_APPLICABLE;
+ } else if (assertPass(overrideExpressionResult())) {
+ return Status.OVERRIDDEN;
+ } else if (assertPass(submittabilityExpressionResult())) {
+ return Status.SATISFIED;
+ } else {
+ return Status.UNSATISFIED;
+ }
+ }
+
+ public static Builder builder() {
+ return new AutoValue_SubmitRequirementResult.Builder();
+ }
+
+ public enum Status {
+ /** Submit requirement is fulfilled. */
+ SATISFIED,
+
+ /**
+ * Submit requirement is not satisfied. Happens when {@link
+ * SubmitRequirement#submittabilityExpression()} evaluates to false.
+ */
+ UNSATISFIED,
+
+ /**
+ * Submit requirement is overridden. Happens when {@link SubmitRequirement#overrideExpression()}
+ * evaluates to true.
+ */
+ OVERRIDDEN,
+
+ /**
+ * Submit requirement is not applicable for a given change. Happens when {@link
+ * SubmitRequirement#applicabilityExpression()} evaluates to false.
+ */
+ NOT_APPLICABLE,
+
+ /**
+ * Any of the applicability, blocking or override expressions contain invalid syntax and are not
+ * parsable.
+ */
+ ERROR
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder applicabilityExpressionResult(
+ Optional<SubmitRequirementExpressionResult> value);
+
+ public abstract Builder submittabilityExpressionResult(SubmitRequirementExpressionResult value);
+
+ public abstract Builder overrideExpressionResult(
+ Optional<SubmitRequirementExpressionResult> value);
+
+ public abstract SubmitRequirementResult build();
+ }
+
+ private boolean assertPass(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
+ }
+
+ private boolean assertPass(SubmitRequirementExpressionResult expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
+ }
+
+ private boolean assertFail(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.FAIL);
+ }
+
+ private boolean assertError(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
+ }
+
+ private boolean assertError(SubmitRequirementExpressionResult expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
+ }
+
+ private boolean assertStatus(
+ SubmitRequirementExpressionResult expressionResult,
+ SubmitRequirementExpressionResult.Status status) {
+ return expressionResult.status() == status;
+ }
+
+ private boolean assertStatus(
+ Optional<SubmitRequirementExpressionResult> expressionResult,
+ SubmitRequirementExpressionResult.Status status) {
+ return expressionResult.isPresent() && assertStatus(expressionResult.get(), status);
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java
index ad015d1..47a377f 100644
--- a/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java
+++ b/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java
@@ -28,7 +28,8 @@
.setDescription(Optional.ofNullable(Strings.emptyToNull(proto.getDescription())))
.setApplicabilityExpression(
SubmitRequirementExpression.of(proto.getApplicabilityExpression()))
- .setBlockingExpression(SubmitRequirementExpression.create(proto.getBlockingExpression()))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create(proto.getSubmittabilityExpression()))
.setOverrideExpression(SubmitRequirementExpression.of(proto.getOverrideExpression()))
.setAllowOverrideInChildProjects(proto.getAllowOverrideInChildProjects())
.build();
@@ -40,10 +41,11 @@
.setName(submitRequirement.name())
.setDescription(submitRequirement.description().orElse(""))
.setApplicabilityExpression(
- submitRequirement.applicabilityExpression().orElse(emptyExpression).expression())
- .setBlockingExpression(submitRequirement.blockingExpression().expression())
+ submitRequirement.applicabilityExpression().orElse(emptyExpression).expressionString())
+ .setSubmittabilityExpression(
+ submitRequirement.submittabilityExpression().expressionString())
.setOverrideExpression(
- submitRequirement.overrideExpression().orElse(emptyExpression).expression())
+ submitRequirement.overrideExpression().orElse(emptyExpression).expressionString())
.setAllowOverrideInChildProjects(submitRequirement.allowOverrideInChildProjects())
.build();
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java b/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
index 71cb8c9..76573f6 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
@@ -15,9 +15,12 @@
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.Sets;
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.InsertedObject;
import java.io.IOException;
@@ -127,4 +130,14 @@
}
return footerLines.get(key.getName().toLowerCase());
}
+
+ public boolean isAttentionSetCommitOnly(boolean hasChangeMessage) {
+ return !hasChangeMessage
+ && footerLines
+ .keySet()
+ .equals(
+ Sets.newHashSet(
+ FOOTER_PATCH_SET.getName().toLowerCase(),
+ FOOTER_ATTENTION.getName().toLowerCase()));
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index f4d6cd3..f12176b 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -327,7 +327,6 @@
}
private void parse(ChangeNotesCommit commit) throws ConfigInvalidException {
- updateCount++;
Timestamp commitTimestamp = getCommitTimestamp(commit);
createdOn = commitTimestamp;
@@ -370,7 +369,8 @@
originalSubject = currSubject;
}
- parseChangeMessage(psId, accountId, realAccountId, commit, commitTimestamp);
+ boolean hasChangeMessage =
+ parseChangeMessage(psId, accountId, realAccountId, commit, commitTimestamp);
if (topic == null) {
topic = parseTopic(commit);
}
@@ -435,6 +435,9 @@
previousWorkInProgressFooter = null;
parseWorkInProgress(commit);
+ if (countTowardsMaxUpdatesLimit(commit, hasChangeMessage)) {
+ updateCount++;
+ }
}
private void parseSubmission(ChangeNotesCommit commit, Timestamp commitTimestamp)
@@ -720,7 +723,7 @@
}
}
- private void parseChangeMessage(
+ private boolean parseChangeMessage(
PatchSet.Id psId,
Account.Id accountId,
Account.Id realAccountId,
@@ -728,7 +731,7 @@
Timestamp ts) {
Optional<String> changeMsgString = getChangeMessageString(commit);
if (!changeMsgString.isPresent()) {
- return;
+ return false;
}
ChangeMessage changeMessage =
@@ -740,7 +743,7 @@
changeMsgString.get(),
realAccountId,
tag);
- allChangeMessages.add(changeMessage);
+ return allChangeMessages.add(changeMessage);
}
public static Optional<String> getChangeMessageString(ChangeNotesCommit commit) {
@@ -1197,4 +1200,9 @@
.orElseThrow(
() -> parseException("cannot retrieve account id: %s", ident.getEmailAddress()));
}
+
+ protected boolean countTowardsMaxUpdatesLimit(
+ ChangeNotesCommit commit, boolean hasChangeMessage) {
+ return !commit.isAttentionSetCommitOnly(hasChangeMessage);
+ }
}
diff --git a/java/com/google/gerrit/server/patch/ComparisonType.java b/java/com/google/gerrit/server/patch/ComparisonType.java
index eca2658..e450779 100644
--- a/java/com/google/gerrit/server/patch/ComparisonType.java
+++ b/java/com/google/gerrit/server/patch/ComparisonType.java
@@ -30,7 +30,7 @@
/**
* 1-based parent. Available if the old commit is the parent of the new commit and old commit is
- * not the auto-merge.
+ * not the auto-merge. If set to 0, then comparison is for a root commit.
*/
abstract Optional<Integer> parentNum();
@@ -48,6 +48,10 @@
return new AutoValue_ComparisonType(Optional.empty(), true);
}
+ public static ComparisonType againstRoot() {
+ return new AutoValue_ComparisonType(Optional.of(0), false);
+ }
+
private static ComparisonType create(Optional<Integer> parent, boolean automerge) {
return new AutoValue_ComparisonType(parent, automerge);
}
diff --git a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
index 6217239..dbbb7a6 100644
--- a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
+++ b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.MERGE_LIST;
+import static org.eclipse.jgit.lib.Constants.EMPTY_TREE_ID;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
@@ -341,6 +342,10 @@
abstract ObjectId newCommit();
+ /**
+ * Base commit represents the old commit of the diff. For diffs against the root commit, this
+ * should be set to {@code EMPTY_TREE_ID}.
+ */
abstract ObjectId baseCommit();
abstract ComparisonType comparisonType();
@@ -386,6 +391,11 @@
return result.build();
}
int numParents = baseCommitUtil.getNumParents(project, newCommit);
+ if (numParents == 0) {
+ result.baseCommit(EMPTY_TREE_ID);
+ result.comparisonType(ComparisonType.againstRoot());
+ return result.build();
+ }
if (numParents == 1) {
result.baseCommit(baseCommitUtil.getBaseCommit(project, newCommit, parent));
result.comparisonType(ComparisonType.againstParent(1));
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
index 395312f..2133474 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -26,6 +26,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
@@ -93,7 +94,7 @@
persist(DIFF, FileDiffCacheKey.class, FileDiffOutput.class)
.maximumWeight(10 << 20)
.weigher(FileDiffWeigher.class)
- .version(4)
+ .version(5)
.keySerializer(FileDiffCacheKey.Serializer.INSTANCE)
.valueSerializer(FileDiffOutput.Serializer.INSTANCE)
.loader(FileDiffLoader.class);
@@ -189,6 +190,9 @@
private ComparisonType getComparisonType(
RevWalk rw, ObjectReader reader, ObjectId oldCommitId, ObjectId newCommitId)
throws IOException {
+ if (oldCommitId.equals(EMPTY_TREE_ID)) {
+ return ComparisonType.againstRoot();
+ }
RevCommit oldCommit = DiffUtil.getRevCommit(rw, oldCommitId);
RevCommit newCommit = DiffUtil.getRevCommit(rw, newCommitId);
for (int i = 0; i < newCommit.getParentCount(); i++) {
@@ -211,7 +215,7 @@
}
/**
- * Creates a {@link FileDiffOutput} entry for the "Commit message" and "Merge list" file paths.
+ * Creates a {@link FileDiffOutput} entry for the "Commit message" or "Merge list" magic paths.
*/
private FileDiffOutput createMagicPathEntry(
FileDiffCacheKey key, ObjectReader reader, RevWalk rw, MagicPath magicPath) {
@@ -219,7 +223,10 @@
RawTextComparator cmp = comparatorFor(key.whitespace());
ComparisonType comparisonType =
getComparisonType(rw, reader, key.oldCommit(), key.newCommit());
- RevCommit aCommit = DiffUtil.getRevCommit(rw, key.oldCommit());
+ RevCommit aCommit =
+ key.oldCommit().equals(EMPTY_TREE_ID)
+ ? null
+ : DiffUtil.getRevCommit(rw, key.oldCommit());
RevCommit bCommit = DiffUtil.getRevCommit(rw, key.newCommit());
return magicPath == MagicPath.COMMIT
? createCommitEntry(reader, aCommit, bCommit, comparisonType, cmp, key.diffAlgorithm())
@@ -248,16 +255,19 @@
}
}
+ /**
+ * Creates a commit entry. {@code oldCommit} is null if the comparison is against a root commit.
+ */
private FileDiffOutput createCommitEntry(
ObjectReader reader,
- RevCommit oldCommit,
+ @Nullable RevCommit oldCommit,
RevCommit newCommit,
ComparisonType comparisonType,
RawTextComparator rawTextComparator,
GitFileDiffCacheImpl.DiffAlgorithm diffAlgorithm)
throws IOException {
Text aText =
- comparisonType.isAgainstParentOrAutoMerge()
+ oldCommit == null || comparisonType.isAgainstParentOrAutoMerge()
? Text.EMPTY
: Text.forCommit(reader, oldCommit);
Text bText = Text.forCommit(reader, newCommit);
@@ -272,16 +282,20 @@
diffAlgorithm);
}
+ /**
+ * Creates a merge list entry. {@code oldCommit} is null if the comparison is against a root
+ * commit.
+ */
private FileDiffOutput createMergeListEntry(
ObjectReader reader,
- RevCommit oldCommit,
+ @Nullable RevCommit oldCommit,
RevCommit newCommit,
ComparisonType comparisonType,
RawTextComparator rawTextComparator,
GitFileDiffCacheImpl.DiffAlgorithm diffAlgorithm)
throws IOException {
Text aText =
- comparisonType.isAgainstParentOrAutoMerge()
+ oldCommit == null || comparisonType.isAgainstParentOrAutoMerge()
? Text.EMPTY
: Text.forMergeList(comparisonType, reader, oldCommit);
Text bText = Text.forMergeList(comparisonType, reader, newCommit);
@@ -297,7 +311,7 @@
}
private static FileDiffOutput createMagicFileDiffOutput(
- ObjectId oldCommit,
+ @Nullable ObjectId oldCommit,
ObjectId newCommit,
ComparisonType comparisonType,
RawTextComparator rawTextComparator,
@@ -317,7 +331,7 @@
FileHeader fileHeader = new FileHeader(rawHdr, edits, PatchType.UNIFIED);
Patch.ChangeType changeType = FileHeaderUtil.getChangeType(fileHeader);
return FileDiffOutput.builder()
- .oldCommitId(oldCommit)
+ .oldCommitId(oldCommit == null ? EMPTY_TREE_ID : oldCommit)
.newCommitId(newCommit)
.comparisonType(comparisonType)
.oldPath(FileHeaderUtil.getOldPath(fileHeader))
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java
index a478fcf..7ac9343 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.cache.serialize.CacheSerializer;
import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithm;
+import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
/** Cache key for the {@link FileDiffCache}. */
@@ -35,7 +36,10 @@
/** A specific git project / repository. */
public abstract Project.NameKey project();
- /** The 20 bytes SHA-1 commit ID of the old commit used in the diff. */
+ /**
+ * The 20 bytes SHA-1 commit ID of the old commit used in the diff. If set to {@link
+ * Constants#EMPTY_TREE_ID}, the diff is performed against the root commit.
+ */
public abstract ObjectId oldCommit();
/** The 20 bytes SHA-1 commit ID of the new commit used in the diff. */
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 027cf17..9f898d9 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -130,7 +130,7 @@
public static final String KEY_SR_NAME = "name";
public static final String KEY_SR_DESCRIPTION = "description";
public static final String KEY_SR_APPLICABILITY_EXPRESSION = "applicabilityExpression";
- public static final String KEY_SR_BLOCKING_EXPRESSION = "blockingExpression";
+ public static final String KEY_SR_SUBMITTABILITY_EXPRESSION = "submittabilityExpression";
public static final String KEY_SR_OVERRIDE_EXPRESSION = "overrideExpression";
public static final String KEY_SR_OVERRIDE_IN_CHILD_PROJECTS = "canOverrideInChildProjects";
@@ -985,7 +985,7 @@
lowerNames.put(lower, name);
String description = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_DESCRIPTION);
String appExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_APPLICABILITY_EXPRESSION);
- String blockExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_BLOCKING_EXPRESSION);
+ String blockExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_SUBMITTABILITY_EXPRESSION);
String overrideExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_OVERRIDE_EXPRESSION);
boolean canInherit =
rc.getBoolean(SUBMIT_REQUIREMENT, name, KEY_SR_OVERRIDE_IN_CHILD_PROJECTS, false);
@@ -995,7 +995,7 @@
ValidationError.create(
PROJECT_CONFIG,
(String.format(
- "Submit requirement \"%s\" does not define a blocking expression."
+ "Submit requirement \"%s\" does not define a submittability expression."
+ " Skipping this requirement.",
name))));
continue;
@@ -1009,7 +1009,7 @@
.setName(name)
.setDescription(Optional.ofNullable(description))
.setApplicabilityExpression(SubmitRequirementExpression.of(appExpr))
- .setBlockingExpression(SubmitRequirementExpression.create(blockExpr))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create(blockExpr))
.setOverrideExpression(SubmitRequirementExpression.of(overrideExpr))
.setAllowOverrideInChildProjects(canInherit)
.build();
@@ -1701,19 +1701,19 @@
SUBMIT_REQUIREMENT,
name,
KEY_SR_APPLICABILITY_EXPRESSION,
- sr.applicabilityExpression().get().expression());
+ sr.applicabilityExpression().get().expressionString());
}
rc.setString(
SUBMIT_REQUIREMENT,
name,
- KEY_SR_BLOCKING_EXPRESSION,
- sr.blockingExpression().expression());
+ KEY_SR_SUBMITTABILITY_EXPRESSION,
+ sr.submittabilityExpression().expressionString());
if (sr.overrideExpression().isPresent()) {
rc.setString(
SUBMIT_REQUIREMENT,
name,
KEY_SR_OVERRIDE_EXPRESSION,
- sr.overrideExpression().get().expression());
+ sr.overrideExpression().get().expressionString());
}
rc.setBoolean(
SUBMIT_REQUIREMENT,
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
new file mode 100644
index 0000000..1d154dd
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -0,0 +1,101 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.PredicateResult;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.util.Optional;
+
+/** Evaluates submit requirements for different change data. */
+@Singleton
+public class SubmitRequirementsEvaluator {
+ private final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
+
+ @Inject
+ private SubmitRequirementsEvaluator(Provider<ChangeQueryBuilder> changeQueryBuilderProvider) {
+ this.changeQueryBuilderProvider = changeQueryBuilderProvider;
+ }
+
+ /**
+ * Validate a {@link SubmitRequirementExpression}. Callers who wish to validate submit
+ * requirements upon creation or update should use this method.
+ *
+ * @param expression entity containing the expression string.
+ * @throws QueryParseException the expression string contains invalid syntax and can't be parsed.
+ */
+ public void validateExpression(SubmitRequirementExpression expression)
+ throws QueryParseException {
+ changeQueryBuilderProvider.get().parse(expression.expressionString());
+ }
+
+ /** Evaluate a {@link SubmitRequirement} on a given {@link ChangeData}. */
+ public SubmitRequirementResult evaluate(SubmitRequirement sr, ChangeData cd) {
+ SubmitRequirementExpressionResult blockingResult =
+ evaluateExpression(sr.submittabilityExpression(), cd);
+
+ Optional<SubmitRequirementExpressionResult> applicabilityResult =
+ sr.applicabilityExpression().isPresent()
+ ? Optional.of(evaluateExpression(sr.applicabilityExpression().get(), cd))
+ : Optional.empty();
+
+ Optional<SubmitRequirementExpressionResult> overrideResult =
+ sr.overrideExpression().isPresent()
+ ? Optional.of(evaluateExpression(sr.overrideExpression().get(), cd))
+ : Optional.empty();
+
+ return SubmitRequirementResult.builder()
+ .submittabilityExpressionResult(blockingResult)
+ .applicabilityExpressionResult(applicabilityResult)
+ .overrideExpressionResult(overrideResult)
+ .build();
+ }
+
+ /** Evaluate a {@link SubmitRequirementExpression} using change data. */
+ public SubmitRequirementExpressionResult evaluateExpression(
+ SubmitRequirementExpression expression, ChangeData changeData) {
+ try {
+ Predicate<ChangeData> predicate =
+ changeQueryBuilderProvider.get().parse(expression.expressionString());
+ PredicateResult predicateResult = evaluatePredicateTree(predicate, changeData);
+ return SubmitRequirementExpressionResult.create(predicateResult);
+ } catch (QueryParseException e) {
+ return SubmitRequirementExpressionResult.error(e.getMessage());
+ }
+ }
+
+ /** Evaluate the predicate recursively using change data. */
+ private PredicateResult evaluatePredicateTree(
+ Predicate<ChangeData> predicate, ChangeData changeData) {
+ PredicateResult.Builder predicateResult =
+ PredicateResult.builder()
+ .predicateString(predicate.toString())
+ .status(predicate.asMatchable().match(changeData));
+ predicate
+ .getChildren()
+ .forEach(
+ c -> predicateResult.addChildPredicateResult(evaluatePredicateTree(c, changeData)));
+ return predicateResult.build();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 9bdc420..1a55b82 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
@@ -25,6 +26,7 @@
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toMap;
+import static org.eclipse.jgit.lib.Constants.EMPTY_TREE_ID;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
@@ -35,7 +37,10 @@
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.RawInputUtil;
+import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.Permission;
import com.google.gerrit.extensions.api.changes.FileApi;
import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
@@ -46,6 +51,8 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.webui.EditWebLink;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import com.google.inject.Inject;
import java.awt.image.BufferedImage;
import java.io.ByteArrayOutputStream;
@@ -57,6 +64,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;
+import java.util.stream.Collectors;
import java.util.stream.IntStream;
import javax.imageio.ImageIO;
import org.eclipse.jgit.lib.ObjectId;
@@ -81,11 +89,14 @@
private static final String FILE_CONTENT2 = "1st line\n2nd line\n3rd line\n";
@Inject private ExtensionRegistry extensionRegistry;
+ @Inject private DiffOperations diffOperations;
+ @Inject private ProjectOperations projectOperations;
private boolean intraline;
private boolean useNewDiffCacheListFiles;
private boolean useNewDiffCacheGetDiff;
+ private ObjectId initialCommit;
private ObjectId commit1;
private String changeId;
private String initialPatchSetId;
@@ -104,6 +115,8 @@
baseConfig.getBoolean("cache", "diff_cache", "runNewDiffCache_GetDiff", false);
ObjectId headCommit = testRepo.getRepository().resolve("HEAD");
+ initialCommit = headCommit;
+
commit1 =
addCommit(headCommit, ImmutableMap.of(FILE_NAME, FILE_CONTENT, FILE_NAME2, FILE_CONTENT2));
@@ -124,10 +137,33 @@
assertDiffForNewFile(result, COMMIT_MSG, result.getCommit().getFullMessage());
}
- @Ignore
@Test
public void diffWithRootCommit() throws Exception {
- // TODO(ghareeb): Implement this test
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref("refs/*").group(adminGroupUuid()).force(true))
+ .update();
+
+ testRepo.reset(initialCommit);
+ PushOneCommit push =
+ pushFactory
+ .create(admin.newIdent(), testRepo, "subject", ImmutableMap.of("f.txt", "content"))
+ .noParent();
+ push.setForce(true);
+ PushOneCommit.Result result = push.to("refs/heads/master");
+
+ Map<String, FileDiffOutput> modifiedFiles =
+ diffOperations.listModifiedFilesAgainstParent(project, result.getCommit(), null);
+
+ assertThat(modifiedFiles.keySet()).containsExactly("/COMMIT_MSG", "f.txt");
+ assertThat(
+ modifiedFiles.values().stream()
+ .map(FileDiffOutput::oldCommitId)
+ .collect(Collectors.toSet()))
+ .containsExactly(EMPTY_TREE_ID);
+ assertThat(modifiedFiles.get("/COMMIT_MSG").changeType()).isEqualTo(Patch.ChangeType.ADDED);
+ assertThat(modifiedFiles.get("f.txt").changeType()).isEqualTo(Patch.ChangeType.ADDED);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
new file mode 100644
index 0000000..23047a4
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -0,0 +1,265 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.project;
+
+import static com.google.common.truth.Truth.assertThat;
+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.value;
+
+import com.google.common.collect.MoreCollectors;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.LabelFunction;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.PredicateResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Optional;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+public class SubmitRequirementsEvaluatorIT extends AbstractDaemonTest {
+ @Inject SubmitRequirementsEvaluator evaluator;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private Provider<InternalChangeQuery> changeQueryProvider;
+
+ private ChangeData changeData;
+ private String changeId;
+
+ @Before
+ public void setUp() throws Exception {
+ PushOneCommit.Result pushResult =
+ createChange(testRepo, "refs/heads/master", "Fix a bug", "file.txt", "content", "topic");
+ changeData = pushResult.getChange();
+ changeId = pushResult.getChangeId();
+ }
+
+ @Test
+ public void invalidExpression() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create("invalid_field:invalid_value");
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.ERROR);
+ assertThat(result.errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ @Test
+ public void expressionWithPassingPredicate() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create("branch:refs/heads/master");
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.PASS);
+ assertThat(result.errorMessage()).isEqualTo(Optional.empty());
+ }
+
+ @Test
+ public void expressionWithFailingPredicate() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create("branch:refs/heads/foo");
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.FAIL);
+ assertThat(result.errorMessage()).isEqualTo(Optional.empty());
+ }
+
+ @Test
+ public void compositeExpression() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create(
+ String.format(
+ "(project:%s AND branch:refs/heads/foo) OR message:\"Fix a bug\"", project.get()));
+
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.PASS);
+
+ assertThat(result.getPassingAtoms())
+ .containsExactly(
+ PredicateResult.builder()
+ .predicateString(String.format("project:%s", project.get()))
+ .status(true)
+ .build(),
+ PredicateResult.builder()
+ .predicateString("message:\"Fix a bug\"")
+ .status(true)
+ .build());
+
+ assertThat(result.getFailingAtoms())
+ .containsExactly(
+ PredicateResult.builder()
+ // TODO(ghareeb): querying "branch:" creates a RefPredicate. Fix names so that they
+ // match
+ .predicateString(String.format("ref:refs/heads/foo"))
+ .status(false)
+ .build());
+ }
+
+ @Test
+ public void submitRequirementIsNotApplicable_whenApplicabilityExpressionIsFalse()
+ throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:non-existent-project",
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.NOT_APPLICABLE);
+ }
+
+ @Test
+ public void submitRequirementIsSatisfied_whenSubmittabilityExpressionIsTrue() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "message:\"Fix a bug\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ }
+
+ @Test
+ public void submitRequirementIsUnsatisfied_whenSubmittabilityExpressionIsFalse()
+ throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.UNSATISFIED);
+ }
+
+ @Test
+ public void submitRequirementIsOverridden_whenOverrideExpressionIsTrue() throws Exception {
+ addLabel("build-cop-override");
+ voteLabel(changeId, "build-cop-override", 1);
+
+ // Reload change data after applying the vote
+ changeData =
+ changeQueryProvider.get().byLegacyChangeId(changeData.getId()).stream()
+ .collect(MoreCollectors.onlyElement());
+
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "label:\"build-cop-override=+1\"");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.OVERRIDDEN);
+ }
+
+ @Test
+ public void submitRequirementIsError_whenApplicabilityExpressionHasInvalidSyntax()
+ throws Exception {
+ addLabel("build-cop-override");
+
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "invalid_field:invalid_value",
+ /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "label:\"build-cop-override=+1\"");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
+ assertThat(result.applicabilityExpressionResult().get().errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ @Test
+ public void submitRequirementIsError_whenSubmittabilityExpressionHasInvalidSyntax()
+ throws Exception {
+ addLabel("build-cop-override");
+
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "invalid_field:invalid_value",
+ /* overrideExpr= */ "label:\"build-cop-override=+1\"");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
+ assertThat(result.submittabilityExpressionResult().errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ @Test
+ public void submitRequirementIsError_whenOverrideExpressionHasInvalidSyntax() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "invalid_field:invalid_value");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
+ assertThat(result.overrideExpressionResult().get().errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ private void voteLabel(String changeId, String labelName, int score) throws RestApiException {
+ gApi.changes().id(changeId).current().review(new ReviewInput().label(labelName, score));
+ }
+
+ private void addLabel(String labelName) throws Exception {
+ configLabel(
+ project,
+ labelName,
+ LabelFunction.NO_OP,
+ value(1, "ok"),
+ value(0, "No score"),
+ value(-1, "Needs work"));
+
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(labelName).ref("refs/heads/master").group(REGISTERED_USERS).range(-1, +1))
+ .update();
+ }
+
+ private SubmitRequirement createSubmitRequirement(
+ @Nullable String applicabilityExpr,
+ String submittabilityExpr,
+ @Nullable String overrideExpr) {
+ return SubmitRequirement.builder()
+ .setName("sr-name")
+ .setDescription(Optional.of("sr-description"))
+ .setApplicabilityExpression(SubmitRequirementExpression.of(applicabilityExpr))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create(submittabilityExpr))
+ .setOverrideExpression(SubmitRequirementExpression.of(overrideExpr))
+ .setAllowOverrideInChildProjects(false)
+ .build();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java
index c2e8e0c..a1dee1a 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java
@@ -29,7 +29,7 @@
.setName("code-review")
.setDescription(Optional.of("require code review +2"))
.setApplicabilityExpression(SubmitRequirementExpression.of("branch(refs/heads/master)"))
- .setBlockingExpression(SubmitRequirementExpression.create("label(code-review, 2+)"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label(code-review, 2+)"))
.setOverrideExpression(Optional.empty())
.setAllowOverrideInChildProjects(true)
.build();
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
new file mode 100644
index 0000000..f105cf1
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
@@ -0,0 +1,124 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.util.time.TimeUtil;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ChangeNotesCommitTest extends AbstractChangeNotesTest {
+ private TestRepository<InMemoryRepository> testRepo;
+ private ChangeNotesCommit.ChangeNotesRevWalk walk;
+
+ @Before
+ public void setUpTestRepo() throws Exception {
+ testRepo = new TestRepository<>(repo);
+ walk = ChangeNotesCommit.newRevWalk(repo);
+ }
+
+ @After
+ public void tearDownTestRepo() throws Exception {
+ walk.close();
+ }
+
+ @Test
+ public void attentionSetCommitOnlyWhenNoChangeMessageIsPresentAndCorrectFooter()
+ throws Exception {
+ RevCommit commit =
+ writeCommit(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1000000 \\u003c1000000@adce0b11-8f2e-4ab6-ac69-e675f183d871\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Administrator using the hovercard menu\"}");
+
+ newParser(commit).parseAll();
+ assertThat(((ChangeNotesCommit) commit).isAttentionSetCommitOnly(false)).isEqualTo(true);
+ }
+
+ @Test
+ public void noAttentionSetCommitOnlyWhenNoChangeMessageIsPresentAndFooterNotOnlyAS()
+ throws Exception {
+ RevCommit commit =
+ writeCommit(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Subject: Change subject\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1000000 \\u003c1000000@adce0b11-8f2e-4ab6-ac69-e675f183d871\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Administrator using the hovercard menu\"}");
+
+ newParser(commit).parseAll();
+ assertThat(((ChangeNotesCommit) commit).isAttentionSetCommitOnly(false)).isEqualTo(false);
+ }
+
+ @Test
+ public void noAttentionSetCommitOnlyWhenNoChangeMessageIsPresentAndGenericFooter()
+ throws Exception {
+ RevCommit commit = writeCommit("Update patch set 1\n" + "\n" + "Patch-set: 1\n");
+
+ newParser(commit).parseAll();
+ assertThat(((ChangeNotesCommit) commit).isAttentionSetCommitOnly(false)).isEqualTo(false);
+ }
+
+ @Test
+ public void noAttentionSetCommitOnlyWhenChangeMessageIsPresent() throws Exception {
+ RevCommit commit =
+ writeCommit(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1000000 \\u003c1000000@adce0b11-8f2e-4ab6-ac69-e675f183d871\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Administrator using the hovercard menu\"}");
+
+ newParser(commit).parseAll();
+ assertThat(((ChangeNotesCommit) commit).isAttentionSetCommitOnly(true)).isEqualTo(false);
+ }
+
+ private ChangeNotesParser newParser(ObjectId tip) throws Exception {
+ walk.reset();
+ ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
+ return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
+ }
+
+ private RevCommit writeCommit(String body) throws Exception {
+ Change change = newChange(true);
+ ChangeNotes notes = newNotes(change).load();
+ ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
+ PersonIdent author =
+ noteUtil.newAccountIdIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent);
+ try (ObjectInserter ins = testRepo.getRepository().newObjectInserter()) {
+ CommitBuilder cb = new CommitBuilder();
+ cb.setParentId(notes.getRevision());
+ cb.setAuthor(author);
+ cb.setCommitter(new PersonIdent(serverIdent, author.getWhen()));
+ cb.setTreeId(testRepo.tree());
+ cb.setMessage(body);
+ ObjectId id = ins.insert(cb);
+ ins.flush();
+ RevCommit commit = walk.parseCommit(id);
+ walk.parseBody(commit);
+ return commit;
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 5bfe97c..6a32fa1 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -497,6 +497,73 @@
}
@Test
+ public void attentionSetOnlyShouldNotCountTowardsMaxUpdatesLimit() throws Exception {
+ RevCommit commit =
+ writeCommit(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1000000 \\u003c1000000@adce0b11-8f2e-4ab6-ac69-e675f183d871\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Administrator using the hovercard menu\"}",
+ false);
+ ChangeNotesParser changeNotesParser = newParser(commit);
+ changeNotesParser.parseAll();
+ final boolean hasChangeMessage = false;
+ assertThat(
+ changeNotesParser.countTowardsMaxUpdatesLimit(
+ (ChangeNotesCommit) commit, hasChangeMessage))
+ .isEqualTo(false);
+ }
+
+ @Test
+ public void attentionSetWithExtraFooterShouldCountTowardsMaxUpdatesLimit() throws Exception {
+ RevCommit commit =
+ writeCommit(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Subject: Change subject\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1000000 \\u003c1000000@adce0b11-8f2e-4ab6-ac69-e675f183d871\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Administrator using the hovercard menu\"}",
+ false);
+ ChangeNotesParser changeNotesParser = newParser(commit);
+ changeNotesParser.parseAll();
+ final boolean hasChangeMessage = false;
+ assertThat(
+ changeNotesParser.countTowardsMaxUpdatesLimit(
+ (ChangeNotesCommit) commit, hasChangeMessage))
+ .isEqualTo(true);
+ }
+
+ @Test
+ public void changeWithoutAttentionSetShouldCountTowardsMaxUpdatesLimit() throws Exception {
+ RevCommit commit = writeCommit("Update WIP change\n" + "\n" + "Patch-set: 1\n", true);
+ ChangeNotesParser changeNotesParser = newParser(commit);
+ changeNotesParser.parseAll();
+ final boolean hasChangeMessage = false;
+ assertThat(
+ changeNotesParser.countTowardsMaxUpdatesLimit(
+ (ChangeNotesCommit) commit, hasChangeMessage))
+ .isEqualTo(true);
+ }
+
+ @Test
+ public void attentionSetWithCommentShouldCountTowardsMaxUpdatesLimit() throws Exception {
+ RevCommit commit =
+ writeCommit(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1000000 \\u003c1000000@adce0b11-8f2e-4ab6-ac69-e675f183d871\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Administrator using the hovercard menu\"}",
+ false);
+ ChangeNotesParser changeNotesParser = newParser(commit);
+ changeNotesParser.parseAll();
+ final boolean hasChangeMessage = true;
+ assertThat(
+ changeNotesParser.countTowardsMaxUpdatesLimit(
+ (ChangeNotesCommit) commit, hasChangeMessage))
+ .isEqualTo(true);
+ }
+
+ @Test
public void caseInsensitiveFooters() throws Exception {
assertParseSucceeds(
"Update change\n"
diff --git a/javatests/com/google/gerrit/server/notedb/OpenRepoTest.java b/javatests/com/google/gerrit/server/notedb/OpenRepoTest.java
new file mode 100644
index 0000000..e4c2196
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/OpenRepoTest.java
@@ -0,0 +1,143 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.entities.AttentionSetUpdate;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.update.ChainedReceiveCommands;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.junit.Test;
+
+public class OpenRepoTest extends AbstractChangeNotesTest {
+
+ private final Optional<Integer> NO_UPDATES_AT_ALL = Optional.of(0);
+ private final Optional<Integer> ONLY_ONE_UPDATE = Optional.of(1);
+ private final Optional<Integer> ONLY_TWO_UPDATES = Optional.of(2);
+ private final Optional<Integer> MAX_PATCH_SETS = Optional.empty();
+
+ private FakeChainedReceiveCommands fakeChainedReceiveCommands;
+
+ @Override
+ public void setUpTestEnvironment() throws Exception {
+ super.setUpTestEnvironment();
+ fakeChainedReceiveCommands = new FakeChainedReceiveCommands(repo);
+ }
+
+ @Test
+ public void throwExceptionWhenExceedingMaxUpdatesLimit() throws Exception {
+ try (OpenRepo openRepo = openRepo()) {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setStatus(Change.Status.NEW);
+
+ ListMultimap<String, ChangeUpdate> changeUpdates =
+ new ImmutableListMultimap.Builder<String, ChangeUpdate>().put("one", update).build();
+
+ assertThrows(
+ LimitExceededException.class,
+ () -> openRepo.addUpdates(changeUpdates, NO_UPDATES_AT_ALL, MAX_PATCH_SETS));
+ }
+ }
+
+ @Test
+ public void allowExceedingLimitWhenAttentionSetUpdateOnly() throws Exception {
+ try (OpenRepo openRepo = openRepo()) {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setStatus(Change.Status.NEW);
+
+ // Add to attention set
+ AttentionSetUpdate attentionSetUpdate =
+ AttentionSetUpdate.createForWrite(
+ otherUser.getAccountId(), AttentionSetUpdate.Operation.ADD, "test");
+ update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+
+ ListMultimap<String, ChangeUpdate> changeUpdates =
+ new ImmutableListMultimap.Builder<String, ChangeUpdate>().put("one", update).build();
+
+ openRepo.addUpdates(changeUpdates, NO_UPDATES_AT_ALL, MAX_PATCH_SETS);
+
+ assertThat(fakeChainedReceiveCommands.commands.size()).isEqualTo(1);
+ }
+ }
+
+ @Test
+ public void attentionSetUpdateShouldNotContributeToOperationsCount() throws Exception {
+ try (OpenRepo openRepo = openRepo()) {
+ Change c1 = newChange();
+
+ ChangeUpdate update1 = newUpdateForNewChange(c1, changeOwner);
+ // Add to attention set
+ AttentionSetUpdate attentionSetUpdate =
+ AttentionSetUpdate.createForWrite(
+ otherUser.getAccountId(), AttentionSetUpdate.Operation.ADD, "test");
+ update1.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+
+ ChangeUpdate update2 = newUpdateForNewChange(c1, changeOwner);
+ update2.setStatus(Change.Status.NEW);
+
+ ListMultimap<String, ChangeUpdate> changeUpdates =
+ new ImmutableListMultimap.Builder<String, ChangeUpdate>().put("two", update2).build();
+
+ openRepo.addUpdates(changeUpdates, ONLY_TWO_UPDATES, MAX_PATCH_SETS);
+
+ assertThat(fakeChainedReceiveCommands.commands.size()).isEqualTo(1);
+ }
+ }
+
+ @Test
+ public void normalChangeShouldContributeToOperationsCount() throws Exception {
+ try (OpenRepo openRepo = openRepo()) {
+ Change c1 = newChange();
+
+ ChangeUpdate update2 = newUpdateForNewChange(c1, changeOwner);
+ update2.setStatus(Change.Status.NEW);
+
+ ListMultimap<String, ChangeUpdate> changeUpdates =
+ new ImmutableListMultimap.Builder<String, ChangeUpdate>().put("two", update2).build();
+
+ assertThrows(
+ LimitExceededException.class,
+ () -> openRepo.addUpdates(changeUpdates, ONLY_ONE_UPDATE, MAX_PATCH_SETS));
+ }
+ }
+
+ private static class FakeChainedReceiveCommands extends ChainedReceiveCommands {
+ Map<String, ReceiveCommand> commands = new HashMap<>();
+
+ public FakeChainedReceiveCommands(Repository repo) {
+ super(repo);
+ }
+
+ @Override
+ public void add(ReceiveCommand cmd) {
+ commands.put(cmd.getRefName(), cmd);
+ }
+ }
+
+ private OpenRepo openRepo() {
+ return new OpenRepo(repo, rw, null, fakeChainedReceiveCommands, false);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index 9dad9ae..2e934e9 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -212,11 +212,11 @@
"[submitRequirement \"Code-review\"]\n"
+ " description = At least one Code Review +2\n"
+ " applicabilityExpression = branch(refs/heads/master)\n"
- + " blockingExpression = label(code-review, +2)\n"
+ + " submittabilityExpression = label(code-review, +2)\n"
+ "[submitRequirement \"api-review\"]\n"
+ " description = Additional review required for API modifications\n"
+ " applicabilityExpression = commit_filepath_contains(\\\"/api/.*\\\")\n"
- + " blockingExpression = label(api-review, +2)\n"
+ + " submittabilityExpression = label(api-review, +2)\n"
+ " overrideExpression = label(build-cop-override, +1)\n"
+ " canOverrideInChildProjects = true\n")
.create();
@@ -231,7 +231,8 @@
.setDescription(Optional.of("At least one Code Review +2"))
.setApplicabilityExpression(
SubmitRequirementExpression.of("branch(refs/heads/master)"))
- .setBlockingExpression(SubmitRequirementExpression.create("label(code-review, +2)"))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label(code-review, +2)"))
.setOverrideExpression(Optional.empty())
.setAllowOverrideInChildProjects(false)
.build(),
@@ -241,7 +242,8 @@
.setDescription(Optional.of("Additional review required for API modifications"))
.setApplicabilityExpression(
SubmitRequirementExpression.of("commit_filepath_contains(\"/api/.*\")"))
- .setBlockingExpression(SubmitRequirementExpression.create("label(api-review, +2)"))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label(api-review, +2)"))
.setOverrideExpression(
SubmitRequirementExpression.of("label(build-cop-override, +1)"))
.setAllowOverrideInChildProjects(true)
@@ -256,7 +258,7 @@
.add(
"project.config",
"[submitRequirement \"code-review\"]\n"
- + " blockingExpression = label(code-review, +2)\n")
+ + " submittabilityExpression = label(code-review, +2)\n")
.create();
ProjectConfig cfg = read(rev);
@@ -266,7 +268,8 @@
"code-review",
SubmitRequirement.builder()
.setName("code-review")
- .setBlockingExpression(SubmitRequirementExpression.create("label(code-review, +2)"))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label(code-review, +2)"))
.setAllowOverrideInChildProjects(false)
.build());
}
@@ -280,10 +283,10 @@
"project.config",
"[submitRequirement \"code-review\"]\n"
+ " description = At least one Code Review +2\n"
- + " blockingExpression = label(code-review, +2)\n"
+ + " submittabilityExpression = label(code-review, +2)\n"
+ "[submitRequirement \"Code-Review\"]\n"
+ " description = Another code review label\n"
- + " blockingExpression = label(code-review, +2)\n"
+ + " submittabilityExpression = label(code-review, +2)\n"
+ " canOverrideInChildProjects = true\n")
.create();
@@ -295,7 +298,8 @@
SubmitRequirement.builder()
.setName("code-review")
.setDescription(Optional.of("At least one Code Review +2"))
- .setBlockingExpression(SubmitRequirementExpression.create("label(code-review, +2)"))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label(code-review, +2)"))
.setAllowOverrideInChildProjects(false)
.build());
assertThat(cfg.getValidationErrors()).hasSize(1);
@@ -307,7 +311,7 @@
}
@Test
- public void readSubmitRequirementNoBlockingExpression() throws Exception {
+ public void readSubmitRequirementNoSubmittabilityExpression() throws Exception {
RevCommit rev =
tr.commit()
.add("groups", group(developers))
@@ -323,7 +327,7 @@
assertThat(cfg.getValidationErrors()).hasSize(1);
assertThat(Iterables.getOnlyElement(cfg.getValidationErrors()).getMessage())
.isEqualTo(
- "project.config: Submit requirement \"code-review\" does not define a blocking expression."
+ "project.config: Submit requirement \"code-review\" does not define a submittability expression."
+ " Skipping this requirement.");
}
@@ -942,7 +946,7 @@
"[submitRequirement \"code-review\"]\n"
+ " description = At least one Code Review +2\n"
+ " applicabilityExpression = branch(refs/heads/master)\n"
- + " blockingExpression = label(code-review, +2)\n"
+ + " submittabilityExpression = label(code-review, +2)\n"
+ "[notify \"name\"]\n"
+ " email = example@example.com\n")
.create();
diff --git a/plugins/replication b/plugins/replication
index fd593ee..dc9bb2e 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit fd593ee1d3cb19d73c49f94c0f9b4a78856d6198
+Subproject commit dc9bb2e946e4c6c31e8a4665f30eca6d00017523
diff --git a/polygerrit-ui/app/constants/reporting.ts b/polygerrit-ui/app/constants/reporting.ts
index 0738825..d22026a 100644
--- a/polygerrit-ui/app/constants/reporting.ts
+++ b/polygerrit-ui/app/constants/reporting.ts
@@ -90,3 +90,9 @@
// This measures the same interval as ExpandAllDiffs, but the result is divided by the number of diffs expanded.
FILE_EXPAND_ALL_AVG = 'ExpandAllPerDiff',
}
+
+export enum Interaction {
+ TOGGLE_SHOW_ALL_BUTTON = 'toggle show all button',
+ SHOW_TAB = 'show-tab',
+ ATTENTION_SET_CHIP = 'attention-set-chip',
+}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.ts
index 8a4cbbe..a9de24a 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.ts
@@ -118,19 +118,15 @@
assert.isTrue(saveStub.called);
});
- test('_getRepoBranchesSuggestions empty', done => {
- element._getRepoBranchesSuggestions('nonexistent').then(branches => {
- assert.equal(branches.length, 0);
- done();
- });
+ test('_getRepoBranchesSuggestions empty', async () => {
+ const branches = await element._getRepoBranchesSuggestions('nonexistent');
+ assert.equal(branches.length, 0);
});
- test('_getRepoBranchesSuggestions non-empty', done => {
- element._getRepoBranchesSuggestions('test-branch').then(branches => {
- assert.equal(branches.length, 1);
- assert.equal(branches[0].name, 'test-branch');
- done();
- });
+ test('_getRepoBranchesSuggestions non-empty', async () => {
+ const branches = await element._getRepoBranchesSuggestions('test-branch');
+ assert.equal(branches.length, 1);
+ assert.equal(branches[0].name, 'test-branch');
});
test('_computeBranchClass', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
index c8e058e..4864af5 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
@@ -23,20 +23,22 @@
const basicFixture = fixtureFromElement('gr-repo-list');
-let counter;
-const repoGenerator = () => {
+function createRepo(name, counter) {
return {
- id: `test${++counter}`,
- name: `test`,
+ id: `${name}${counter}`,
+ name: `${name}`,
state: 'ACTIVE',
web_links: [
{
name: 'diffusion',
- url: `https://phabricator.example.org/r/project/test${counter}`,
+ url: `https://phabricator.example.org/r/project/${name}${counter}`,
},
],
};
-};
+}
+
+let counter;
+const repoGenerator = () => createRepo('test', ++counter);
suite('gr-repo-list tests', () => {
let element;
@@ -123,6 +125,15 @@
done();
});
});
+
+ test('filter is case insensitive', async () => {
+ const repoStub = stubRestApi('getRepos');
+ const repos = [createRepo('aSDf', 0)];
+ repoStub.withArgs('asdf').returns(Promise.resolve(repos));
+ element._filter = 'asdf';
+ await element._getRepos('asdf', 25, 0);
+ assert.equal(element._repos.length, 1);
+ });
});
suite('loading', () => {
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
index d6f4bdd..41a7598 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
@@ -252,7 +252,7 @@
})
.catch(err => {
fireTitleChange(this, title || this._computeTitle(user));
- console.warn(err);
+ this.reporting.error(err);
})
.then(() => {
this._loading = false;
diff --git a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts
index 1924a66..42a6847 100644
--- a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header_html.ts
@@ -29,7 +29,7 @@
</style>
<gr-avatar
account="[[_accountDetails]]"
- image-size="100"
+ imageSize="100"
aria-label="Account avatar"
></gr-avatar>
<div class="info">
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index 1790651..02e4161 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -30,6 +30,7 @@
} from '../../../test/test-data-generators';
import {ChangeStatus, HttpMethod} from '../../../constants/constants';
import {
+ mockPromise,
query,
queryAll,
queryAndAssert,
@@ -144,13 +145,11 @@
return element.reload();
});
- test('show-revision-actions event should fire', done => {
+ test('show-revision-actions event should fire', async () => {
const spy = sinon.spy(element, '_sendShowRevisionActions');
element.reload();
- flush(() => {
- assert.isTrue(spy.called);
- done();
- });
+ await flush();
+ assert.isTrue(spy.called);
});
test('primary and secondary actions split properly', () => {
@@ -198,7 +197,7 @@
);
});
- test('plugin revision actions', done => {
+ test('plugin revision actions', async () => {
const stub = stubRestApi('getChangeActionURL').returns(
Promise.resolve('the-url')
);
@@ -206,20 +205,18 @@
'plugin~action': {},
};
assert.isOk(element.revisionActions['plugin~action']);
- flush(() => {
- assert.isTrue(
- stub.calledWith(
- element.changeNum,
- element.latestPatchNum,
- '/plugin~action'
- )
- );
- assert.equal(
- (element.revisionActions['plugin~action'] as UIActionInfo)!.__url,
- 'the-url'
- );
- done();
- });
+ await flush();
+ assert.isTrue(
+ stub.calledWith(
+ element.changeNum,
+ element.latestPatchNum,
+ '/plugin~action'
+ )
+ );
+ assert.equal(
+ (element.revisionActions['plugin~action'] as UIActionInfo)!.__url,
+ 'the-url'
+ );
});
test('plugin change actions', async () => {
@@ -266,71 +263,61 @@
);
});
- test('hide revision action', done => {
- flush(() => {
- const buttonEl = queryAndAssert(element, '[data-action-key="submit"]');
- assert.isOk(buttonEl);
- element.setActionHidden(
- element.ActionType.REVISION,
- element.RevisionActions.SUBMIT,
- true
- );
- assert.lengthOf(element._hiddenActions, 1);
- element.setActionHidden(
- element.ActionType.REVISION,
- element.RevisionActions.SUBMIT,
- true
- );
- assert.lengthOf(element._hiddenActions, 1);
- flush(() => {
- const buttonEl = element.shadowRoot?.querySelector(
- '[data-action-key="submit"]'
- );
- assert.isNotOk(buttonEl);
+ test('hide revision action', async () => {
+ await flush();
+ let buttonEl: Element | undefined = queryAndAssert(
+ element,
+ '[data-action-key="submit"]'
+ );
+ assert.isOk(buttonEl);
+ element.setActionHidden(
+ element.ActionType.REVISION,
+ element.RevisionActions.SUBMIT,
+ true
+ );
+ assert.lengthOf(element._hiddenActions, 1);
+ element.setActionHidden(
+ element.ActionType.REVISION,
+ element.RevisionActions.SUBMIT,
+ true
+ );
+ assert.lengthOf(element._hiddenActions, 1);
+ await flush();
+ buttonEl = query(element, '[data-action-key="submit"]');
+ assert.isNotOk(buttonEl);
- element.setActionHidden(
- element.ActionType.REVISION,
- element.RevisionActions.SUBMIT,
- false
- );
- flush(() => {
- const buttonEl = queryAndAssert(
- element,
- '[data-action-key="submit"]'
- );
- assert.isFalse(buttonEl.hasAttribute('hidden'));
- done();
- });
- });
- });
+ element.setActionHidden(
+ element.ActionType.REVISION,
+ element.RevisionActions.SUBMIT,
+ false
+ );
+ await flush();
+ buttonEl = queryAndAssert(element, '[data-action-key="submit"]');
+ assert.isFalse(buttonEl.hasAttribute('hidden'));
});
- test('buttons exist', done => {
+ test('buttons exist', async () => {
element._loading = false;
- flush(() => {
- const buttonEls = queryAll(element, 'gr-button');
- const menuItems = element.$.moreActions.items;
+ await flush();
+ const buttonEls = queryAll(element, 'gr-button');
+ const menuItems = element.$.moreActions.items;
- // Total button number is one greater than the number of total actions
- // due to the existence of the overflow menu trigger.
- assert.equal(
- buttonEls!.length + menuItems!.length,
- element._allActionValues.length + 1
- );
- assert.isFalse(element.hidden);
- done();
- });
+ // Total button number is one greater than the number of total actions
+ // due to the existence of the overflow menu trigger.
+ assert.equal(
+ buttonEls!.length + menuItems!.length,
+ element._allActionValues.length + 1
+ );
+ assert.isFalse(element.hidden);
});
- test('delete buttons have explicit labels', done => {
- flush(() => {
- const deleteItems = element.$.moreActions.items!.filter(item =>
- item.id!.startsWith('delete')
- );
- assert.equal(deleteItems.length, 1);
- assert.equal(deleteItems[0].name, 'Delete change');
- done();
- });
+ test('delete buttons have explicit labels', async () => {
+ await flush();
+ const deleteItems = element.$.moreActions.items!.filter(item =>
+ item.id!.startsWith('delete')
+ );
+ assert.equal(deleteItems.length, 1);
+ assert.equal(deleteItems[0].name, 'Delete change');
});
test('get revision object from change', () => {
@@ -369,7 +356,7 @@
assert.deepEqual(result, actions);
});
- test('submit change', () => {
+ test('submit change', async () => {
const showSpy = sinon.spy(element, '_showActionDialog');
stubRestApi('getFromProjectLookup').returns(
Promise.resolve('test' as RepoName)
@@ -390,12 +377,15 @@
);
tap(submitButton);
- flush();
+ await flush();
assert.isTrue(showSpy.calledWith(element.$.confirmSubmitDialog));
});
- test('submit change, tap on icon', done => {
- sinon.stub(element.$.confirmSubmitDialog, 'resetFocus').callsFake(done);
+ test('submit change, tap on icon', async () => {
+ const submitted = mockPromise();
+ sinon
+ .stub(element.$.confirmSubmitDialog, 'resetFocus')
+ .callsFake(() => submitted.resolve());
stubRestApi('getFromProjectLookup').returns(
Promise.resolve('test' as RepoName)
);
@@ -414,6 +404,7 @@
'gr-button[data-action-key="submit"] iron-icon'
);
tap(submitIcon);
+ await submitted;
});
test('_handleSubmitConfirm', () => {
@@ -435,19 +426,16 @@
assert.isFalse(fireStub.called);
});
- test('submit change with plugin hook', done => {
+ test('submit change with plugin hook', async () => {
sinon.stub(element, '_canSubmitChange').callsFake(() => false);
const fireActionStub = sinon.stub(element, '_fireAction');
- flush(() => {
- const submitButton = queryAndAssert(
- element,
- 'gr-button[data-action-key="submit"]'
- );
- tap(submitButton);
- assert.equal(fireActionStub.callCount, 0);
-
- done();
- });
+ await flush();
+ const submitButton = queryAndAssert(
+ element,
+ 'gr-button[data-action-key="submit"]'
+ );
+ tap(submitButton);
+ assert.equal(fireActionStub.callCount, 0);
});
test('chain state', () => {
@@ -490,55 +478,51 @@
);
});
- test('rebase change', done => {
+ test('rebase change', async () => {
const fireActionStub = sinon.stub(element, '_fireAction');
const fetchChangesStub = sinon
.stub(element.$.confirmRebase, 'fetchRecentChanges')
.returns(Promise.resolve([]));
element._hasKnownChainState = true;
- flush(() => {
- const rebaseButton = queryAndAssert(
- element,
- 'gr-button[data-action-key="rebase"]'
- );
- tap(rebaseButton);
- const rebaseAction = {
- __key: 'rebase',
- __type: 'revision',
- __primary: false,
- enabled: true,
- label: 'Rebase',
- method: HttpMethod.POST,
- title: 'Rebase onto tip of branch or parent change',
- };
- assert.isTrue(fetchChangesStub.called);
- element._handleRebaseConfirm(
- new CustomEvent('', {detail: {base: '1234'}})
- );
- assert.deepEqual(fireActionStub.lastCall.args, [
- '/rebase',
- assertUIActionInfo(rebaseAction),
- true,
- {base: '1234'},
- ]);
- done();
- });
+ await flush();
+ const rebaseButton = queryAndAssert(
+ element,
+ 'gr-button[data-action-key="rebase"]'
+ );
+ tap(rebaseButton);
+ const rebaseAction = {
+ __key: 'rebase',
+ __type: 'revision',
+ __primary: false,
+ enabled: true,
+ label: 'Rebase',
+ method: HttpMethod.POST,
+ title: 'Rebase onto tip of branch or parent change',
+ };
+ assert.isTrue(fetchChangesStub.called);
+ element._handleRebaseConfirm(
+ new CustomEvent('', {detail: {base: '1234'}})
+ );
+ assert.deepEqual(fireActionStub.lastCall.args, [
+ '/rebase',
+ assertUIActionInfo(rebaseAction),
+ true,
+ {base: '1234'},
+ ]);
});
- test('rebase change fires reload event', done => {
+ test('rebase change fires reload event', async () => {
const eventStub = sinon.stub(element, 'dispatchEvent');
element._handleResponse(
{__key: 'rebase', __type: ActionType.CHANGE, label: 'l'},
new Response()
);
- flush(() => {
- assert.isTrue(eventStub.called);
- assert.equal(eventStub.lastCall.args[0].type, 'reload');
- done();
- });
+ await flush();
+ assert.isTrue(eventStub.called);
+ assert.equal(eventStub.lastCall.args[0].type, 'reload');
});
- test("rebase dialog gets recent changes each time it's opened", done => {
+ test("rebase dialog gets recent changes each time it's opened", async () => {
const fetchChangesStub = sinon
.stub(element.$.confirmRebase, 'fetchRecentChanges')
.returns(Promise.resolve([]));
@@ -550,17 +534,15 @@
tap(rebaseButton);
assert.isTrue(fetchChangesStub.calledOnce);
- flush(() => {
- element.$.confirmRebase.dispatchEvent(
- new CustomEvent('cancel', {
- composed: true,
- bubbles: true,
- })
- );
- tap(rebaseButton);
- assert.isTrue(fetchChangesStub.calledTwice);
- done();
- });
+ await flush();
+ element.$.confirmRebase.dispatchEvent(
+ new CustomEvent('cancel', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+ tap(rebaseButton);
+ assert.isTrue(fetchChangesStub.calledTwice);
});
test('two dialogs are not shown at the same time', async () => {
@@ -624,7 +606,7 @@
});
suite('change edits', () => {
- test('disableEdit', () => {
+ test('disableEdit', async () => {
element.set('editMode', false);
element.set('editPatchsetLoaded', false);
element.change = {
@@ -632,7 +614,7 @@
status: ChangeStatus.NEW,
};
element.set('disableEdit', true);
- flush();
+ await flush();
assert.isNotOk(
query(element, 'gr-button[data-action-key="publishEdit"]')
@@ -647,7 +629,7 @@
assert.isNotOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
});
- test('shows confirm dialog for delete edit', () => {
+ test('shows confirm dialog for delete edit', async () => {
element.set('editMode', true);
element.set('editPatchsetLoaded', true);
@@ -660,12 +642,12 @@
'gr-button[primary]'
)
);
- flush();
+ await flush();
assert.equal(fireActionStub.lastCall.args[0], '/edit');
});
- test('edit patchset is loaded, needs rebase', () => {
+ test('edit patchset is loaded, needs rebase', async () => {
element.set('editMode', true);
element.set('editPatchsetLoaded', true);
element.change = {
@@ -673,7 +655,7 @@
status: ChangeStatus.NEW,
};
element.editBasedOnCurrentPatchSet = false;
- flush();
+ await flush();
assert.isNotOk(
query(element, 'gr-button[data-action-key="publishEdit"]')
@@ -684,7 +666,7 @@
assert.isNotOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
});
- test('edit patchset is loaded, does not need rebase', () => {
+ test('edit patchset is loaded, does not need rebase', async () => {
element.set('editMode', true);
element.set('editPatchsetLoaded', true);
element.change = {
@@ -692,7 +674,7 @@
status: ChangeStatus.NEW,
};
element.editBasedOnCurrentPatchSet = true;
- flush();
+ await flush();
assert.isOk(query(element, 'gr-button[data-action-key="publishEdit"]'));
assert.isNotOk(
@@ -703,14 +685,14 @@
assert.isNotOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
});
- test('edit mode is loaded, no edit patchset', () => {
+ test('edit mode is loaded, no edit patchset', async () => {
element.set('editMode', true);
element.set('editPatchsetLoaded', false);
element.change = {
...createChangeViewChange(),
status: ChangeStatus.NEW,
};
- flush();
+ await flush();
assert.isNotOk(
query(element, 'gr-button[data-action-key="publishEdit"]')
@@ -725,14 +707,14 @@
assert.isOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
});
- test('normal patch set', () => {
+ test('normal patch set', async () => {
element.set('editMode', false);
element.set('editPatchsetLoaded', false);
element.change = {
...createChangeViewChange(),
status: ChangeStatus.NEW,
};
- flush();
+ await flush();
assert.isNotOk(
query(element, 'gr-button[data-action-key="publishEdit"]')
@@ -747,16 +729,17 @@
assert.isNotOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
});
- test('edit action', done => {
+ test('edit action', async () => {
+ const editTapped = mockPromise();
element.addEventListener('edit-tap', () => {
- done();
+ editTapped.resolve();
});
element.set('editMode', true);
element.change = {
...createChangeViewChange(),
status: ChangeStatus.NEW,
};
- flush();
+ await flush();
assert.isNotOk(query(element, 'gr-button[data-action-key="edit"]'));
assert.isOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
@@ -764,7 +747,7 @@
...createChangeViewChange(),
status: ChangeStatus.MERGED,
};
- flush();
+ await flush();
assert.isNotOk(query(element, 'gr-button[data-action-key="edit"]'));
element.change = {
@@ -772,13 +755,14 @@
status: ChangeStatus.NEW,
};
element.set('editMode', false);
- flush();
+ await flush();
const editButton = queryAndAssert(
element,
'gr-button[data-action-key="edit"]'
);
tap(editButton);
+ await editTapped;
});
});
@@ -896,62 +880,57 @@
status: ChangeStatus.NEW,
},
];
- setup(done => {
+ setup(async () => {
stubRestApi('getChanges').returns(Promise.resolve(changes));
element._handleCherrypickTap();
- flush(() => {
- const radioButtons = queryAll(
- element.$.confirmCherrypick,
- "input[name='cherryPickOptions']"
- );
- assert.equal(radioButtons.length, 2);
- tap(radioButtons[1]);
- flush(() => {
- done();
- });
- });
+ await flush();
+ const radioButtons = queryAll(
+ element.$.confirmCherrypick,
+ "input[name='cherryPickOptions']"
+ );
+ assert.equal(radioButtons.length, 2);
+ tap(radioButtons[1]);
+ await flush();
});
- test('cherry pick topic dialog is rendered', done => {
+ test('cherry pick topic dialog is rendered', async () => {
const dialog = element.$.confirmCherrypick;
- flush(() => {
- const changesTable = queryAndAssert(dialog, 'table');
- const headers = Array.from(changesTable.querySelectorAll('th'));
- const expectedHeadings = [
- '',
- 'Change',
- 'Status',
- 'Subject',
- 'Project',
- 'Progress',
- '',
- ];
- const headings = headers.map(header => header.innerText);
- assert.equal(headings.length, expectedHeadings.length);
- for (let i = 0; i < headings.length; i++) {
- assert.equal(headings[i].trim(), expectedHeadings[i]);
- }
- const changeRows = queryAll(changesTable, 'tbody > tr');
- const change = Array.from(changeRows[0].querySelectorAll('td')).map(
- e => e.innerText
- );
- const expectedChange = [
- '',
- '1234567890',
- 'MERGED',
- 'random',
- 'A',
- 'NOT STARTED',
- '',
- ];
- for (let i = 0; i < change.length; i++) {
- assert.equal(change[i].trim(), expectedChange[i]);
- }
- done();
- });
+ await flush();
+ const changesTable = queryAndAssert(dialog, 'table');
+ const headers = Array.from(changesTable.querySelectorAll('th'));
+ const expectedHeadings = [
+ '',
+ 'Change',
+ 'Status',
+ 'Subject',
+ 'Project',
+ 'Progress',
+ '',
+ ];
+ const headings = headers.map(header => header.innerText);
+ assert.equal(headings.length, expectedHeadings.length);
+ for (let i = 0; i < headings.length; i++) {
+ assert.equal(headings[i].trim(), expectedHeadings[i]);
+ }
+ const changeRows = queryAll(changesTable, 'tbody > tr');
+ const change = Array.from(changeRows[0].querySelectorAll('td')).map(
+ e => e.innerText
+ );
+ const expectedChange = [
+ '',
+ '1234567890',
+ 'MERGED',
+ 'random',
+ 'A',
+ 'NOT STARTED',
+ '',
+ ];
+ for (let i = 0; i < change.length; i++) {
+ assert.equal(change[i].trim(), expectedChange[i]);
+ }
});
- test('changes with duplicate project show an error', done => {
+ test('changes with duplicate project show an error', async () => {
const dialog = element.$.confirmCherrypick;
const error = queryAndAssert(
dialog,
@@ -974,13 +953,11 @@
project: 'A' as RepoName,
},
]);
- flush(() => {
- assert.equal(
- error.innerText,
- 'Two changes cannot be of the same' + ' project'
- );
- done();
- });
+ await flush();
+ assert.equal(
+ error.innerText,
+ 'Two changes cannot be of the same' + ' project'
+ );
});
});
});
@@ -1021,24 +998,24 @@
});
});
- test('custom actions', done => {
+ test('custom actions', async () => {
// Add a button with the same key as a server-based one to ensure
// collisions are taken care of.
const key = element.addActionButton(element.ActionType.REVISION, 'Bork!');
- element.addEventListener(key + '-tap', e => {
+ const keyTapped = mockPromise();
+ element.addEventListener(key + '-tap', async e => {
assert.equal(
(e as CustomEvent).detail.node.getAttribute('data-action-key'),
key
);
element.removeActionButton(key);
- flush(() => {
- assert.notOk(query(element, '[data-action-key="' + key + '"]'));
- done();
- });
+ await flush();
+ assert.notOk(query(element, '[data-action-key="' + key + '"]'));
+ keyTapped.resolve();
});
- flush(() => {
- tap(queryAndAssert(element, '[data-action-key="' + key + '"]'));
- });
+ await flush();
+ tap(queryAndAssert(element, '[data-action-key="' + key + '"]'));
+ await keyTapped;
});
test('_setLoadingOnButtonWithKey top-level', () => {
@@ -1095,32 +1072,28 @@
return element.reload();
});
- test('abandon change with message', done => {
+ test('abandon change with message', async () => {
const newAbandonMsg = 'Test Abandon Message';
element.$.confirmAbandonDialog.message = newAbandonMsg;
- flush(() => {
- const abandonButton = queryAndAssert(
- element,
- 'gr-button[data-action-key="abandon"]'
- );
- tap(abandonButton);
+ await flush();
+ const abandonButton = queryAndAssert(
+ element,
+ 'gr-button[data-action-key="abandon"]'
+ );
+ tap(abandonButton);
- assert.equal(element.$.confirmAbandonDialog.message, newAbandonMsg);
- done();
- });
+ assert.equal(element.$.confirmAbandonDialog.message, newAbandonMsg);
});
- test('abandon change with no message', done => {
- flush(() => {
- const abandonButton = queryAndAssert(
- element,
- 'gr-button[data-action-key="abandon"]'
- );
- tap(abandonButton);
+ test('abandon change with no message', async () => {
+ await flush();
+ const abandonButton = queryAndAssert(
+ element,
+ 'gr-button[data-action-key="abandon"]'
+ );
+ tap(abandonButton);
- assert.isUndefined(element.$.confirmAbandonDialog.message);
- done();
- });
+ assert.isUndefined(element.$.confirmAbandonDialog.message);
});
test('works', () => {
@@ -1173,7 +1146,7 @@
return element.reload();
});
- test('revert change with plugin hook', done => {
+ test('revert change with plugin hook', async () => {
const newRevertMsg = 'Modified revert msg';
sinon
.stub(element.$.confirmRevertDialog, '_modifyRevertMsg')
@@ -1204,17 +1177,14 @@
'_populateRevertSubmissionMessage'
)
.callsFake(() => 'original msg');
- flush(() => {
- const revertButton = queryAndAssert(
- element,
- 'gr-button[data-action-key="revert"]'
- );
- tap(revertButton);
- flush(() => {
- assert.equal(element.$.confirmRevertDialog._message, newRevertMsg);
- done();
- });
- });
+ await flush();
+ const revertButton = queryAndAssert(
+ element,
+ 'gr-button[data-action-key="revert"]'
+ );
+ tap(revertButton);
+ await flush();
+ assert.equal(element.$.confirmRevertDialog._message, newRevertMsg);
});
suite('revert change submitted together', () => {
@@ -1243,60 +1213,56 @@
);
});
- test('confirm revert dialog shows both options', done => {
+ test('confirm revert dialog shows both options', async () => {
const revertButton = queryAndAssert(
element,
'gr-button[data-action-key="revert"]'
);
tap(revertButton);
- flush(() => {
- assert.equal(getChangesStub.args[0][1], 'submissionid: "199 0"');
- const confirmRevertDialog = element.$.confirmRevertDialog;
- const revertSingleChangeLabel = queryAndAssert(
- confirmRevertDialog,
- '.revertSingleChange'
- ) as HTMLLabelElement;
- const revertSubmissionLabel = queryAndAssert(
- confirmRevertDialog,
- '.revertSubmission'
- ) as HTMLLabelElement;
- assert(
- revertSingleChangeLabel.innerText.trim() ===
- 'Revert single change'
- );
- assert(
- revertSubmissionLabel.innerText.trim() ===
- 'Revert entire submission (2 Changes)'
- );
- let expectedMsg =
- 'Revert submission 199 0' +
- '\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
- assert.equal(confirmRevertDialog._message, expectedMsg);
- const radioInputs = queryAll(
- confirmRevertDialog,
- 'input[name="revertOptions"]'
- );
- tap(radioInputs[0]);
- flush(() => {
- expectedMsg =
- 'Revert "random commit message"\n\nThis reverts ' +
- 'commit 2000.\n\nReason' +
- ' for revert: <INSERT REASONING HERE>\n';
- assert.equal(confirmRevertDialog._message, expectedMsg);
- done();
- });
- });
+ await flush();
+ assert.equal(getChangesStub.args[0][1], 'submissionid: "199 0"');
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ const revertSingleChangeLabel = queryAndAssert(
+ confirmRevertDialog,
+ '.revertSingleChange'
+ ) as HTMLLabelElement;
+ const revertSubmissionLabel = queryAndAssert(
+ confirmRevertDialog,
+ '.revertSubmission'
+ ) as HTMLLabelElement;
+ assert(
+ revertSingleChangeLabel.innerText.trim() === 'Revert single change'
+ );
+ assert(
+ revertSubmissionLabel.innerText.trim() ===
+ 'Revert entire submission (2 Changes)'
+ );
+ let expectedMsg =
+ 'Revert submission 199 0' +
+ '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>' +
+ '\n' +
+ 'Reverted Changes:' +
+ '\n' +
+ '1234567890:random' +
+ '\n' +
+ '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+ '\n';
+ assert.equal(confirmRevertDialog._message, expectedMsg);
+ const radioInputs = queryAll(
+ confirmRevertDialog,
+ 'input[name="revertOptions"]'
+ );
+ tap(radioInputs[0]);
+ await flush();
+ expectedMsg =
+ 'Revert "random commit message"\n\nThis reverts ' +
+ 'commit 2000.\n\nReason' +
+ ' for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog._message, expectedMsg);
});
- test('submit fails if message is not edited', done => {
+ test('submit fails if message is not edited', async () => {
const revertButton = queryAndAssert(
element,
'gr-button[data-action-key="revert"]'
@@ -1304,69 +1270,58 @@
const confirmRevertDialog = element.$.confirmRevertDialog;
tap(revertButton);
const fireStub = sinon.stub(confirmRevertDialog, 'dispatchEvent');
- flush(() => {
- const confirmButton = queryAndAssert(
- queryAndAssert(element.$.confirmRevertDialog, 'gr-dialog'),
- '#confirm'
- );
- tap(confirmButton);
- flush(() => {
- assert.isTrue(confirmRevertDialog._showErrorMessage);
- assert.isFalse(fireStub.called);
- done();
- });
- });
+ await flush();
+ const confirmButton = queryAndAssert(
+ queryAndAssert(element.$.confirmRevertDialog, 'gr-dialog'),
+ '#confirm'
+ );
+ tap(confirmButton);
+ await flush();
+ assert.isTrue(confirmRevertDialog._showErrorMessage);
+ assert.isFalse(fireStub.called);
});
- test('message modification is retained on switching', done => {
+ test('message modification is retained on switching', async () => {
const revertButton = queryAndAssert(
element,
'gr-button[data-action-key="revert"]'
);
const confirmRevertDialog = element.$.confirmRevertDialog;
tap(revertButton);
- flush(() => {
- const radioInputs = queryAll(
- confirmRevertDialog,
- 'input[name="revertOptions"]'
- );
- const revertSubmissionMsg =
- 'Revert submission 199 0' +
- '\n\n' +
- 'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
- const singleChangeMsg =
- 'Revert "random commit message"\n\nThis reverts ' +
- 'commit 2000.\n\nReason' +
- ' for revert: <INSERT REASONING HERE>\n';
- assert.equal(confirmRevertDialog._message, revertSubmissionMsg);
- const newRevertMsg = revertSubmissionMsg + 'random';
- const newSingleChangeMsg = singleChangeMsg + 'random';
- confirmRevertDialog._message = newRevertMsg;
- tap(radioInputs[0]);
- flush(() => {
- assert.equal(confirmRevertDialog._message, singleChangeMsg);
- confirmRevertDialog._message = newSingleChangeMsg;
- tap(radioInputs[1]);
- flush(() => {
- assert.equal(confirmRevertDialog._message, newRevertMsg);
- tap(radioInputs[0]);
- flush(() => {
- assert.equal(
- confirmRevertDialog._message,
- newSingleChangeMsg
- );
- done();
- });
- });
- });
- });
+ await flush();
+ const radioInputs = queryAll(
+ confirmRevertDialog,
+ 'input[name="revertOptions"]'
+ );
+ const revertSubmissionMsg =
+ 'Revert submission 199 0' +
+ '\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>' +
+ '\n' +
+ 'Reverted Changes:' +
+ '\n' +
+ '1234567890:random' +
+ '\n' +
+ '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+ '\n';
+ const singleChangeMsg =
+ 'Revert "random commit message"\n\nThis reverts ' +
+ 'commit 2000.\n\nReason' +
+ ' for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog._message, revertSubmissionMsg);
+ const newRevertMsg = revertSubmissionMsg + 'random';
+ const newSingleChangeMsg = singleChangeMsg + 'random';
+ confirmRevertDialog._message = newRevertMsg;
+ tap(radioInputs[0]);
+ await flush();
+ assert.equal(confirmRevertDialog._message, singleChangeMsg);
+ confirmRevertDialog._message = newSingleChangeMsg;
+ tap(radioInputs[1]);
+ await flush();
+ assert.equal(confirmRevertDialog._message, newRevertMsg);
+ tap(radioInputs[0]);
+ await flush();
+ assert.equal(confirmRevertDialog._message, newSingleChangeMsg);
});
});
@@ -1389,7 +1344,7 @@
);
});
- test('submit fails if message is not edited', done => {
+ test('submit fails if message is not edited', async () => {
const revertButton = queryAndAssert(
element,
'gr-button[data-action-key="revert"]'
@@ -1397,55 +1352,46 @@
const confirmRevertDialog = element.$.confirmRevertDialog;
tap(revertButton);
const fireStub = sinon.stub(confirmRevertDialog, 'dispatchEvent');
- flush(() => {
- const confirmButton = queryAndAssert(
- queryAndAssert(element.$.confirmRevertDialog, 'gr-dialog'),
- '#confirm'
- );
- tap(confirmButton);
- flush(() => {
- assert.isTrue(confirmRevertDialog._showErrorMessage);
- assert.isFalse(fireStub.called);
- done();
- });
- });
+ await flush();
+ const confirmButton = queryAndAssert(
+ queryAndAssert(element.$.confirmRevertDialog, 'gr-dialog'),
+ '#confirm'
+ );
+ tap(confirmButton);
+ await flush();
+ assert.isTrue(confirmRevertDialog._showErrorMessage);
+ assert.isFalse(fireStub.called);
});
- test('confirm revert dialog shows no radio button', done => {
+ test('confirm revert dialog shows no radio button', async () => {
const revertButton = queryAndAssert(
element,
'gr-button[data-action-key="revert"]'
);
tap(revertButton);
- flush(() => {
- const confirmRevertDialog = element.$.confirmRevertDialog;
- const radioInputs = queryAll(
- confirmRevertDialog,
- 'input[name="revertOptions"]'
- );
- assert.equal(radioInputs.length, 0);
- const msg =
- 'Revert "random commit message"\n\n' +
- 'This reverts commit 2000.\n\nReason ' +
- 'for revert: <INSERT REASONING HERE>\n';
- assert.equal(confirmRevertDialog._message, msg);
- const editedMsg = msg + 'hello';
- confirmRevertDialog._message += 'hello';
- const confirmButton = queryAndAssert(
- queryAndAssert(element.$.confirmRevertDialog, 'gr-dialog'),
- '#confirm'
- );
- tap(confirmButton);
- flush(() => {
- assert.equal(fireActionStub.getCall(0).args[0], '/revert');
- assert.equal(fireActionStub.getCall(0).args[1].__key, 'revert');
- assert.equal(
- fireActionStub.getCall(0).args[3].message,
- editedMsg
- );
- done();
- });
- });
+ await flush();
+ const confirmRevertDialog = element.$.confirmRevertDialog;
+ const radioInputs = queryAll(
+ confirmRevertDialog,
+ 'input[name="revertOptions"]'
+ );
+ assert.equal(radioInputs.length, 0);
+ const msg =
+ 'Revert "random commit message"\n\n' +
+ 'This reverts commit 2000.\n\nReason ' +
+ 'for revert: <INSERT REASONING HERE>\n';
+ assert.equal(confirmRevertDialog._message, msg);
+ const editedMsg = msg + 'hello';
+ confirmRevertDialog._message += 'hello';
+ const confirmButton = queryAndAssert(
+ queryAndAssert(element.$.confirmRevertDialog, 'gr-dialog'),
+ '#confirm'
+ );
+ tap(confirmButton);
+ await flush();
+ assert.equal(fireActionStub.getCall(0).args[0], '/revert');
+ assert.equal(fireActionStub.getCall(0).args[1].__key, 'revert');
+ assert.equal(fireActionStub.getCall(0).args[3].message, editedMsg);
});
});
});
@@ -1477,27 +1423,23 @@
test(
'make sure the mark private change button is not outside of the ' +
'overflow menu',
- done => {
- flush(() => {
- assert.isNotOk(query(element, '[data-action-key="private"]'));
- done();
- });
+ async () => {
+ await flush();
+ assert.isNotOk(query(element, '[data-action-key="private"]'));
}
);
- test('private change', done => {
- flush(() => {
- assert.isOk(
- query(element.$.moreActions, 'span[data-id="private-change"]')
- );
- element.setActionOverflow(ActionType.CHANGE, 'private', false);
- flush();
- assert.isOk(query(element, '[data-action-key="private"]'));
- assert.isNotOk(
- query(element.$.moreActions, 'span[data-id="private-change"]')
- );
- done();
- });
+ test('private change', async () => {
+ await flush();
+ assert.isOk(
+ query(element.$.moreActions, 'span[data-id="private-change"]')
+ );
+ element.setActionOverflow(ActionType.CHANGE, 'private', false);
+ await flush();
+ assert.isOk(query(element, '[data-action-key="private"]'));
+ assert.isNotOk(
+ query(element.$.moreActions, 'span[data-id="private-change"]')
+ );
});
});
@@ -1528,35 +1470,23 @@
test(
'make sure the unmark private change button is not outside of the ' +
'overflow menu',
- done => {
- flush(() => {
- assert.isNotOk(
- query(element, '[data-action-key="private.delete"]')
- );
- done();
- });
+ async () => {
+ await flush();
+ assert.isNotOk(query(element, '[data-action-key="private.delete"]'));
}
);
- test('unmark the private change', done => {
- flush(() => {
- assert.isOk(
- query(
- element.$.moreActions,
- 'span[data-id="private.delete-change"]'
- )
- );
- element.setActionOverflow(ActionType.CHANGE, 'private.delete', false);
- flush();
- assert.isOk(query(element, '[data-action-key="private.delete"]'));
- assert.isNotOk(
- query(
- element.$.moreActions,
- 'span[data-id="private.delete-change"]'
- )
- );
- done();
- });
+ test('unmark the private change', async () => {
+ await flush();
+ assert.isOk(
+ query(element.$.moreActions, 'span[data-id="private.delete-change"]')
+ );
+ element.setActionOverflow(ActionType.CHANGE, 'private.delete', false);
+ await flush();
+ assert.isOk(query(element, '[data-action-key="private.delete"]'));
+ assert.isNotOk(
+ query(element.$.moreActions, 'span[data-id="private.delete-change"]')
+ );
});
});
@@ -1586,7 +1516,7 @@
assert.isFalse(fireActionStub.called);
});
- test('shows confirm dialog', () => {
+ test('shows confirm dialog', async () => {
element._handleDeleteTap();
assert.isFalse(
(queryAndAssert(element, '#confirmDeleteDialog') as GrDialog).hidden
@@ -1597,11 +1527,11 @@
'gr-button[primary]'
)
);
- flush();
+ await flush();
assert.isTrue(fireActionStub.calledWith('/', deleteAction, false));
});
- test('hides delete confirm on cancel', () => {
+ test('hides delete confirm on cancel', async () => {
element._handleDeleteTap();
tap(
queryAndAssert(
@@ -1609,7 +1539,7 @@
'gr-button:not([primary])'
)
);
- flush();
+ await flush();
assert.isTrue(
(queryAndAssert(element, '#confirmDeleteDialog') as GrDialog).hidden
);
@@ -1618,7 +1548,7 @@
});
suite('ignore change', () => {
- setup(done => {
+ setup(async () => {
sinon.stub(element, '_fireAction');
const IgnoreAction = {
@@ -1638,19 +1568,18 @@
element.changeNum = 2 as NumericChangeId;
element.latestPatchNum = 2 as PatchSetNum;
- element.reload().then(() => {
- flush(done);
- });
+ await element.reload();
+ await flush();
});
test('make sure the ignore button is not outside of the overflow menu', () => {
assert.isNotOk(query(element, '[data-action-key="ignore"]'));
});
- test('ignoring change', () => {
+ test('ignoring change', async () => {
queryAndAssert(element.$.moreActions, 'span[data-id="ignore-change"]');
element.setActionOverflow(ActionType.CHANGE, 'ignore', false);
- flush();
+ await flush();
queryAndAssert(element, '[data-action-key="ignore"]');
assert.isNotOk(
query(element.$.moreActions, 'span[data-id="ignore-change"]')
@@ -1659,7 +1588,7 @@
});
suite('unignore change', () => {
- setup(done => {
+ setup(async () => {
sinon.stub(element, '_fireAction');
const UnignoreAction = {
@@ -1679,21 +1608,20 @@
element.changeNum = 2 as NumericChangeId;
element.latestPatchNum = 2 as PatchSetNum;
- element.reload().then(() => {
- flush(done);
- });
+ await element.reload();
+ await flush();
});
test('unignore button is not outside of the overflow menu', () => {
assert.isNotOk(query(element, '[data-action-key="unignore"]'));
});
- test('unignoring change', () => {
+ test('unignoring change', async () => {
assert.isOk(
query(element.$.moreActions, 'span[data-id="unignore-change"]')
);
element.setActionOverflow(ActionType.CHANGE, 'unignore', false);
- flush();
+ await flush();
assert.isOk(query(element, '[data-action-key="unignore"]'));
assert.isNotOk(
query(element.$.moreActions, 'span[data-id="unignore-change"]')
@@ -1702,7 +1630,7 @@
});
suite('quick approve', () => {
- setup(() => {
+ setup(async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1719,7 +1647,7 @@
foo: ['-1', ' 0', '+1'],
},
};
- flush();
+ await flush();
});
test('added when can approve', () => {
@@ -1730,7 +1658,7 @@
assert.isNotNull(approveButton);
});
- test('hide quick approve', () => {
+ test('hide quick approve', async () => {
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -1740,7 +1668,7 @@
// Assert approve button gets removed from list of buttons.
element.hideQuickApproveAction();
- flush();
+ await flush();
const approveButtonUpdated = query(
element,
"gr-button[data-action-key='review']"
@@ -1756,18 +1684,21 @@
assert.equal(approveButton!.getAttribute('data-label'), 'foo+1');
});
- test('not added when change is merged', () => {
- element.change!.status = ChangeStatus.MERGED;
- flush(() => {
- const approveButton = query(
- element,
- "gr-button[data-action-key='review']"
- );
- assert.isNotOk(approveButton);
- });
+ test('not added when change is merged', async () => {
+ element.change = {
+ ...element.change!,
+ status: ChangeStatus.MERGED,
+ };
+
+ await flush();
+ const approveButton = query(
+ element,
+ "gr-button[data-action-key='review']"
+ );
+ assert.isNotOk(approveButton);
});
- test('not added when already approved', () => {
+ test('not added when already approved', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1781,7 +1712,7 @@
foo: [' 0', '+1'],
},
};
- flush();
+ await flush();
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -1789,7 +1720,7 @@
assert.isNotOk(approveButton);
});
- test('not added when label not permitted', () => {
+ test('not added when label not permitted', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1800,7 +1731,7 @@
bar: [],
},
};
- flush();
+ await flush();
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -1808,17 +1739,17 @@
assert.isNotOk(approveButton);
});
- test('approves when tapped', () => {
+ test('approves when tapped', async () => {
const fireActionStub = sinon.stub(element, '_fireAction');
tap(queryAndAssert(element, "gr-button[data-action-key='review']"));
- flush();
+ await flush();
assert.isTrue(fireActionStub.called);
assert.isTrue(fireActionStub.calledWith('/review'));
const payload = fireActionStub.lastCall.args[3];
assert.deepEqual((payload as ReviewInput).labels, {foo: 1});
});
- test('not added when multiple labels are required without code review', () => {
+ test('not added when multiple labels are required without code review', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1831,7 +1762,7 @@
bar: [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -1839,7 +1770,7 @@
assert.isNotOk(approveButton);
});
- test('code review shown with multiple missing approval', () => {
+ test('code review shown with multiple missing approval', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1861,7 +1792,7 @@
'Code-Review': [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = queryAndAssert(
element,
"gr-button[data-action-key='review']"
@@ -1869,7 +1800,7 @@
assert.isOk(approveButton);
});
- test('button label for missing approval', () => {
+ test('button label for missing approval', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1887,7 +1818,7 @@
bar: [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = queryAndAssert(
element,
"gr-button[data-action-key='review']"
@@ -1895,7 +1826,7 @@
assert.equal(approveButton.getAttribute('data-label'), 'foo+1');
});
- test('no quick approve if score is not maximal for a label', () => {
+ test('no quick approve if score is not maximal for a label', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1913,7 +1844,7 @@
bar: [' 0', '+1'],
},
};
- flush();
+ await flush();
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -1921,7 +1852,7 @@
assert.isNotOk(approveButton);
});
- test('approving label with a non-max score', () => {
+ test('approving label with a non-max score', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1939,7 +1870,7 @@
bar: [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = queryAndAssert(
element,
"gr-button[data-action-key='review']"
@@ -1947,7 +1878,7 @@
assert.equal(approveButton.getAttribute('data-label'), 'bar+2');
});
- test('added when can approve an already-approved code review label', () => {
+ test('added when can approve an already-approved code review label', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -1965,7 +1896,7 @@
'Code-Review': [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = queryAndAssert(
element,
"gr-button[data-action-key='review']"
@@ -1973,7 +1904,7 @@
assert.isNotNull(approveButton);
});
- test('not added when the user has already approved', () => {
+ test('not added when the user has already approved', async () => {
const vote = {
...createApproval(),
_account_id: 123 as AccountId,
@@ -1998,7 +1929,7 @@
'Code-Review': [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -2006,7 +1937,7 @@
assert.isNotOk(approveButton);
});
- test('not added when user owns the change', () => {
+ test('not added when user owns the change', async () => {
element.change = {
...createChangeViewChange(),
current_revision: 'abc1234' as CommitId,
@@ -2025,7 +1956,7 @@
'Code-Review': [' 0', '+1', '+2'],
},
};
- flush();
+ await flush();
const approveButton = query(
element,
"gr-button[data-action-key='review']"
@@ -2034,12 +1965,12 @@
});
});
- test('adds download revision action', () => {
+ test('adds download revision action', async () => {
const handler = sinon.stub();
element.addEventListener('download-tap', handler);
assert.ok(element.revisionActions.download);
element._handleDownloadTap();
- flush();
+ await flush();
assert.isTrue(handler.called);
});
@@ -2061,14 +1992,14 @@
});
suite('setActionOverflow', () => {
- test('move action from overflow', () => {
+ test('move action from overflow', async () => {
assert.isNotOk(query(element, '[data-action-key="cherrypick"]'));
assert.strictEqual(
element.$.moreActions!.items![0].id,
'cherrypick-revision'
);
element.setActionOverflow(ActionType.REVISION, 'cherrypick', false);
- flush();
+ await flush();
assert.isOk(query(element, '[data-action-key="cherrypick"]'));
assert.notEqual(
element.$.moreActions!.items![0].id,
@@ -2076,10 +2007,10 @@
);
});
- test('move action to overflow', () => {
+ test('move action to overflow', async () => {
assert.isOk(query(element, '[data-action-key="submit"]'));
element.setActionOverflow(ActionType.REVISION, 'submit', true);
- flush();
+ await flush();
assert.isNotOk(query(element, '[data-action-key="submit"]'));
assert.strictEqual(
element.$.moreActions.items![3].id,
@@ -2233,31 +2164,24 @@
);
});
- test('revert submission single change', done => {
- element
- ._send(
- HttpMethod.POST,
- {message: 'Revert submission'},
- '/revert_submission',
- false,
- cleanup,
- {} as UIActionInfo
- )
- .then(() => {
- element
- ._handleResponse(
- {
- __key: 'revert_submission',
- __type: ActionType.CHANGE,
- label: 'l',
- },
- new Response()
- )!
- .then(() => {
- assert.isTrue(navigateToSearchQueryStub.called);
- done();
- });
- });
+ test('revert submission single change', async () => {
+ await element._send(
+ HttpMethod.POST,
+ {message: 'Revert submission'},
+ '/revert_submission',
+ false,
+ cleanup,
+ {} as UIActionInfo
+ );
+ await element._handleResponse(
+ {
+ __key: 'revert_submission',
+ __type: ActionType.CHANGE,
+ label: 'l',
+ },
+ new Response()
+ );
+ assert.isTrue(navigateToSearchQueryStub.called);
});
});
@@ -2280,55 +2204,42 @@
);
});
- test('revert submission multiple change', done => {
- element
- ._send(
- HttpMethod.POST,
- {message: 'Revert submission'},
- '/revert_submission',
- false,
- cleanup,
- {} as UIActionInfo
- )
- .then(() => {
- element
- ._handleResponse(
- {
- __key: 'revert_submission',
- __type: ActionType.CHANGE,
- label: 'l',
- },
- new Response()
- )!
- .then(() => {
- assert.isFalse(showActionDialogStub.called);
- assert.isTrue(
- navigateToSearchQueryStub.calledWith('topic: T')
- );
- done();
- });
- });
+ test('revert submission multiple change', async () => {
+ await element._send(
+ HttpMethod.POST,
+ {message: 'Revert submission'},
+ '/revert_submission',
+ false,
+ cleanup,
+ {} as UIActionInfo
+ );
+ await element._handleResponse(
+ {
+ __key: 'revert_submission',
+ __type: ActionType.CHANGE,
+ label: 'l',
+ },
+ new Response()
+ );
+ assert.isFalse(showActionDialogStub.called);
+ assert.isTrue(navigateToSearchQueryStub.calledWith('topic: T'));
});
});
- test('revision action', done => {
- element
- ._send(
- HttpMethod.DELETE,
- payload,
- '/endpoint',
- true,
- cleanup,
- {} as UIActionInfo
- )
- .then(() => {
- assert.isFalse(onShowError.called);
- assert.isTrue(cleanup.calledOnce);
- assert.isTrue(
- sendStub.calledWith(42, 'DELETE', '/endpoint', 12, payload)
- );
- done();
- });
+ test('revision action', async () => {
+ await element._send(
+ HttpMethod.DELETE,
+ payload,
+ '/endpoint',
+ true,
+ cleanup,
+ {} as UIActionInfo
+ );
+ assert.isFalse(onShowError.called);
+ assert.isTrue(cleanup.calledOnce);
+ assert.isTrue(
+ sendStub.calledWith(42, 'DELETE', '/endpoint', 12, payload)
+ );
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index 3d55097..e1a1cbb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -86,6 +86,7 @@
AutocompleteSuggestion,
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
+import {Interaction} from '../../../constants/reporting';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -619,7 +620,7 @@
_onShowAllClick() {
this._showAllSections = !this._showAllSections;
- this.reporting.reportInteraction('toggle show all button', {
+ this.reporting.reportInteraction(Interaction.TOGGLE_SHOW_ALL_BUTTON, {
sectionName: 'metadata',
toState: this._showAllSections ? 'Show all' : 'Show less',
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.ts b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.ts
index 1d7db85..7fc3ddf 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.ts
@@ -35,6 +35,7 @@
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {appContext} from '../../../services/app-context';
import {labelCompare} from '../../../utils/label-util';
+import {Interaction} from '../../../constants/reporting';
interface ChangeRequirement extends Requirement {
satisfied: boolean;
@@ -181,7 +182,7 @@
_handleShowHide() {
this._showOptionalLabels = !this._showOptionalLabels;
- this.reporting.reportInteraction('toggle show all button', {
+ this.reporting.reportInteraction(Interaction.TOGGLE_SHOW_ALL_BUTTON, {
sectionName: 'optional labels',
toState: this._showOptionalLabels ? 'Show all' : 'Show less',
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index bd9103e..c34a6ae 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -327,10 +327,9 @@
margin-bottom: var(--spacing-m);
}
.zeroState {
- color: var(--primary-text-color);
+ color: var(--deemphasized-text-color);
}
.loading.zeroState {
- color: var(--deemphasized-text-color);
margin-right: var(--spacing-m);
}
div.error {
@@ -561,7 +560,7 @@
account =>
html`<gr-avatar
.account="${account}"
- image-size="32"
+ imageSize="32"
></gr-avatar>`
)}
${countUnresolvedComments} unresolved</gr-summary-chip
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 659d5bd..58cf489 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -72,7 +72,7 @@
hasEditPatchsetLoaded,
PatchSet,
} from '../../../utils/patch-set-util';
-import {changeStatuses} from '../../../utils/change-util';
+import {changeStatuses, isOwner, isReviewer} from '../../../utils/change-util';
import {EventType as PluginEventType} from '../../../api/plugin';
import {customElement, property, observe} from '@polymer/decorators';
import {GrApplyFixDialog} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
@@ -172,7 +172,7 @@
import {aPluginHasRegistered$} from '../../../services/checks/checks-model';
import {Subject} from 'rxjs';
import {debounce, DelayedTask} from '../../../utils/async-util';
-import {Timing} from '../../../constants/reporting';
+import {Interaction, Timing} from '../../../constants/reporting';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
@@ -634,7 +634,7 @@
this._dynamicTabContentEndpoints.length !==
this._dynamicTabHeaderEndpoints.length
) {
- console.warn('Different number of tab headers and tab content.');
+ this.reporting.error(new Error('Mismatch of headers and content.'));
}
})
.then(() => this._initActiveTabs(this.params));
@@ -767,7 +767,7 @@
}
}
if (activeIndex === -1) {
- console.warn('tab not found with given info', activeDetails);
+ this.reporting.error(new Error(`tab not found for ${activeDetails}`));
return;
}
const tabName = tabs[activeIndex].dataset['name'];
@@ -777,7 +777,7 @@
if (paperTabs.selected !== activeIndex) {
// paperTabs.selected is undefined during rendering
if (paperTabs.selected !== undefined) {
- this.reporting.reportInteraction('show-tab', {tabName, src});
+ this.reporting.reportInteraction(Interaction.SHOW_TAB, {tabName, src});
}
paperTabs.selected = activeIndex;
}
@@ -840,7 +840,7 @@
if (hasUnresolvedThreads) this.unresolvedOnly = true;
}
- this.reporting.reportInteraction('show-tab', {
+ this.reporting.reportInteraction(Interaction.SHOW_TAB, {
tabName,
src: 'paper-tab-click',
});
@@ -1225,10 +1225,7 @@
this._shownFileCount = e.detail.length;
}
- _expandAllDiffs(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
- return;
- }
+ _expandAllDiffs() {
this.$.fileList.expandAllDiffs();
}
@@ -2133,7 +2130,10 @@
.then(() => {
this.reporting.timeEnd(Timing.CHANGE_RELOAD);
if (isLocationChange) {
- this.reporting.changeDisplayed();
+ this.reporting.changeDisplayed({
+ isOwner: isOwner(this._change, this._account),
+ isReviewer: isReviewer(this._change, this._account),
+ });
}
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 532097b..ff303e3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -44,6 +44,7 @@
import {DiffViewMode} from '../../../constants/constants';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fireEvent} from '../../../utils/event-util';
+import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
declare global {
interface HTMLElementTagNameMap {
@@ -60,7 +61,7 @@
}
@customElement('gr-file-list-header')
-export class GrFileListHeader extends PolymerElement {
+export class GrFileListHeader extends KeyboardShortcutMixin(PolymerElement) {
static get template() {
return htmlTemplate;
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
index 69be729..8b9b80c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
@@ -173,6 +173,7 @@
class="download"
title="[[createTitle(Shortcut.OPEN_DOWNLOAD_DIALOG,
ShortcutSection.ACTIONS)]]"
+ has-tooltip=""
on-click="_handleDownloadTap"
>Download</gr-button
>
@@ -186,6 +187,7 @@
link=""
title="[[createTitle(Shortcut.TOGGLE_ALL_INLINE_DIFFS,
ShortcutSection.FILE_LIST)]]"
+ has-tooltip=""
on-click="_expandAllDiffs"
>Expand All</gr-button
>
@@ -195,6 +197,7 @@
on-click="_collapseAllDiffs"
title="[[createTitle(Shortcut.TOGGLE_ALL_INLINE_DIFFS,
ShortcutSection.FILE_LIST)]]"
+ has-tooltip=""
>Collapse All</gr-button
>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index f61e024..5ce6e3b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -385,25 +385,19 @@
this._dynamicHeaderEndpoints.length !==
this._dynamicContentEndpoints.length
) {
- console.warn(
- 'Different number of dynamic file-list header and content.'
- );
+ this.reporting.error(new Error('dynamic header/content mismatch'));
}
if (
this._dynamicPrependedHeaderEndpoints.length !==
this._dynamicPrependedContentEndpoints.length
) {
- console.warn(
- 'Different number of dynamic file-list header and content.'
- );
+ this.reporting.error(new Error('dynamic header/content mismatch'));
}
if (
this._dynamicHeaderEndpoints.length !==
this._dynamicSummaryEndpoints.length
) {
- console.warn(
- 'Different number of dynamic file-list headers and summary.'
- );
+ this.reporting.error(new Error('dynamic header/content mismatch'));
}
});
}
@@ -1429,7 +1423,9 @@
console.info('Expanding diff', iter, 'of', initialCount, ':', path);
const diffElem = this._findDiffByPath(path, diffElements);
if (!diffElem) {
- console.warn(`Did not find <gr-diff-host> element for ${path}`);
+ this.reporting.error(
+ new Error(`Did not find <gr-diff-host> element for ${path}`)
+ );
return Promise.resolve();
}
if (!this.changeComments || !this.patchRange || !this.diffPrefs) {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index 4a41316..57c1a30 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -20,7 +20,7 @@
<style include="gr-voting-styles">
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
</style>
- <style include="shared-styles">
+ <style>
:host {
display: block;
position: relative;
@@ -148,6 +148,7 @@
vertical-align: top;
}
.score {
+ box-sizing: border-box;
border-radius: var(--border-radius);
color: var(--vote-text-color);
display: inline-block;
@@ -199,6 +200,10 @@
font-weight: var(--font-weight-bold);
}
}
+ iron-icon {
+ --iron-icon-height: 20px;
+ --iron-icon-width: 20px;
+ }
@media screen and (max-width: 50em) {
.expanded .content {
padding-left: 0;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
index 6be0c07..4e2aaf1 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -270,7 +270,9 @@
| undefined;
if (!el && this._showAllActivity) {
- console.warn(`Failed to scroll to message: ${messageID}`);
+ this.reporting.error(
+ new Error(`Failed to scroll to message: ${messageID}`)
+ );
return;
}
if (!el) {
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 9c9b21a..360d7de 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -40,6 +40,7 @@
getRevisionKey,
isChangeInfo,
} from '../../../utils/change-util';
+import {Interaction} from '../../../constants/reporting';
/** What is the maximum number of shown changes in collapsed list? */
const DEFALT_NUM_CHANGES_WHEN_COLLAPSED = 3;
@@ -746,7 +747,7 @@
private toggle(e: MouseEvent) {
e.stopPropagation();
this.showAll = !this.showAll;
- this.reporting.reportInteraction('toggle show all button', {
+ this.reporting.reportInteraction(Interaction.TOGGLE_SHOW_ALL_BUTTON, {
sectionName: this.title,
toState: this.showAll ? 'Show all' : 'Show less',
});
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 5504877..939b23c 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -113,7 +113,7 @@
import {ErrorCallback} from '../../../api/rest';
import {debounce, DelayedTask} from '../../../utils/async-util';
import {StorageLocation} from '../../../services/storage/gr-storage';
-import {Timing} from '../../../constants/reporting';
+import {Interaction, Timing} from '../../../constants/reporting';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -758,7 +758,7 @@
if (changeReviewers) {
for (const key of Object.keys(changeReviewers)) {
if (key !== 'REVIEWER' && key !== 'CC') {
- console.warn('unexpected reviewer state:', key);
+ this.reporting.error(new Error(`Unexpected reviewer state: ${key}`));
continue;
}
if (!changeReviewers[key]) continue;
@@ -820,12 +820,12 @@
if (this._newAttentionSet.has(id)) {
this._newAttentionSet.delete(id);
- this.reporting.reportInteraction('attention-set-chip', {
+ this.reporting.reportInteraction(Interaction.ATTENTION_SET_CHIP, {
action: `REMOVE${self}${role}`,
});
} else {
this._newAttentionSet.add(id);
- this.reporting.reportInteraction('attention-set-chip', {
+ this.reporting.reportInteraction(Interaction.ATTENTION_SET_CHIP, {
action: `ADD${self}${role}`,
});
}
@@ -1085,9 +1085,8 @@
} else if (isReviewerGroupSuggestion(suggestion)) {
entry = suggestion.group;
} else {
- console.warn(
- 'received suggestion that was neither account nor group:',
- suggestion
+ this.reporting.error(
+ new Error(`Suggestion is neither account nor group: ${suggestion}`)
);
return false;
}
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
index 17e5e26..c3c1d54 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
@@ -391,8 +391,8 @@
account="[[account]]"
force-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]"
selected="[[_computeHasNewAttention(account, _newAttentionSet)]]"
- deselected="[[!_computeHasNewAttention(account, _newAttentionSet)]]"
hide-hovercard=""
+ selection-chip-style
on-click="_handleAttentionClick"
></gr-account-label>
</template>
@@ -462,8 +462,8 @@
account="[[_owner]]"
force-attention="[[_computeHasNewAttention(_owner, _newAttentionSet)]]"
selected="[[_computeHasNewAttention(_owner, _newAttentionSet)]]"
- deselected="[[!_computeHasNewAttention(_owner, _newAttentionSet)]]"
hide-hovercard=""
+ selection-chip-style
on-click="_handleAttentionClick"
>
</gr-account-label>
@@ -477,8 +477,8 @@
account="[[_uploader]]"
force-attention="[[_computeHasNewAttention(_uploader, _newAttentionSet)]]"
selected="[[_computeHasNewAttention(_uploader, _newAttentionSet)]]"
- deselected="[[!_computeHasNewAttention(_uploader, _newAttentionSet)]]"
hide-hovercard=""
+ selection-chip-style
on-click="_handleAttentionClick"
>
</gr-account-label>
@@ -496,9 +496,9 @@
<gr-account-label
account="[[account]]"
force-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]"
- selected="[[_computeHasNewAttention(account, _newAttentionSet)]]"
- deselected="[[!_computeHasNewAttention(account, _newAttentionSet)]]"
+ selected="[[_computeHasNewAttention(account, _newAttentionSet)]]
hide-hovercard=""
+ selection-chip-style
on-click="_handleAttentionClick"
>
</gr-account-label>
@@ -518,8 +518,8 @@
account="[[account]]"
force-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]"
selected="[[_computeHasNewAttention(account, _newAttentionSet)]]"
- deselected="[[!_computeHasNewAttention(account, _newAttentionSet)]]"
hide-hovercard=""
+ selection-chip-style
on-click="_handleAttentionClick"
>
</gr-account-label>
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 2f4adf5..8283be79 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -112,7 +112,7 @@
}
_computeEmptyThreadsMessage(threads: CommentThread[]) {
- return !threads.length ? 'No comments.' : 'No unresolved comments';
+ return !threads.length ? 'No comments' : 'No unresolved comments';
}
_showPartyPopper(threads: CommentThread[]) {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
index 1fe00ff..3d6cfd4 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
@@ -611,7 +611,7 @@
test('default empty message should show', () => {
assert.isTrue(
element.shadowRoot.querySelector('#threads').textContent.trim()
- .includes('No comments.'));
+ .includes('No comments'));
});
});
});
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 53d25ec..7796a6e 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -40,24 +40,24 @@
} from '../../api/checks';
import {sharedStyles} from '../../styles/shared-styles';
import {
+ CheckRun,
+ checksSelectedPatchsetNumber$,
+ RunResult,
+ someProvidersAreLoadingSelected$,
topLevelActionsSelected$,
topLevelLinksSelected$,
- CheckRun,
- RunResult,
- checksSelectedPatchsetNumber$,
- someProvidersAreLoadingSelected$,
} from '../../services/checks/checks-model';
import {
allResults,
fireActionTriggered,
+ firstPrimaryLink,
hasCompletedWithoutResults,
iconForCategory,
iconForLink,
otherPrimaryLinks,
- firstPrimaryLink,
primaryRunAction,
- tooltipForLink,
secondaryLinks,
+ tooltipForLink,
} from '../../services/checks/checks-util';
import {assertIsDefined, check} from '../../utils/common-util';
import {toggleClass, whenVisible} from '../../utils/dom-util';
@@ -928,10 +928,14 @@
private scrollElIntoView(selector: string) {
this.updateComplete.then(() => {
let el = this.shadowRoot?.querySelector(selector);
- // <gr-result-row> has display:contents and cannot be scrolled into view
- // itself. Thus we are preferring to scroll the first child into view.
- el = el?.shadowRoot?.firstElementChild ?? el;
- el?.scrollIntoView({block: 'center'});
+ // el might be a <gr-result-row> with an empty shadowRoot. Let's wait a
+ // moment before trying to find a child element in it.
+ setTimeout(() => {
+ // <gr-result-row> has display:contents and cannot be scrolled into view
+ // itself. Thus we are preferring to scroll the first child into view.
+ el = el?.shadowRoot?.firstElementChild ?? el;
+ el?.scrollIntoView({block: 'center'});
+ }, 0);
});
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index 963d28a..e7a7659 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -27,7 +27,7 @@
} from '../../services/checks/checks-model';
import './gr-checks-runs';
import './gr-checks-results';
-import {changeNum$} from '../../services/change/change-model';
+import {changeNum$, latestPatchNum$} from '../../services/change/change-model';
import {NumericChangeId, PatchSetNumber} from '../../types/common';
import {ActionTriggeredEvent} from '../../services/checks/checks-util';
import {AttemptSelectedEvent, RunSelectedEvent} from './gr-checks-util';
@@ -56,6 +56,9 @@
checksPatchsetNumber: PatchSetNumber | undefined = undefined;
@property()
+ latestPatchsetNumber: PatchSetNumber | undefined = undefined;
+
+ @property()
changeNum: NumericChangeId | undefined = undefined;
@state()
@@ -75,6 +78,7 @@
this.subscribe('runs', allRunsSelectedPatchset$);
this.subscribe('results', allResultsSelected$);
this.subscribe('checksPatchsetNumber', checksSelectedPatchsetNumber$);
+ this.subscribe('latestPatchsetNumber', latestPatchNum$);
this.subscribe('changeNum', changeNum$);
this.addEventListener('action-triggered', (e: ActionTriggeredEvent) =>
@@ -135,10 +139,11 @@
handleActionTriggered(action: Action, run?: CheckRun) {
if (!this.changeNum) return;
- if (!this.checksPatchsetNumber) return;
+ const patchSet = this.checksPatchsetNumber ?? this.latestPatchsetNumber;
+ if (!patchSet) return;
const promise = action.callback(
this.changeNum,
- this.checksPatchsetNumber,
+ patchSet,
run?.attempt,
run?.externalId,
run?.checkName,
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_html.ts b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_html.ts
index 0fa2f1e..97f4a89 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_html.ts
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_html.ts
@@ -45,7 +45,7 @@
account="[[account]]"
hidden$="[[!_hasAvatars]]"
hidden=""
- image-size="56"
+ imageSize="56"
aria-label="Account avatar"
></gr-avatar>
</gr-dropdown>
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index 351e1bb..8f9dfe2 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -231,7 +231,7 @@
);
}
}
- console.info(`server error: ${errorText}`);
+ this.reporting.error(new Error(`Server error: ${errorText}`));
});
};
@@ -319,7 +319,7 @@
private readonly handleNetworkError = (e: NetworkErrorEvent) => {
this._showAlert('Server unavailable');
- console.error(e.detail.error.message);
+ this.reporting.error(new Error(`network error: ${e.detail.error.message}`));
};
// TODO(dhruvsr): allow less priority alerts to override high priority alerts
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js
index 91f119e..fe4d9da 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js
@@ -207,7 +207,6 @@
});
test('show network error', done => {
- const consoleErrorStub = sinon.stub(console, 'error');
const showAlertStub = sinon.stub(element, '_showAlert');
element.dispatchEvent(
new CustomEvent('network-error', {
@@ -218,8 +217,6 @@
assert.isTrue(showAlertStub.calledOnce);
assert.isTrue(showAlertStub.lastCall.calledWithExactly(
'Server unavailable'));
- assert.isTrue(consoleErrorStub.calledOnce);
- assert.isTrue(consoleErrorStub.lastCall.calledWithExactly('ZOMG'));
done();
});
});
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.css.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.css.ts
deleted file mode 100644
index ccba289..0000000
--- a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.css.ts
+++ /dev/null
@@ -1,30 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {css} from 'lit-element';
-
-export const cssTemplate = css`
- .key {
- background-color: var(--chip-background-color);
- color: var(--primary-text-color);
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- display: inline-block;
- font-weight: var(--font-weight-bold);
- padding: var(--spacing-xxs) var(--spacing-m);
- text-align: center;
- }
-`;
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
index 091684f..657d7cc 100644
--- a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
+++ b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
@@ -16,9 +16,7 @@
*/
import {html} from 'lit-html';
import {GrLitElement} from '../../lit/gr-lit-element';
-import {customElement, property} from 'lit-element';
-import {cssTemplate} from './gr-key-binding-display.css';
-import {sharedStyles} from '../../../styles/shared-styles';
+import {css, customElement, property} from 'lit-element';
declare global {
interface HTMLElementTagNameMap {
@@ -29,7 +27,20 @@
@customElement('gr-key-binding-display')
export class GrKeyBindingDisplay extends GrLitElement {
static get styles() {
- return [sharedStyles, cssTemplate];
+ return [
+ css`
+ .key {
+ background-color: var(--chip-background-color);
+ color: var(--primary-text-color);
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ display: inline-block;
+ font-weight: var(--font-weight-bold);
+ padding: var(--spacing-xxs) var(--spacing-m);
+ text-align: center;
+ }
+ `,
+ ];
}
render() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index b42deda..72c7a62 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -398,7 +398,7 @@
if (e instanceof Response) {
this._handleGetDiffError(e);
} else {
- console.warn('Error encountered loading diff:', e);
+ this.reporting.error(e);
}
} finally {
this.reporting.timeEnd(Timing.DIFF_TOTAL);
@@ -483,12 +483,12 @@
});
})
.catch(err => {
- console.warn('Applying coverage from provider failed: ', err);
+ this.reporting.error(err);
});
});
})
.catch(err => {
- console.warn('Loading coverage ranges failed: ', err);
+ this.reporting.error(err);
});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index d626fe5..dacd8e7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -71,7 +71,6 @@
ConfigInfo,
EditInfo,
EditPatchSetNum,
- ElementPropertyDeepChange,
FileInfo,
NumericChangeId,
ParentPatchSetNum,
@@ -275,6 +274,11 @@
@property({type: Number})
_focusLineNum?: number;
+ private getReviewedParams: {
+ changeNum?: NumericChangeId;
+ patchNum?: PatchSetNum;
+ } = {};
+
get keyBindings() {
return {
esc: '_handleEscKey',
@@ -455,8 +459,13 @@
_setReviewed(reviewed: boolean) {
if (this._editMode) return;
this.$.reviewed.checked = reviewed;
- if (!this._patchRange?.patchNum) return;
+ if (!this._patchRange?.patchNum || !this._path) return;
+ const path = this._path;
+ if (reviewed) this._reviewedFiles.add(path);
+ else this._reviewedFiles.delete(path);
this._saveReviewedState(reviewed).catch(err => {
+ if (this._reviewedFiles.has(path)) this._reviewedFiles.delete(path);
+ else this._reviewedFiles.add(path);
fireAlert(this, ERR_REVIEW_STATUS);
throw err;
});
@@ -896,31 +905,26 @@
return {path: fileList[idx]};
}
- _getReviewedFiles(
- changeNum?: NumericChangeId,
- patchNum?: PatchSetNum
- ): Promise<Set<string>> {
- if (!changeNum || !patchNum) return Promise.resolve(new Set<string>());
- return this.restApiService
- .getReviewedFiles(changeNum, patchNum)
- .then(files => {
- this._reviewedFiles = new Set(files);
- return this._reviewedFiles;
- });
+ _getReviewedFiles(changeNum?: NumericChangeId, patchNum?: PatchSetNum) {
+ if (!changeNum || !patchNum) return;
+ if (
+ this.getReviewedParams.changeNum === changeNum &&
+ this.getReviewedParams.patchNum === patchNum
+ ) {
+ return;
+ }
+ this.getReviewedParams = {
+ changeNum,
+ patchNum,
+ };
+ this.restApiService.getReviewedFiles(changeNum, patchNum).then(files => {
+ this._reviewedFiles = new Set(files);
+ });
}
- _getReviewedStatus(
- editMode?: boolean,
- changeNum?: NumericChangeId,
- patchNum?: PatchSetNum,
- path?: string
- ) {
- if (editMode || !path) {
- return Promise.resolve(false);
- }
- return this._getReviewedFiles(changeNum, patchNum).then(files =>
- files.has(path)
- );
+ _getReviewedStatus(path: string) {
+ if (this._editMode) return false;
+ return this._reviewedFiles.has(path);
}
_initLineOfInterestAndCursor(leftSide: boolean) {
@@ -1095,7 +1099,9 @@
// the top-level change info view) and therefore undefined in `params`.
// If route is of type /comment/<commentId>/ then no patchNum is present
if (!value.patchNum && !value.commentLink) {
- console.warn('invalid url, no patchNum found');
+ this.reporting.error(
+ new Error(`Invalid diff view URL, no patchNum found: ${value}`)
+ );
return;
}
@@ -1199,47 +1205,39 @@
}
}
- @observe('_loggedIn', 'params.*', '_prefs', '_patchRange.*')
+ @observe('_path', '_prefs', '_reviewedFiles')
_setReviewedObserver(
+ path?: string,
+ prefs?: DiffPreferencesInfo,
+ reviewedFiles?: Set<string>
+ ) {
+ if (prefs === undefined) return;
+ if (path === undefined) return;
+ if (reviewedFiles === undefined) return;
+ if (prefs.manual_review) {
+ // Checkbox state needs to be set explicitly only when manual_review
+ // is specified.
+ this.$.reviewed.checked = this._getReviewedStatus(path);
+ } else {
+ this._setReviewed(true);
+ }
+ }
+
+ @observe('_loggedIn', '_changeNum', '_patchRange')
+ getReviewedFiles(
_loggedIn?: boolean,
- paramsRecord?: ElementPropertyDeepChange<GrDiffView, 'params'>,
- _prefs?: DiffPreferencesInfo,
- patchRangeRecord?: ElementPropertyDeepChange<GrDiffView, '_patchRange'>
+ _changeNum?: NumericChangeId,
+ patchRange?: PatchRange
) {
if (_loggedIn === undefined) return;
- if (paramsRecord === undefined) return;
- if (_prefs === undefined) return;
- if (patchRangeRecord === undefined) return;
- if (patchRangeRecord.base === undefined) return;
+ if (_changeNum === undefined) return;
+ if (patchRange === undefined) return;
- const patchRange = patchRangeRecord.base;
if (!_loggedIn) {
return;
}
- if (_prefs.manual_review) {
- // Checkbox state needs to be set explicitly only when manual_review
- // is specified.
-
- if (patchRange.patchNum) {
- this._getReviewedStatus(
- this._editMode,
- this._changeNum,
- patchRange.patchNum,
- this._path
- ).then((status: boolean) => {
- this.$.reviewed.checked = status;
- });
- }
- return;
- }
- // shift + m navigates to next unreviewed file so request list of reviewed
- // files even if manual review is not set
this._getReviewedFiles(this._changeNum, patchRange.patchNum);
-
- if (paramsRecord.base?.view === GerritNav.View.DIFF) {
- this._setReviewed(true);
- }
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 23cb936..ebcfa18 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -91,7 +91,6 @@
}
let getDiffChangeDetailStub;
- let getReviewedFilesStub;
setup(async () => {
clock = sinon.useFakeTimers();
stubRestApi('getConfig').returns(Promise.resolve({change: {}}));
@@ -105,8 +104,6 @@
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getPortedComments').returns(Promise.resolve({}));
- getReviewedFilesStub = stubRestApi('getReviewedFiles').returns(
- Promise.resolve([]));
element = basicFixture.instantiate();
element._changeNum = '42';
@@ -1157,10 +1154,11 @@
const saveReviewedStub = sinon.stub(element, '_saveReviewedState')
.callsFake(() => Promise.resolve());
const getReviewedStub = sinon.stub(element, '_getReviewedStatus')
- .callsFake(() => Promise.resolve());
+ .returns(false);
sinon.stub(element.$.diffHost, 'reload');
element._loggedIn = true;
+ element._prefs = {manual_review: true};
element.params = {
view: GerritNav.View.DIFF,
changeNum: '42',
@@ -1172,17 +1170,19 @@
patchNum: 2,
basePatchNum: 1,
};
- element._prefs = {manual_review: true};
flush();
assert.isFalse(saveReviewedStub.called);
assert.isTrue(getReviewedStub.called);
+ const oldCount = getReviewedStub.callCount;
+
element._prefs = {};
+ element._path = 'abcd';
flush();
assert.isTrue(saveReviewedStub.called);
- assert.isTrue(getReviewedStub.calledOnce);
+ assert.equal(getReviewedStub.callCount, oldCount);
});
test('file review status', () => {
@@ -1202,6 +1202,7 @@
patchNum: 2,
basePatchNum: 1,
};
+ element._path = 'abcd';
element._prefs = {};
flush();
@@ -1673,25 +1674,6 @@
[{value: '/foo'}, {value: '/bar'}]), 'show');
});
- test('_getReviewedStatus', () => {
- const promises = [];
- getReviewedFilesStub.returns(Promise.resolve(['path']));
-
- promises.push(element._getReviewedStatus(true, null, null, 'path')
- .then(reviewed => assert.isFalse(reviewed)));
-
- promises.push(element._getReviewedStatus(false, null, null, 'otherPath')
- .then(reviewed => assert.isFalse(reviewed)));
-
- promises.push(element._getReviewedStatus(false, null, null, 'path')
- .then(reviewed => assert.isFalse(reviewed)));
-
- promises.push(element._getReviewedStatus(false, 3, 5, 'path')
- .then(reviewed => assert.isTrue(reviewed)));
-
- return Promise.all(promises);
- });
-
test('f open file dropdown', () => {
assert.isFalse(element.$.dropdown.$.dropdown.opened);
MockInteractions.pressAndReleaseKeyOn(element, 70, null, 'f');
diff --git a/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts b/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts
index 0c6dd4d..0c36cd5 100644
--- a/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-event-helper/gr-event-helper.ts
@@ -57,7 +57,7 @@
try {
mayContinue = callback(e);
} catch (exception) {
- console.warn(`Plugin error handing event: ${exception}`);
+ this.reporting.error(exception);
}
if (mayContinue === false) {
e.stopImmediatePropagation();
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_html.ts b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_html.ts
index 6ca484d..e530ac8 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_html.ts
@@ -35,7 +35,7 @@
<section>
<span class="title"></span>
<span class="value">
- <gr-avatar account="[[_account]]" image-size="120"></gr-avatar>
+ <gr-avatar account="[[_account]]" imageSize="120"></gr-avatar>
</span>
</section>
<section class$="[[_hideAvatarChangeUrl(_avatarChangeUrl)]]">
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index ab4c5a5..d7078ed 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -97,6 +97,13 @@
_config?: ServerInfo;
@property({type: Boolean, reflectToAttribute: true})
+ selectionChipStyle = false;
+
+ @property({
+ type: Boolean,
+ reflectToAttribute: true,
+ observer: 'selectedChanged',
+ })
selected = false;
@property({type: Boolean, reflectToAttribute: true})
@@ -126,6 +133,10 @@
});
}
+ selectedChanged(selected?: boolean) {
+ this.deselected = !selected;
+ }
+
_isAttentionSetEnabled(
highlight: boolean,
account: AccountInfo,
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
index c5b66ce3..a642337 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
@@ -37,13 +37,13 @@
:host::after {
content: var(--account-label-suffix);
}
- :host([deselected]) {
+ :host([deselected][selection-chip-style]) {
background-color: var(--background-color-primary);
border: 1px solid var(--comment-separator-color);
border-radius: 8px;
color: var(--deemphasized-text-color);
}
- :host([selected]) {
+ :host([selected][selection-chip-style]) {
background-color: var(--chip-selected-background-color);
border: 1px solid var(--chip-selected-background-color);
border-radius: 8px;
@@ -128,7 +128,7 @@
class$="[[_computeHasAttentionClass(highlightAttention, account, change, forceAttention)]]"
>
<template is="dom-if" if="[[!hideAvatar]]">
- <gr-avatar account="[[account]]" image-size="32"></gr-avatar>
+ <gr-avatar account="[[account]]" imageSize="32"></gr-avatar>
</template>
<span class="text">
<span class="name">[[_computeName(account, _config, firstName)]]</span>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
index 231fc36..130969e 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
@@ -326,7 +326,9 @@
return;
}
}
- console.warn('received remove event for missing account', toRemove);
+ this.reporting.error(
+ new Error(`Received "remove" event for missing account: ${toRemove}`)
+ );
}
_getNativeInput(paperInput: PaperInputElementExt) {
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
index e30e995..80576a7 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
@@ -14,22 +14,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-avatar_html';
import {getBaseUrl} from '../../../utils/url-util';
import {getPluginLoader} from '../gr-js-api-interface/gr-plugin-loader';
-import {customElement, property} from '@polymer/decorators';
import {AccountInfo} from '../../../types/common';
import {appContext} from '../../../services/app-context';
+import {GrLitElement} from '../../lit/gr-lit-element';
+import {css, customElement, html, property} from 'lit-element';
@customElement('gr-avatar')
-export class GrAvatar extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
- @property({type: Object, observer: '_accountChanged'})
+export class GrAvatar extends GrLitElement {
+ @property({type: Object})
account?: AccountInfo;
@property({type: Number})
@@ -40,6 +34,27 @@
private readonly restApiService = appContext.restApiService;
+ static get styles() {
+ return [
+ css`
+ :host {
+ display: inline-block;
+ border-radius: 50%;
+ background-size: cover;
+ background-color: var(
+ --avatar-background-color,
+ var(--gray-background)
+ );
+ }
+ `,
+ ];
+ }
+
+ render() {
+ this._updateAvatarURL();
+ return html``;
+ }
+
/** @override */
connectedCallback() {
super.connectedCallback();
@@ -57,10 +72,6 @@
return this.restApiService.getConfig();
}
- _accountChanged() {
- this._updateAvatarURL();
- }
-
_updateAvatarURL() {
if (!this._hasAvatars || !this.account) {
this.hidden = true;
@@ -80,7 +91,7 @@
);
}
- _buildAvatarURL(account: AccountInfo) {
+ _buildAvatarURL(account?: AccountInfo) {
if (!account) {
return '';
}
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_html.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_html.ts
deleted file mode 100644
index a1e51df..0000000
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_html.ts
+++ /dev/null
@@ -1,28 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style>
- :host {
- display: inline-block;
- border-radius: 50%;
- background-size: cover;
- background-color: var(--avatar-background-color, var(--gray-background));
- }
- </style>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index 12173d0..65e8e9f 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -93,8 +93,12 @@
resolveWeblinks?: GeneratedWebLink[],
status?: ChangeStates
): boolean {
+ const isRevertCreatedOrSubmitted =
+ (status === ChangeStates.REVERT_SUBMITTED ||
+ status === ChangeStates.REVERT_CREATED) &&
+ revertedChange !== undefined;
return (
- revertedChange !== undefined ||
+ isRevertCreatedOrSubmitted ||
!!(status === ChangeStates.MERGE_CONFLICT && resolveWeblinks?.length)
);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_html.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_html.ts
index 116dcaf..2ca2744b 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_html.ts
@@ -87,7 +87,7 @@
>
<template
is="dom-if"
- if="[[!!hasStatusLink(revertedChange, resolveWeblinks, status)]]">
+ if="[[hasStatusLink(revertedChange, resolveWeblinks, status)]]">
<a class="status-link"
href="[[getStatusLink(revertedChange, resolveWeblinks, status)]]">
<div class="chip" aria-label$="Label: [[status]]">
@@ -100,7 +100,7 @@
</div>
</a>
</template>
- <template is="dom-if" if="[[!hasStatusLink(revertedChange)]]">
+ <template is="dom-if" if="[[!hasStatusLink(revertedChange, resolveWeblinks, status)]]">
<div class="chip" aria-label$="Label: [[status]]">
[[_computeStatusString(status)]]
</div>
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index b4b4c19..d6ad030 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -706,7 +706,9 @@
const index = this._indexOf(comment, this.comments);
if (index === -1) {
// This should never happen: comment belongs to another thread.
- console.warn('Comment update for another comment thread.');
+ this.reporting.error(
+ new Error(`Comment update for another comment thread: ${comment}`)
+ );
return;
}
this.set(['comments', index], comment);
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index 4ef25e2..83cd380 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -25,6 +25,7 @@
import {debounce, DelayedTask} from '../../../utils/async-util';
import {queryAndAssert} from '../../../utils/common-util';
import {IronAutogrowTextareaElement} from '@polymer/iron-autogrow-textarea/iron-autogrow-textarea';
+import {Interaction} from '../../../constants/reporting';
const RESTORED_MESSAGE = 'Content restored from a previous edit.';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -232,7 +233,7 @@
_toggleCommitCollapsed() {
this._commitCollapsed = !this._commitCollapsed;
- this.reporting.reportInteraction('toggle show all button', {
+ this.reporting.reportInteraction(Interaction.TOGGLE_SHOW_ALL_BUTTON, {
sectionName: 'Commit message',
toState: !this._commitCollapsed ? 'Show all' : 'Show less',
});
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 17b659f..7298af7 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -20,6 +20,7 @@
import {customElement, property} from '@polymer/decorators';
import {htmlTemplate} from './gr-formatted-text_html';
import {CommentLinks} from '../../../types/common';
+import {appContext} from '../../../services/app-context';
const CODE_MARKER_PATTERN = /^(`{1,3})([^`]+?)\1$/;
@@ -57,6 +58,8 @@
@property({type: Boolean})
noTrailingMargin = false;
+ private readonly reporting = appContext.reportingService;
+
static get observers() {
return ['_contentOrConfigChanged(content, config)'];
}
@@ -304,7 +307,7 @@
return ul;
}
- console.warn('Unrecognized type.');
+ this.reporting.error(new Error(`Unrecognized block type: ${block.type}`));
return document.createElement('span');
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
index d0986de..dac5962 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
@@ -75,7 +75,7 @@
<template is="dom-if" if="[[_isShowing]]">
<div class="top">
<div class="avatar">
- <gr-avatar account="[[account]]" image-size="56"></gr-avatar>
+ <gr-avatar account="[[account]]" imageSize="56"></gr-avatar>
</div>
<div class="account">
<h3 class="name heading-3">[[account.name]]</h3>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts
index 3f75970..82c7118 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.ts
@@ -42,7 +42,9 @@
): GrAnnotationActionsInterface {
this.reporting.trackApi(this.plugin, 'annotation', 'setCoverageProvider');
if (this.coverageProvider) {
- console.warn('Overwriting an existing coverage provider.');
+ this.reporting.error(
+ new Error(`Overwriting cov provider: ${this.plugin.getPluginName()}`)
+ );
}
this.coverageProvider = coverageProvider;
return this;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
index 15d4680..b9c4c32 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
@@ -80,7 +80,7 @@
*/
private setEl(el?: GrChangeActionsElement) {
if (!el) {
- console.warn('changeActions() is not ready');
+ this.reporting.error(new Error(`changeActions() API is not ready`));
return;
}
this.el = el;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts
index 2135c30..46c7ad6 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts
@@ -22,6 +22,7 @@
import {windowLocationReload} from '../../../utils/dom-util';
import {PopupPluginApi} from '../../../api/popup';
import {GrPopupInterface} from '../../plugins/gr-popup-interface/gr-popup-interface';
+import {appContext} from '../../../services/app-context';
interface ButtonCallBacks {
onclick: (event: Event) => boolean;
@@ -30,6 +31,8 @@
export class GrPluginActionContext {
private popups: PopupPluginApi[] = [];
+ private readonly reporting = appContext.reportingService;
+
constructor(
public readonly plugin: PluginApi,
public readonly action: UIActionInfo,
@@ -108,7 +111,9 @@
call(payload: RequestPayload, onSuccess: (result: unknown) => void) {
if (!this.action.method) return;
if (!this.action.__url) {
- console.warn(`Unable to ${this.action.method} to ${this.action.__key}!`);
+ this.reporting.error(
+ new Error(`Unable to ${this.action.method} to ${this.action.__key}!`)
+ );
return;
}
this.plugin
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
index fe101c5..6fc8f1b 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
@@ -132,7 +132,7 @@
try {
url = new URL(url);
} catch (e) {
- console.warn(e);
+ this._getReporting().error(e);
return false;
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
index 35acdef..cfa6c04 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
@@ -79,9 +79,10 @@
this.domHooks = new GrDomHooksManager(this);
if (!url) {
- console.warn(
- 'Plugin not being loaded from /plugins base path.',
- 'Unable to determine name.'
+ this.report.error(
+ new Error(
+ `Plugin not being loaded from /plugins base path. Unable to determine name.`
+ )
);
return this;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
index db22ce5..31a709a 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
@@ -81,6 +81,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly reporting = appContext.reportingService;
+
// TODO(TS): not used, remove later
_xhrPromise?: Promise<void>;
@@ -209,7 +211,7 @@
}
})
.catch(err => {
- console.warn(err);
+ this.reporting.error(err);
target.disabled = false;
return;
});
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
index 56ab8c6..b6583d9 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
@@ -88,7 +88,7 @@
<p
class$="placeholder [[_computeShowPlaceholder(labelInfo, change.labels.*)]]"
>
- No votes.
+ No votes
</p>
<table>
<template
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_html.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_html.ts
index 4bdc1ab..0d44bc8 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_html.ts
@@ -17,7 +17,7 @@
import {html} from '@polymer/polymer/lib/utils/html-tag';
export const htmlTemplate = html`
- <style include="shared-styles">
+ <style>
:host {
display: block;
}
@@ -30,6 +30,9 @@
text-decoration: none;
pointer-events: none;
}
+ a {
+ color: var(--link-color);
+ }
</style>
<span id="output"></span>
`;
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index 509a140..ac072f3 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -18,7 +18,12 @@
import {NumericChangeId} from '../../types/common';
import {EventDetails} from '../../api/reporting';
import {PluginApi} from '../../api/plugin';
-import {Execution, LifeCycle, Timing} from '../../constants/reporting';
+import {
+ Execution,
+ Interaction,
+ LifeCycle,
+ Timing,
+} from '../../constants/reporting';
export type EventValue = string | number | {error?: Error};
@@ -43,7 +48,7 @@
beforeLocationChanged(): void;
locationChanged(page: string): void;
dashboardDisplayed(): void;
- changeDisplayed(): void;
+ changeDisplayed(eventDetails?: EventDetails): void;
changeFullyLoaded(): void;
diffViewDisplayed(): void;
diffViewFullyLoaded(): void;
@@ -104,7 +109,10 @@
object: string,
method: string
): void;
- reportInteraction(eventName: string, details?: EventDetails): void;
+ reportInteraction(
+ eventName: string | Interaction,
+ details?: EventDetails
+ ): void;
/**
* A draft interaction was started. Update the time-between-draft-actions
* timer.
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
index 4e6ece2..0b93f07 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -21,7 +21,12 @@
import {NumericChangeId} from '../../types/common';
import {EventDetails} from '../../api/reporting';
import {PluginApi} from '../../api/plugin';
-import {Execution, LifeCycle, Timing} from '../../constants/reporting';
+import {
+ Execution,
+ Interaction,
+ LifeCycle,
+ Timing,
+} from '../../constants/reporting';
// Latency reporting constants.
@@ -508,11 +513,12 @@
}
}
- changeDisplayed() {
+ changeDisplayed(eventDetails?: EventDetails) {
+ eventDetails = {...eventDetails, ...this._pageLoadDetails()};
if (hasOwnProperty(this._baselines, Timing.STARTUP_CHANGE_DISPLAYED)) {
- this.timeEnd(Timing.STARTUP_CHANGE_DISPLAYED, this._pageLoadDetails());
+ this.timeEnd(Timing.STARTUP_CHANGE_DISPLAYED, eventDetails);
} else {
- this.timeEnd(Timing.CHANGE_DISPLAYED, this._pageLoadDetails());
+ this.timeEnd(Timing.CHANGE_DISPLAYED, eventDetails);
}
}
@@ -772,7 +778,7 @@
);
}
- reportInteraction(eventName: string, details: EventDetails) {
+ reportInteraction(eventName: string | Interaction, details: EventDetails) {
this.reporter(
INTERACTION.TYPE,
INTERACTION.CATEGORY.DEFAULT,
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
index c180439..24c27e0 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
@@ -17,7 +17,7 @@
import {ReportingService, Timer} from './gr-reporting';
import {EventDetails} from '../../api/reporting';
import {PluginApi} from '../../api/plugin';
-import {Execution} from '../../constants/reporting';
+import {Execution, Interaction} from '../../constants/reporting';
export class MockTimer implements Timer {
end(): this {
@@ -72,7 +72,10 @@
log(`trackApi '${plugin}', ${object}, ${method}`);
},
reportExtension: () => {},
- reportInteraction: (eventName: string, details?: EventDetails) => {
+ reportInteraction: (
+ eventName: string | Interaction,
+ details?: EventDetails
+ ) => {
log(`reportInteraction '${eventName}': ${JSON.stringify(details)}`);
},
reportLifeCycle: () => {},
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 414a80c..380d06c 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -174,13 +174,16 @@
return states;
}
-export function isOwner(change?: ChangeInfo, account?: AccountInfo): boolean {
+export function isOwner(
+ change?: ChangeInfo | ParsedChangeInfo,
+ account?: AccountInfo
+): boolean {
if (!change || !account) return false;
return change.owner?._account_id === account._account_id;
}
export function isReviewer(
- change?: ChangeInfo,
+ change?: ChangeInfo | ParsedChangeInfo,
account?: AccountInfo
): boolean {
if (!change || !account) return false;
diff --git a/polygerrit-ui/app/utils/patch-set-util_test.js b/polygerrit-ui/app/utils/patch-set-util_test.js
deleted file mode 100644
index 93073fa..0000000
--- a/polygerrit-ui/app/utils/patch-set-util_test.js
+++ /dev/null
@@ -1,211 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../test/common-test-setup-karma.js';
-import {
- _testOnly_computeWipForPatchSets, computeAllPatchSets,
- findEditParentPatchNum, findEditParentRevision,
- getParentIndex, getRevisionByPatchNum,
- isMergeParent,
- sortRevisions,
-} from './patch-set-util.js';
-
-suite('gr-patch-set-util tests', () => {
- test('getRevisionByPatchNum', () => {
- const revisions = [
- {_number: 0},
- {_number: 1},
- {_number: 2},
- ];
- assert.deepEqual(getRevisionByPatchNum(revisions, 1), revisions[1]);
- assert.deepEqual(getRevisionByPatchNum(revisions, 2), revisions[2]);
- assert.equal(getRevisionByPatchNum(revisions, 3), undefined);
- });
-
- test('_computeWipForPatchSets', () => {
- // Compute patch sets for a given timeline on a change. The initial WIP
- // property of the change can be true or false. The map of tags by
- // revision is keyed by patch set number. Each value is a list of change
- // message tags in the order that they occurred in the timeline. These
- // indicate actions that modify the WIP property of the change and/or
- // create new patch sets.
- //
- // Returns the actual results with an assertWip method that can be used
- // to compare against an expected value for a particular patch set.
- const compute = (initialWip, tagsByRevision) => {
- const change = {
- messages: [],
- work_in_progress: initialWip,
- };
- const revs = Object.keys(tagsByRevision).sort((a, b) => a - b);
- for (const rev of revs) {
- for (const tag of tagsByRevision[rev]) {
- change.messages.push({
- tag,
- _revision_number: rev,
- });
- }
- }
- let patchNums = revs.map(rev => { return {num: rev}; });
- patchNums = _testOnly_computeWipForPatchSets(
- change, patchNums);
- const actualWipsByRevision = {};
- for (const patchNum of patchNums) {
- actualWipsByRevision[patchNum.num] = patchNum.wip;
- }
- const verifier = {
- assertWip(revision, expectedWip) {
- const patchNum = patchNums.find(patchNum => patchNum.num == revision);
- if (!patchNum) {
- assert.fail('revision ' + revision + ' not found');
- }
- assert.equal(patchNum.wip, expectedWip,
- 'wip state for ' + revision + ' is ' +
- patchNum.wip + '; expected ' + expectedWip);
- return verifier;
- },
- };
- return verifier;
- };
-
- compute(false, {1: ['upload']}).assertWip(1, false);
- compute(true, {1: ['upload']}).assertWip(1, true);
-
- const setWip = 'autogenerated:gerrit:setWorkInProgress';
- const uploadInWip = 'autogenerated:gerrit:newWipPatchSet';
- const clearWip = 'autogenerated:gerrit:setReadyForReview';
-
- compute(false, {
- 1: ['upload', setWip],
- 2: ['upload'],
- 3: ['upload', clearWip],
- 4: ['upload', setWip],
- }).assertWip(1, false) // Change was created with PS1 ready for review
- .assertWip(2, true) // PS2 was uploaded during WIP
- .assertWip(3, false) // PS3 was marked ready for review after upload
- .assertWip(4, false); // PS4 was uploaded ready for review
-
- compute(false, {
- 1: [uploadInWip, null, 'addReviewer'],
- 2: ['upload'],
- 3: ['upload', clearWip, setWip],
- 4: ['upload'],
- 5: ['upload', clearWip],
- 6: [uploadInWip],
- }).assertWip(1, true) // Change was created in WIP
- .assertWip(2, true) // PS2 was uploaded during WIP
- .assertWip(3, false) // PS3 was marked ready for review
- .assertWip(4, true) // PS4 was uploaded during WIP
- .assertWip(5, false) // PS5 was marked ready for review
- .assertWip(6, true); // PS6 was uploaded with WIP option
- });
-
- test('isMergeParent', () => {
- assert.isFalse(isMergeParent(1));
- assert.isFalse(isMergeParent(4321));
- assert.isFalse(isMergeParent('52'));
- assert.isFalse(isMergeParent('edit'));
- assert.isFalse(isMergeParent('PARENT'));
- assert.isFalse(isMergeParent(0));
-
- assert.isTrue(isMergeParent(-23));
- assert.isTrue(isMergeParent(-1));
- assert.isTrue(isMergeParent('-42'));
- });
-
- test('findEditParentRevision', () => {
- let revisions = [
- {_number: 0},
- {_number: 1},
- {_number: 2},
- ];
- assert.strictEqual(findEditParentRevision(revisions), null);
-
- revisions = [...revisions, {_number: 'edit', basePatchNum: 3}];
- assert.strictEqual(findEditParentRevision(revisions), null);
-
- revisions = [...revisions, {_number: 3}];
- assert.deepEqual(findEditParentRevision(revisions), {_number: 3});
- });
-
- test('findEditParentPatchNum', () => {
- let revisions = [
- {_number: 0},
- {_number: 1},
- {_number: 2},
- ];
- assert.equal(findEditParentPatchNum(revisions), -1);
-
- revisions =
- [...revisions, {_number: 'edit', basePatchNum: 3}, {_number: 3}];
- assert.deepEqual(findEditParentPatchNum(revisions), 3);
- });
-
- test('sortRevisions', () => {
- const revisions = [
- {_number: 0},
- {_number: 2},
- {_number: 1},
- ];
- const sorted = [
- {_number: 2},
- {_number: 1},
- {_number: 0},
- ];
-
- assert.deepEqual(sortRevisions(revisions), sorted);
-
- // Edit patchset should follow directly after its basePatchNum.
- revisions.push({_number: 'edit', basePatchNum: 2});
- sorted.unshift({_number: 'edit', basePatchNum: 2});
- assert.deepEqual(sortRevisions(revisions), sorted);
-
- revisions[0].basePatchNum = 0;
- const edit = sorted.shift();
- edit.basePatchNum = 0;
- // Edit patchset should be at index 2.
- sorted.splice(2, 0, edit);
- assert.deepEqual(sortRevisions(revisions), sorted);
- });
-
- test('getParentIndex', () => {
- assert.equal(getParentIndex('-13'), 13);
- assert.equal(getParentIndex(-4), 4);
- });
-
- test('computeAllPatchSets', () => {
- const expected = [
- {num: 4, desc: 'test', sha: 'rev4'},
- {num: 3, desc: 'test', sha: 'rev3'},
- {num: 2, desc: 'test', sha: 'rev2'},
- {num: 1, desc: 'test', sha: 'rev1'},
- ];
- const patchNums = computeAllPatchSets({
- revisions: {
- rev3: {_number: 3, description: 'test', date: 3},
- rev1: {_number: 1, description: 'test', date: 1},
- rev4: {_number: 4, description: 'test', date: 4},
- rev2: {_number: 2, description: 'test', date: 2},
- },
- });
- assert.equal(patchNums.length, expected.length);
- for (let i = 0; i < expected.length; i++) {
- assert.deepEqual(patchNums[i], expected[i]);
- }
- });
-});
-
diff --git a/polygerrit-ui/app/utils/patch-set-util_test.ts b/polygerrit-ui/app/utils/patch-set-util_test.ts
new file mode 100644
index 0000000..a9d9549
--- /dev/null
+++ b/polygerrit-ui/app/utils/patch-set-util_test.ts
@@ -0,0 +1,246 @@
+/**
+ * @license
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../test/common-test-setup-karma';
+import {
+ createChange,
+ createChangeMessageInfo,
+ createRevision,
+} from '../test/test-data-generators';
+import {
+ BasePatchSetNum,
+ ChangeInfo,
+ EditPatchSetNum,
+ PatchSetNum,
+ ReviewInputTag,
+} from '../types/common';
+import {
+ _testOnly_computeWipForPatchSets,
+ computeAllPatchSets,
+ findEditParentPatchNum,
+ findEditParentRevision,
+ getParentIndex,
+ getRevisionByPatchNum,
+ isMergeParent,
+ sortRevisions,
+} from './patch-set-util';
+
+suite('gr-patch-set-util tests', () => {
+ test('getRevisionByPatchNum', () => {
+ const revisions = [createRevision(0), createRevision(1), createRevision(2)];
+ assert.deepEqual(
+ getRevisionByPatchNum(revisions, 1 as PatchSetNum),
+ revisions[1]
+ );
+ assert.deepEqual(
+ getRevisionByPatchNum(revisions, 2 as PatchSetNum),
+ revisions[2]
+ );
+ assert.equal(getRevisionByPatchNum(revisions, 3 as PatchSetNum), undefined);
+ });
+
+ test('_computeWipForPatchSets', () => {
+ // Compute patch sets for a given timeline on a change. The initial WIP
+ // property of the change can be true or false. The map of tags by
+ // revision is keyed by patch set number. Each value is a list of change
+ // message tags in the order that they occurred in the timeline. These
+ // indicate actions that modify the WIP property of the change and/or
+ // create new patch sets.
+ //
+ // Returns the actual results with an assertWip method that can be used
+ // to compare against an expected value for a particular patch set.
+ const compute = (
+ initialWip: boolean,
+ tagsByRevision: Map<
+ number | 'edit' | 'PARENT',
+ (ReviewInputTag | undefined)[]
+ >
+ ) => {
+ const change: ChangeInfo = {
+ ...createChange(),
+ messages: [],
+ work_in_progress: initialWip,
+ };
+ for (const rev of tagsByRevision.keys()) {
+ for (const tag of tagsByRevision.get(rev)!) {
+ change.messages!.push({
+ ...createChangeMessageInfo(),
+ tag,
+ _revision_number: rev as PatchSetNum,
+ });
+ }
+ }
+ const patchSets = Array.from(tagsByRevision.keys()).map(rev => {
+ return {num: rev as PatchSetNum, desc: 'test', sha: `rev${rev}`};
+ });
+ const patchNums = _testOnly_computeWipForPatchSets(change, patchSets);
+ const verifier = {
+ assertWip(revision: number, expectedWip: boolean) {
+ const patchNum = patchNums.find(
+ patchNum => patchNum.num === (revision as PatchSetNum)
+ );
+ if (!patchNum) {
+ assert.fail(`revision ${revision} not found`);
+ }
+ assert.equal(
+ patchNum.wip,
+ expectedWip,
+ `wip state for ${revision} ` +
+ `is ${patchNum.wip}; expected ${expectedWip}`
+ );
+ return verifier;
+ },
+ };
+ return verifier;
+ };
+
+ const upload = 'upload' as ReviewInputTag;
+
+ compute(false, new Map([[1, [upload]]])).assertWip(1, false);
+ compute(true, new Map([[1, [upload]]])).assertWip(1, true);
+
+ const setWip = 'autogenerated:gerrit:setWorkInProgress' as ReviewInputTag;
+ const uploadInWip = 'autogenerated:gerrit:newWipPatchSet' as ReviewInputTag;
+ const clearWip = 'autogenerated:gerrit:setReadyForReview' as ReviewInputTag;
+
+ compute(
+ false,
+ new Map([
+ [1, [upload, setWip]],
+ [2, [upload]],
+ [3, [upload, clearWip]],
+ [4, [upload, setWip]],
+ ])
+ )
+ .assertWip(1, false) // Change was created with PS1 ready for review
+ .assertWip(2, true) // PS2 was uploaded during WIP
+ .assertWip(3, false) // PS3 was marked ready for review after upload
+ .assertWip(4, false); // PS4 was uploaded ready for review
+
+ compute(
+ false,
+ new Map([
+ [1, [uploadInWip, undefined, 'addReviewer' as ReviewInputTag]],
+ [2, [upload]],
+ [3, [upload, clearWip, setWip]],
+ [4, [upload]],
+ [5, [upload, clearWip]],
+ [6, [uploadInWip]],
+ ])
+ )
+ .assertWip(1, true) // Change was created in WIP
+ .assertWip(2, true) // PS2 was uploaded during WIP
+ .assertWip(3, false) // PS3 was marked ready for review
+ .assertWip(4, true) // PS4 was uploaded during WIP
+ .assertWip(5, false) // PS5 was marked ready for review
+ .assertWip(6, true); // PS6 was uploaded with WIP option
+ });
+
+ test('isMergeParent', () => {
+ assert.isFalse(isMergeParent(1 as PatchSetNum));
+ assert.isFalse(isMergeParent(4321 as PatchSetNum));
+ assert.isFalse(isMergeParent('edit' as PatchSetNum));
+ assert.isFalse(isMergeParent('PARENT' as PatchSetNum));
+ assert.isFalse(isMergeParent(0 as PatchSetNum));
+
+ assert.isTrue(isMergeParent(-23 as PatchSetNum));
+ assert.isTrue(isMergeParent(-1 as PatchSetNum));
+ });
+
+ test('findEditParentRevision', () => {
+ const revisions = [createRevision(0), createRevision(1), createRevision(2)];
+ assert.strictEqual(findEditParentRevision(revisions), null);
+
+ revisions.push({
+ ...createRevision(),
+ _number: EditPatchSetNum,
+ basePatchNum: 3 as BasePatchSetNum,
+ });
+ assert.strictEqual(findEditParentRevision(revisions), null);
+
+ revisions.push(createRevision(3));
+ assert.deepEqual(findEditParentRevision(revisions), createRevision(3));
+ });
+
+ test('findEditParentPatchNum', () => {
+ const revisions = [createRevision(0), createRevision(1), createRevision(2)];
+ assert.equal(findEditParentPatchNum(revisions), -1);
+
+ revisions.push(
+ {
+ ...createRevision(),
+ _number: EditPatchSetNum,
+ basePatchNum: 3 as BasePatchSetNum,
+ },
+ createRevision(3)
+ );
+ assert.deepEqual(findEditParentPatchNum(revisions), 3);
+ });
+
+ test('sortRevisions', () => {
+ const revisions = [createRevision(0), createRevision(2), createRevision(1)];
+ const sorted = [createRevision(2), createRevision(1), createRevision(0)];
+
+ assert.deepEqual(sortRevisions(revisions), sorted);
+
+ // Edit patchset should follow directly after its basePatchNum.
+ revisions.push({
+ ...createRevision(),
+ _number: EditPatchSetNum,
+ basePatchNum: 2 as BasePatchSetNum,
+ });
+ sorted.unshift({
+ ...createRevision(),
+ _number: EditPatchSetNum,
+ basePatchNum: 2 as BasePatchSetNum,
+ });
+ assert.deepEqual(sortRevisions(revisions), sorted);
+
+ revisions[0].basePatchNum = 0 as BasePatchSetNum;
+ const edit = sorted.shift()!;
+ edit.basePatchNum = 0 as BasePatchSetNum;
+ // Edit patchset should be at index 2.
+ sorted.splice(2, 0, edit);
+ assert.deepEqual(sortRevisions(revisions), sorted);
+ });
+
+ test('getParentIndex', () => {
+ assert.equal(getParentIndex(-4 as PatchSetNum), 4);
+ });
+
+ test('computeAllPatchSets', () => {
+ const expected = [
+ {num: 4 as PatchSetNum, desc: 'test', sha: 'rev4'},
+ {num: 3 as PatchSetNum, desc: 'test', sha: 'rev3'},
+ {num: 2 as PatchSetNum, desc: 'test', sha: 'rev2'},
+ {num: 1 as PatchSetNum, desc: 'test', sha: 'rev1'},
+ ];
+ const patchNums = computeAllPatchSets({
+ ...createChange(),
+ revisions: {
+ rev1: {...createRevision(1), description: 'test'},
+ rev2: {...createRevision(2), description: 'test'},
+ rev3: {...createRevision(3), description: 'test'},
+ rev4: {...createRevision(4), description: 'test'},
+ },
+ });
+ assert.equal(patchNums.length, expected.length);
+ for (let i = 0; i < expected.length; i++) {
+ assert.deepEqual(patchNums[i], expected[i]);
+ }
+ });
+});
diff --git a/proto/cache.proto b/proto/cache.proto
index 4efce6f..9d2ba06 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -475,7 +475,7 @@
string name = 1;
string description = 2;
string applicability_expression = 3;
- string blocking_expression = 4;
+ string submittability_expression = 4;
string override_expression = 5;
bool allow_override_in_child_projects = 6;
}
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 3d04592..434196f 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -6,7 +6,7 @@
GUAVA_DOC_URL = "https://google.github.io/guava/releases/" + GUAVA_VERSION + "/api/docs/"
-TESTCONTAINERS_VERSION = "1.15.1"
+TESTCONTAINERS_VERSION = "1.15.3"
def declare_nongoogle_deps():
"""loads dependencies that are not used at Google.
@@ -187,21 +187,21 @@
sha1 = "dc13ae4faca6df981fc7aeb5a522d9db446d5d50",
)
- DOCKER_JAVA_VERS = "3.2.7"
+ DOCKER_JAVA_VERS = "3.2.8"
maven_jar(
name = "docker-java-api",
artifact = "com.github.docker-java:docker-java-api:" + DOCKER_JAVA_VERS,
- sha1 = "81408fc988c229ea11354fee9902c47842343f04",
+ sha1 = "4ac22a72d546a9f3523cd4b5fabffa77c4a6ec7c",
)
maven_jar(
name = "docker-java-transport",
artifact = "com.github.docker-java:docker-java-transport:" + DOCKER_JAVA_VERS,
- sha1 = "315903a129f530422747efc163dd255f0fa2555e",
+ sha1 = "c3b5598c67d0a5e2e780bf48f520da26b9915eab",
)
- # https://github.com/docker-java/docker-java/blob/3.2.7/pom.xml#L61
+ # https://github.com/docker-java/docker-java/blob/3.2.8/pom.xml#L61
# <=> DOCKER_JAVA_VERS
maven_jar(
name = "jackson-annotations",
@@ -212,7 +212,7 @@
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "91e6dfab8f141f77c6a0dd147a94bd186993a22c",
+ sha1 = "95c6cfde71c2209f0c29cb14e432471e0b111880",
)
maven_jar(
