Merge "Remove unused property disableDiffPrefs"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 84e68ed..acf65a5 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4343,7 +4343,7 @@
before it is focefully cancelled.
+
The receive timeout cannot be overriden by setting a higher
-link:user-upload#deadline[deadline] on the git push request.
+link:user-upload.html#deadline[deadline] on the git push request.
+
Default is 4 minutes. If no unit is specified, milliseconds
is assumed.
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 8064482..18a693f 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -202,7 +202,6 @@
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.SystemReader;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Rule;
@@ -365,7 +364,7 @@
}
}
- @AfterClass
+ @ConfigSuite.AfterConfig
public static void stopCommonServer() throws Exception {
if (commonServer != null) {
try {
diff --git a/java/com/google/gerrit/acceptance/TestMetricMaker.java b/java/com/google/gerrit/acceptance/TestMetricMaker.java
index d60ef1a..2620f99 100644
--- a/java/com/google/gerrit/acceptance/TestMetricMaker.java
+++ b/java/com/google/gerrit/acceptance/TestMetricMaker.java
@@ -34,11 +34,11 @@
*
* <pre>
* public class MyTest extends AbstractDaemonTest {
- * @Inject private TestMetricMaker testMetricMaker;
+ * {@literal @}Inject private TestMetricMaker testMetricMaker;
*
* ...
*
- * @Test
+ * {@literal @}Test
* public void testFoo() throws Exception {
* testMetricMaker.reset();
* doSomething();
diff --git a/java/com/google/gerrit/lucene/WrappableSearcherManager.java b/java/com/google/gerrit/lucene/WrappableSearcherManager.java
index 4044b90..c164b29 100644
--- a/java/com/google/gerrit/lucene/WrappableSearcherManager.java
+++ b/java/com/google/gerrit/lucene/WrappableSearcherManager.java
@@ -53,7 +53,6 @@
* {@link #maybeRefresh}. Finally, be sure to call {@link #close} once you are done.
*
* @see SearcherFactory
- * @lucene.experimental
*/
// This file was copied from:
// https://github.com/apache/lucene-solr/blob/lucene_solr_5_0/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index f093958..209901d 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -104,6 +104,7 @@
private boolean allowClosed;
private boolean sendEmail = true;
private String topic;
+ private boolean storeCopiedVotes = true;
// Fields set during some phase of BatchUpdate.Op.
private Change change;
@@ -203,6 +204,17 @@
return this;
}
+ /**
+ * We always want to store copied votes except when the change is getting submitted and a new
+ * patch-set is created on submit (using submit strategies such as "REBASE_ALWAYS"). In such
+ * cases, we already store the votes of the new patch-sets in SubmitStrategyOp#saveApprovals. We
+ * should not also store the copied votes.
+ */
+ public PatchSetInserter setStoreCopiedVotes(boolean storeCopiedVotes) {
+ this.storeCopiedVotes = storeCopiedVotes;
+ return this;
+ }
+
public Change getChange() {
checkState(change != null, "getChange() only valid after executing update");
return change;
@@ -288,7 +300,9 @@
// Approvals that are being set in the new patch-set during this operation are not available yet
// outside of the scope of this method. Only copied approvals are set here.
- approvalsUtil.byPatchSet(ctx.getNotes(), patchSet).forEach(a -> update.putCopiedApproval(a));
+ if (storeCopiedVotes) {
+ approvalsUtil.byPatchSet(ctx.getNotes(), patchSet).forEach(a -> update.putCopiedApproval(a));
+ }
return true;
}
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 4952464..3e67cca 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -92,6 +92,7 @@
private boolean detailedCommitMessage;
private boolean postMessage = true;
private boolean sendEmail = true;
+ private boolean storeCopiedVotes = true;
private boolean matchAuthorToCommitterDate = false;
private CodeReviewCommit rebasedCommit;
@@ -169,6 +170,17 @@
return this;
}
+ /**
+ * We always want to store copied votes except when the change is getting submitted and a new
+ * patch-set is created on submit (using submit strategies such as "REBASE_ALWAYS"). In such
+ * cases, we already store the votes of the new patch-sets in SubmitStrategyOp#saveApprovals. We
+ * should not also store the copied votes.
+ */
+ public RebaseChangeOp setStoreCopiedVotes(boolean storeCopiedVotes) {
+ this.storeCopiedVotes = storeCopiedVotes;
+ return this;
+ }
+
public RebaseChangeOp setSendEmail(boolean sendEmail) {
this.sendEmail = sendEmail;
return this;
@@ -219,7 +231,10 @@
.setFireRevisionCreated(fireRevisionCreated)
.setCheckAddPatchSetPermission(checkAddPatchSetPermission)
.setValidate(validate)
- .setSendEmail(sendEmail);
+ .setSendEmail(sendEmail)
+ // The votes are automatically copied and they don't count as copied votes. See
+ // method's javadoc.
+ .setStoreCopiedVotes(storeCopiedVotes);
if (!rebasedCommit.getFilesWithGitConflicts().isEmpty()
&& !notes.getChange().isWorkInProgress()) {
diff --git a/java/com/google/gerrit/server/notedb/CommitRewriter.java b/java/com/google/gerrit/server/notedb/CommitRewriter.java
index e940b1e..eabee65 100644
--- a/java/com/google/gerrit/server/notedb/CommitRewriter.java
+++ b/java/com/google/gerrit/server/notedb/CommitRewriter.java
@@ -30,6 +30,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
@@ -117,6 +118,16 @@
public boolean verifyCommits = true;
/** Whether to compute and output the diff of the commit history for the backfilled refs. */
public boolean outputDiff = true;
+
+ /** Max number of refs to update in a single {@link BatchRefUpdate}. */
+ public int maxRefsInBatch = 10000;
+ /**
+ * Max number of refs to fix by a single {@link RefsUpdate#backfillProject} run. Since second
+ * run on the same set of refs is a no-op, running with this option in a loop will eventually
+ * fix all refs. Number of executed {@link BatchRefUpdate} depends on {@link #maxRefsInBatch}
+ * option.
+ */
+ public int maxRefsToUpdate = 50000;
}
/** Result of the backfill run for a project. */
@@ -239,15 +250,24 @@
*/
public BackfillResult backfillProject(
Project.NameKey project, Repository repo, RunOptions options) {
+
+ checkState(
+ options.maxRefsInBatch > 0 && options.maxRefsToUpdate > 0,
+ "Expected maxRefsInBatch>0 && <= maxRefsToUpdate>0");
+ checkState(
+ options.maxRefsInBatch <= options.maxRefsToUpdate,
+ "Expected maxRefsInBatch(%s) <= maxRefsToUpdate(%s)",
+ options.maxRefsInBatch,
+ options.maxRefsToUpdate);
BackfillResult result = new BackfillResult();
result.ok = true;
- try (RevWalk revWalk = new RevWalk(repo);
- ObjectInserter ins = newPackInserter(repo)) {
- BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
- bru.setForceRefLog(true);
- bru.setRefLogMessage(CommitRewriter.class.getName(), false);
- bru.setAllowNonFastForwards(true);
+ int refsInUpdate = 0;
+ RefsUpdate refsUpdate = null;
+ try {
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
+ if (result.fixedRefDiff.size() >= options.maxRefsToUpdate) {
+ return result;
+ }
Change.Id changeId = Change.Id.fromRef(ref.getName());
if (changeId == null || !ref.getName().equals(RefNames.changeMetaRef(changeId))) {
continue;
@@ -262,14 +282,26 @@
logger.atWarning().withCause(e).log("Failed to run verification on ref %s", ref);
}
}
+ if (refsUpdate == null) {
+ refsUpdate = RefsUpdate.create(repo);
+ }
ChangeFixProgress changeFixProgress =
- backfillChange(revWalk, ins, ref, accountsInChange, options);
+ backfillChange(refsUpdate, ref, accountsInChange, options);
if (changeFixProgress.anyFixesApplied) {
- bru.addCommand(
- new ReceiveCommand(ref.getObjectId(), changeFixProgress.newTipId, ref.getName()));
+ refsInUpdate++;
+ refsUpdate
+ .batchRefUpdate()
+ .addCommand(
+ new ReceiveCommand(
+ ref.getObjectId(), changeFixProgress.newTipId, ref.getName()));
result.fixedRefDiff.put(ref.getName(), changeFixProgress.commitDiffs);
}
-
+ if (refsInUpdate >= options.maxRefsInBatch
+ || result.fixedRefDiff.size() >= options.maxRefsToUpdate) {
+ processUpdate(options, refsUpdate);
+ refsUpdate = null;
+ refsInUpdate = 0;
+ }
if (!changeFixProgress.isValidAfterFix) {
result.refsStillInvalidAfterFix.add(ref.getName());
}
@@ -278,21 +310,34 @@
result.refsFailedToFix.add(ref.getName());
}
}
-
- if (!bru.getCommands().isEmpty()) {
- if (!options.dryRun) {
- ins.flush();
- RefUpdateUtil.executeChecked(bru, revWalk);
- }
- }
+ processUpdate(options, refsUpdate);
} catch (IOException e) {
- logger.atWarning().withCause(e).log("Failed to fix project %s", project.get());
+ logger.atWarning().log("Failed to fix project %s. Reason: %s", project.get(), e.getMessage());
result.ok = false;
+ } finally {
+ if (refsUpdate != null) {
+ refsUpdate.close();
+ }
}
return result;
}
+ /** Executes a single {@link RefsUpdate#batchRefUpdate}. */
+ private void processUpdate(RunOptions options, @Nullable RefsUpdate refsUpdate)
+ throws IOException {
+ if (refsUpdate == null) {
+ return;
+ }
+ if (!refsUpdate.batchRefUpdate().getCommands().isEmpty()) {
+ if (!options.dryRun) {
+ refsUpdate.inserter().flush();
+ RefUpdateUtil.executeChecked(refsUpdate.batchRefUpdate(), refsUpdate.revWalk());
+ }
+ }
+ refsUpdate.close();
+ }
+
/**
* Retrieves accounts, that are associated with a change (e.g. reviewers, commenters, etc.). These
* accounts are used to verify that commits do not contain user data. See {@link #verifyCommit}
@@ -376,8 +421,7 @@
* ChangeFixProgress#newTipId}.
*/
public ChangeFixProgress backfillChange(
- RevWalk revWalk,
- ObjectInserter inserter,
+ RefsUpdate refsUpdate,
Ref ref,
ImmutableSet<AccountState> accountsInChange,
RunOptions options)
@@ -385,17 +429,17 @@
ObjectId oldTip = ref.getObjectId();
// Walk from the first commit of the branch.
- revWalk.reset();
- revWalk.markStart(revWalk.parseCommit(oldTip));
- revWalk.sort(RevSort.TOPO);
+ refsUpdate.revWalk().reset();
+ refsUpdate.revWalk().markStart(refsUpdate.revWalk().parseCommit(oldTip));
+ refsUpdate.revWalk().sort(RevSort.TOPO);
- revWalk.sort(RevSort.REVERSE);
+ refsUpdate.revWalk().sort(RevSort.REVERSE);
RevCommit originalCommit;
boolean rewriteStarted = false;
ChangeFixProgress changeFixProgress = new ChangeFixProgress(ref.getName());
- while ((originalCommit = revWalk.next()) != null) {
+ while ((originalCommit = refsUpdate.revWalk().next()) != null) {
changeFixProgress.updateAuthorId =
parseIdent(changeFixProgress, originalCommit.getAuthorIdent());
@@ -453,7 +497,8 @@
cb.setEncoding(originalCommit.getEncoding());
byte[] newCommitContent = cb.build();
checkCommitModification(originalCommit, newCommitContent);
- changeFixProgress.newTipId = inserter.insert(Constants.OBJ_COMMIT, newCommitContent);
+ changeFixProgress.newTipId =
+ refsUpdate.inserter().insert(Constants.OBJ_COMMIT, newCommitContent);
// Only compute diff if the content of the commit was actually changed.
if (options.outputDiff && needsFix) {
String diff = computeDiff(originalCommit.getRawBuffer(), newCommitContent);
@@ -1283,4 +1328,33 @@
abstract Optional<String> email();
}
+
+ /**
+ * Objects, needed to fix Refs in a single {@link BatchRefUpdate}. Number of changes in a batch
+ * are limited by {@link RunOptions#maxRefsInBatch}.
+ */
+ @AutoValue
+ abstract static class RefsUpdate implements AutoCloseable {
+ static RefsUpdate create(Repository repo) {
+ RevWalk revWalk = new RevWalk(repo);
+ ObjectInserter inserter = newPackInserter(repo);
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setForceRefLog(true);
+ bru.setRefLogMessage(CommitRewriter.class.getName(), false);
+ bru.setAllowNonFastForwards(true);
+ return new AutoValue_CommitRewriter_RefsUpdate(bru, revWalk, inserter);
+ }
+
+ @Override
+ public void close() {
+ inserter().close();
+ revWalk().close();
+ }
+
+ abstract BatchRefUpdate batchRefUpdate();
+
+ abstract RevWalk revWalk();
+
+ abstract ObjectInserter inserter();
+ }
}
diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
index cc3b75d..355d25f 100644
--- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
@@ -211,7 +211,10 @@
.setPostMessage(false)
.setSendEmail(false)
.setMatchAuthorToCommitterDate(
- args.project.is(BooleanProjectConfig.MATCH_AUTHOR_TO_COMMITTER_DATE));
+ args.project.is(BooleanProjectConfig.MATCH_AUTHOR_TO_COMMITTER_DATE))
+ // The votes are automatically copied and they don't count as copied votes. See
+ // method's javadoc.
+ .setStoreCopiedVotes(/* storeCopiedVotes = */ false);
try {
rebaseOp.updateRepo(ctx);
} catch (MergeConflictException | NoSuchChangeException e) {
diff --git a/java/com/google/gerrit/server/util/CommitMessageUtil.java b/java/com/google/gerrit/server/util/CommitMessageUtil.java
index 55e3951..dc9c2d9 100644
--- a/java/com/google/gerrit/server/util/CommitMessageUtil.java
+++ b/java/com/google/gerrit/server/util/CommitMessageUtil.java
@@ -51,7 +51,7 @@
* the commit message.
*
* @throws BadRequestException if the commit message is null or empty
- * @returns the trimmed message with a trailing newline character
+ * @return the trimmed message with a trailing newline character
*/
public static String checkAndSanitizeCommitMessage(@Nullable String commitMessage)
throws BadRequestException {
diff --git a/java/com/google/gerrit/testing/ConfigSuite.java b/java/com/google/gerrit/testing/ConfigSuite.java
index d7ae397..3101c48 100644
--- a/java/com/google/gerrit/testing/ConfigSuite.java
+++ b/java/com/google/gerrit/testing/ConfigSuite.java
@@ -25,7 +25,9 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import java.lang.annotation.Annotation;
+import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
@@ -35,11 +37,15 @@
import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;
+import org.junit.internal.runners.statements.RunAfters;
+import org.junit.internal.runners.statements.RunBefores;
+import org.junit.rules.TestRule;
import org.junit.runner.Runner;
import org.junit.runners.BlockJUnit4ClassRunner;
import org.junit.runners.Suite;
import org.junit.runners.model.FrameworkMethod;
import org.junit.runners.model.InitializationError;
+import org.junit.runners.model.Statement;
/**
* Suite to run tests with different {@code gerrit.config} values.
@@ -47,6 +53,9 @@
* <p>For each {@link Config} method in the class and base classes, a new group of tests is created
* with the {@link Parameter} field set to the config.
*
+ * <p>Additional actions can be executed before or after each group of tests using
+ * {@literal @}BeforeConfig, {@literal @}AfterConfig or {@literal @}ConfigRule annotations.
+ *
* <pre>
* {@literal @}RunWith(ConfigSuite.class)
* public abstract class MyAbstractTest {
@@ -128,6 +137,38 @@
@Retention(RUNTIME)
public static @interface Name {}
+ /**
+ * Annotation for methods which should be run after executing group of tests with a new
+ * configuration.
+ *
+ * <p>Works similar to {@link org.junit.AfterClass}, but a method can be executed multiple times
+ * if a test class provides multiple configs.
+ */
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.METHOD)
+ public @interface AfterConfig {}
+
+ /**
+ * Annotation for methods which should be run before executing group of tests with a new
+ * configuration.
+ *
+ * <p>Works similar to {@link org.junit.BeforeClass}, but a method can be executed multiple times
+ * if a test class provides multiple configs.
+ */
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.METHOD)
+ public @interface BeforeConfig {}
+
+ /**
+ * Annotation for fields or methods which wraps all tests with the same config
+ *
+ * <p>Works similar to {@link org.junit.ClassRule}, but Statement evaluates multiple time - ones
+ * for each config provided by a test class.
+ */
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target({ElementType.FIELD, ElementType.METHOD})
+ public @interface ConfigRule {}
+
private static class ConfigRunner extends BlockJUnit4ClassRunner {
private final org.eclipse.jgit.lib.Config cfg;
private final Field parameterField;
@@ -168,6 +209,26 @@
String n = method.getName();
return name == null ? n : n + "[" + name + "]";
}
+
+ @Override
+ protected Statement withBeforeClasses(Statement statement) {
+ List<FrameworkMethod> befores = getTestClass().getAnnotatedMethods(BeforeConfig.class);
+ return befores.isEmpty() ? statement : new RunBefores(statement, befores, null);
+ }
+
+ @Override
+ protected Statement withAfterClasses(Statement statement) {
+ List<FrameworkMethod> afters = getTestClass().getAnnotatedMethods(AfterConfig.class);
+ return afters.isEmpty() ? statement : new RunAfters(statement, afters, null);
+ }
+
+ @Override
+ protected List<TestRule> classRules() {
+ List<TestRule> result =
+ getTestClass().getAnnotatedMethodValues(null, ConfigRule.class, TestRule.class);
+ result.addAll(getTestClass().getAnnotatedFieldValues(null, ConfigRule.class, TestRule.class));
+ return result;
+ }
}
private static List<Runner> runnersFor(Class<?> clazz) {
diff --git a/java/com/google/gerrit/testing/SystemPropertiesTestRule.java b/java/com/google/gerrit/testing/SystemPropertiesTestRule.java
new file mode 100644
index 0000000..c7a3542
--- /dev/null
+++ b/java/com/google/gerrit/testing/SystemPropertiesTestRule.java
@@ -0,0 +1,67 @@
+// 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.testing;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.common.Nullable;
+import java.util.Map;
+import java.util.Optional;
+import org.junit.rules.ExternalResource;
+
+/** Setup system properties before tests and return previous value after tests are finished */
+public class SystemPropertiesTestRule extends ExternalResource {
+ ImmutableMap<String, Optional<String>> properties;
+ @Nullable ImmutableMap<String, Optional<String>> previousValues;
+
+ public SystemPropertiesTestRule(String key, String value) {
+ this(ImmutableMap.of(key, Optional.of(value)));
+ }
+
+ public SystemPropertiesTestRule(Map<String, Optional<String>> properties) {
+ this.properties = ImmutableMap.copyOf(properties);
+ }
+
+ @Override
+ protected void before() throws Throwable {
+ super.before();
+ checkState(
+ previousValues == null,
+ "after() wasn't called after the previous call to the before() method");
+ ImmutableMap.Builder<String, Optional<String>> previousValuesBuilder = ImmutableMap.builder();
+ for (String key : properties.keySet()) {
+ previousValuesBuilder.put(key, Optional.ofNullable(System.getProperty(key)));
+ }
+ previousValues = previousValuesBuilder.build();
+ properties.entrySet().stream().forEach(this::setSystemProperty);
+ }
+
+ @Override
+ protected void after() {
+ checkState(previousValues != null, "before() wasn't called");
+ previousValues.entrySet().stream().forEach(this::setSystemProperty);
+ previousValues = null;
+ super.after();
+ }
+
+ private void setSystemProperty(Map.Entry<String, Optional<String>> keyValue) {
+ if (keyValue.getValue().isPresent()) {
+ System.setProperty(keyValue.getKey(), keyValue.getValue().get());
+ } else {
+ System.clearProperty(keyValue.getKey());
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index fff3cb6..b3592e3 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -14,9 +14,11 @@
package com.google.gerrit.acceptance.rest.change;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
+import static java.util.Comparator.comparing;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.ExtensionRegistry;
@@ -26,6 +28,10 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -403,4 +409,54 @@
assertSubmittable(change2Result.getChangeId());
}
+
+ @Test
+ public void stickyVoteStoredOnSubmitOnNewPatchset_withoutCopyCondition() throws Exception {
+ try (ProjectConfigUpdate u = updateProject(NameKey.parse("All-Projects"))) {
+ u.getConfig()
+ .updateLabelType(
+ LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ u.save();
+ }
+ stickyVoteStoredOnSubmitOnNewPatchset();
+ }
+
+ @Test
+ public void stickyVoteStoredOnSubmitOnNewPatchset_withCopyCondition() throws Exception {
+ // Code-Review will be sticky.
+ try (ProjectConfigUpdate u = updateProject(NameKey.parse("All-Projects"))) {
+ u.getConfig()
+ .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
+ u.save();
+ }
+ stickyVoteStoredOnSubmitOnNewPatchset();
+ }
+
+ private void stickyVoteStoredOnSubmitOnNewPatchset() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote.
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Submit, also keeping the Code-Review +2 vote.
+ gApi.changes().id(r.getChangeId()).current().submit();
+
+ // The last approval is stored on the submitted patch-set which was created by cherry-pick
+ // during submit.
+ PatchSetApproval patchSetApprovals =
+ Iterables.getLast(
+ r.getChange().notes().getApprovalsWithCopied().values().stream()
+ .filter(a -> a.labelId().equals(LabelId.create(LabelId.CODE_REVIEW)))
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList()));
+
+ assertThat(patchSetApprovals.patchSetId().get()).isEqualTo(2);
+ assertThat(patchSetApprovals.label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(patchSetApprovals.value()).isEqualTo((short) 2);
+
+ // The approval is not copied since we don't need to persist copied votes on submit, only
+ // persist votes normally.
+ assertThat(patchSetApprovals.copied()).isFalse();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index eeeac2a..aa93815 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -14,20 +14,27 @@
package com.google.gerrit.acceptance.rest.change;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static java.util.Comparator.comparing;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.ExtensionRegistry;
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.exceptions.InternalServerWithUserMessageException;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -159,6 +166,56 @@
}
}
+ @Test
+ public void stickyVoteStoredOnSubmitOnNewPatchset_withoutCopyCondition() throws Exception {
+ try (ProjectConfigUpdate u = updateProject(NameKey.parse("All-Projects"))) {
+ u.getConfig()
+ .updateLabelType(
+ LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ u.save();
+ }
+ stickyVoteStoredOnSubmitOnNewPatchset();
+ }
+
+ @Test
+ public void stickyVoteStoredOnSubmitOnNewPatchset_withCopyCondition() throws Exception {
+ // Code-Review will be sticky.
+ try (ProjectConfigUpdate u = updateProject(NameKey.parse("All-Projects"))) {
+ u.getConfig()
+ .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
+ u.save();
+ }
+ stickyVoteStoredOnSubmitOnNewPatchset();
+ }
+
+ private void stickyVoteStoredOnSubmitOnNewPatchset() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote.
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Submit, also keeping the Code-Review +2 vote.
+ gApi.changes().id(r.getChangeId()).current().submit();
+
+ // The last approval is stored on the submitted patch-set which was created by rebase during
+ // submit.
+ PatchSetApproval patchSetApprovals =
+ Iterables.getLast(
+ r.getChange().notes().getApprovalsWithCopied().values().stream()
+ .filter(a -> a.labelId().equals(LabelId.create(LabelId.CODE_REVIEW)))
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList()));
+
+ assertThat(patchSetApprovals.patchSetId().get()).isEqualTo(2);
+ assertThat(patchSetApprovals.label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(patchSetApprovals.value()).isEqualTo((short) 2);
+
+ // The approval is not copied since we don't need to persist copied votes on submit, only
+ // persist votes normally.
+ assertThat(patchSetApprovals.copied()).isFalse();
+ }
+
private void assertLatestRevisionHasFooters(PushOneCommit.Result change) throws Throwable {
RevCommit c = getCurrentCommit(change);
assertThat(c.getFooterLines(FooterConstants.CHANGE_ID)).isNotEmpty();
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java b/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java
index f5e4e09..8e0d4bb 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java
@@ -20,29 +20,19 @@
import com.google.gerrit.index.IndexType;
import com.google.gerrit.index.testing.AbstractFakeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
+import com.google.gerrit.testing.SystemPropertiesTestRule;
import javax.inject.Inject;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test to check that the expected index backend was bound depending on sys/env properties. */
public class DefaultIndexBindingIT extends AbstractDaemonTest {
+ @ClassRule
+ public static SystemPropertiesTestRule systemProperties =
+ new SystemPropertiesTestRule(IndexType.SYS_PROP, "");
@Inject private ChangeIndexCollection changeIndex;
- private static String propertyBeforeTest;
-
- @BeforeClass
- public static void setup() {
- propertyBeforeTest = System.getProperty(IndexType.SYS_PROP);
- System.setProperty(IndexType.SYS_PROP, "");
- }
-
- @AfterClass
- public static void teardown() {
- System.setProperty(IndexType.SYS_PROP, propertyBeforeTest);
- }
-
@Test
public void fakeIsBoundByDefault() throws Exception {
assertThat(System.getProperty(IndexType.SYS_PROP)).isEmpty();
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java b/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java
index 4122426..acb2e5a 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java
@@ -20,29 +20,19 @@
import com.google.gerrit.index.IndexType;
import com.google.gerrit.index.testing.AbstractFakeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
+import com.google.gerrit.testing.SystemPropertiesTestRule;
import javax.inject.Inject;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test to check that the expected index backend was bound depending on sys/env properties. */
public class FakeIndexBindingIT extends AbstractDaemonTest {
+ @ClassRule
+ public static SystemPropertiesTestRule systemProperties =
+ new SystemPropertiesTestRule(IndexType.SYS_PROP, "fake");
@Inject private ChangeIndexCollection changeIndex;
- private static String propertyBeforeTest;
-
- @BeforeClass
- public static void setup() {
- propertyBeforeTest = System.getProperty(IndexType.SYS_PROP);
- System.setProperty(IndexType.SYS_PROP, "fake");
- }
-
- @AfterClass
- public static void teardown() {
- System.setProperty(IndexType.SYS_PROP, propertyBeforeTest);
- }
-
@Test
public void fakeIsBoundWhenConfigured() throws Exception {
assertThat(System.getProperty(IndexType.SYS_PROP)).isEqualTo("fake");
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java b/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java
index 31e31fd..5dd6f01 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java
@@ -20,29 +20,19 @@
import com.google.gerrit.index.IndexType;
import com.google.gerrit.lucene.LuceneChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
+import com.google.gerrit.testing.SystemPropertiesTestRule;
import javax.inject.Inject;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
/** Test to check that the expected index backend was bound depending on sys/env properties. */
public class LuceneIndexBindingIT extends AbstractDaemonTest {
+ @ClassRule
+ public static SystemPropertiesTestRule systemProperties =
+ new SystemPropertiesTestRule(IndexType.SYS_PROP, "lucene");
@Inject private ChangeIndexCollection changeIndex;
- private static String propertyBeforeTest;
-
- @BeforeClass
- public static void setup() {
- propertyBeforeTest = System.getProperty(IndexType.SYS_PROP);
- System.setProperty(IndexType.SYS_PROP, "lucene");
- }
-
- @AfterClass
- public static void teardown() {
- System.setProperty(IndexType.SYS_PROP, propertyBeforeTest);
- }
-
@Test
public void luceneIsBoundWhenConfigured() throws Exception {
assertThat(System.getProperty(IndexType.SYS_PROP)).isEqualTo("lucene");
diff --git a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
index 056c7dc..98721fd 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
@@ -24,6 +24,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
@@ -33,6 +34,7 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.git.RefUpdateUtil;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -48,7 +50,9 @@
import java.sql.Timestamp;
import java.util.Arrays;
import java.util.List;
+import java.util.Map;
import java.util.stream.IntStream;
+import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
@@ -56,6 +60,8 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -70,6 +76,21 @@
@Before
public void setUp() throws Exception {}
+ @After
+ public void cleanUp() throws Exception {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setAllowNonFastForwards(true);
+ for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
+ Change.Id changeId = Change.Id.fromRef(ref.getName());
+ if (changeId == null || !ref.getName().equals(RefNames.changeMetaRef(changeId))) {
+ continue;
+ }
+ bru.addCommand(new ReceiveCommand(ref.getObjectId(), ObjectId.zeroId(), ref.getName()));
+ }
+
+ RefUpdateUtil.executeChecked(bru, repo);
+ }
+
@Test
public void validHistoryNoOp() throws Exception {
String tag = "jenkins";
@@ -157,6 +178,138 @@
}
@Test
+ public void numRefs_greater_maxRefsToUpdate_allFixed() throws Exception {
+ int numberOfChanges = 12;
+ ImmutableMap.Builder<String, Ref> refsToOldMetaBuilder = new ImmutableMap.Builder<>();
+ for (int i = 0; i < numberOfChanges; i++) {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeMessage("Change has been successfully merged by " + changeOwner.getName());
+ update.commit();
+ ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
+ updateWithSubject.setSubjectForCommit("Update with subject");
+ updateWithSubject.commit();
+ String refName = RefNames.changeMetaRef(c.getId());
+ Ref metaRefBeforeRewrite = repo.exactRef(refName);
+ refsToOldMetaBuilder.put(refName, metaRefBeforeRewrite);
+ }
+ ImmutableMap<String, Ref> refsToOldMeta = refsToOldMetaBuilder.build();
+
+ RunOptions options = new RunOptions();
+ options.dryRun = false;
+ options.outputDiff = false;
+ options.verifyCommits = false;
+ options.maxRefsInBatch = 10;
+ options.maxRefsToUpdate = 12;
+ BackfillResult backfillResult = rewriter.backfillProject(project, repo, options);
+ assertThat(backfillResult.fixedRefDiff.keySet()).isEqualTo(refsToOldMeta.keySet());
+ for (Map.Entry<String, Ref> refEntry : refsToOldMeta.entrySet()) {
+ Ref metaRefAfterRewrite = repo.exactRef(refEntry.getKey());
+ assertThat(refEntry.getValue()).isNotEqualTo(metaRefAfterRewrite);
+ }
+ }
+
+ @Test
+ public void maxRefsToUpdate_coversAllInvalid_inMultipleBatches() throws Exception {
+ testMaxRefsToUpdate(
+ /*numberOfInvalidChanges=*/ 11,
+ /*numberOfValidChanges=*/ 9,
+ /*maxRefsToUpdate=*/ 12,
+ /*maxRefsInBatch=*/ 2);
+ }
+
+ @Test
+ public void maxRefsToUpdate_coversAllInvalid_inSingleBatch() throws Exception {
+ testMaxRefsToUpdate(
+ /*numberOfInvalidChanges=*/ 11,
+ /*numberOfValidChanges=*/ 9,
+ /*maxRefsToUpdate=*/ 12,
+ /*maxRefsInBatch=*/ 12);
+ }
+
+ @Test
+ public void moreInvalidRefs_thenMaxRefsToUpdate_inMultipleBatches() throws Exception {
+ testMaxRefsToUpdate(
+ /*numberOfInvalidChanges=*/ 11,
+ /*numberOfValidChanges=*/ 9,
+ /*maxRefsToUpdate=*/ 10,
+ /*maxRefsInBatch=*/ 2);
+ }
+
+ @Test
+ public void moreInvalidRefs_thenMaxRefsToUpdate_inSingleBatch() throws Exception {
+ testMaxRefsToUpdate(
+ /*numberOfInvalidChanges=*/ 11,
+ /*numberOfValidChanges=*/ 9,
+ /*maxRefsToUpdate=*/ 10,
+ /*maxRefsInBatch=*/ 10);
+ }
+
+ private void testMaxRefsToUpdate(
+ int numberOfInvalidChanges, int numberOfValidChanges, int maxRefsToUpdate, int maxRefsInBatch)
+ throws Exception {
+ ImmutableMap.Builder<String, ObjectId> expectedFixedRefsToOldMetaBuilder =
+ new ImmutableMap.Builder<>();
+ ImmutableMap.Builder<String, ObjectId> expectedSkippedRefsToOldMetaBuilder =
+ new ImmutableMap.Builder<>();
+ for (int i = 0; i < numberOfValidChanges; i++) {
+ Change c = newChange();
+ ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
+ updateWithSubject.setSubjectForCommit("Update with subject");
+ updateWithSubject.commit();
+ String refName = RefNames.changeMetaRef(c.getId());
+ Ref metaRefBeforeRewrite = repo.exactRef(refName);
+ expectedSkippedRefsToOldMetaBuilder.put(refName, metaRefBeforeRewrite.getObjectId());
+ }
+ for (int i = 0; i < numberOfInvalidChanges; i++) {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setChangeMessage("Change has been successfully merged by " + changeOwner.getName());
+ update.commit();
+ ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
+ updateWithSubject.setSubjectForCommit("Update with subject");
+ updateWithSubject.commit();
+ String refName = RefNames.changeMetaRef(c.getId());
+ Ref metaRefBeforeRewrite = repo.exactRef(refName);
+ if (i < maxRefsToUpdate) {
+ expectedFixedRefsToOldMetaBuilder.put(refName, metaRefBeforeRewrite.getObjectId());
+ } else {
+ expectedSkippedRefsToOldMetaBuilder.put(refName, metaRefBeforeRewrite.getObjectId());
+ }
+ }
+ ImmutableMap<String, ObjectId> expectedFixedRefsToOldMeta =
+ expectedFixedRefsToOldMetaBuilder.build();
+ ImmutableMap<String, ObjectId> expectedSkippedRefsToOldMeta =
+ expectedSkippedRefsToOldMetaBuilder.build();
+ RunOptions options = new RunOptions();
+ options.dryRun = false;
+ options.outputDiff = false;
+ options.verifyCommits = false;
+ options.maxRefsInBatch = maxRefsInBatch;
+ options.maxRefsToUpdate = maxRefsToUpdate;
+ BackfillResult backfillResult = rewriter.backfillProject(project, repo, options);
+ assertThat(backfillResult.fixedRefDiff.keySet()).isEqualTo(expectedFixedRefsToOldMeta.keySet());
+ for (Map.Entry<String, ObjectId> refEntry : expectedFixedRefsToOldMeta.entrySet()) {
+ Ref metaRefAfterRewrite = repo.exactRef(refEntry.getKey());
+ assertThat(refEntry.getValue()).isNotEqualTo(metaRefAfterRewrite.getObjectId());
+ }
+ for (Map.Entry<String, ObjectId> refEntry : expectedSkippedRefsToOldMeta.entrySet()) {
+ Ref metaRefAfterRewrite = repo.exactRef(refEntry.getKey());
+ assertThat(refEntry.getValue()).isEqualTo(metaRefAfterRewrite.getObjectId());
+ }
+ RunOptions secondRunOptions = new RunOptions();
+ secondRunOptions.dryRun = false;
+ secondRunOptions.outputDiff = false;
+ secondRunOptions.verifyCommits = false;
+ secondRunOptions.maxRefsInBatch = maxRefsInBatch;
+ secondRunOptions.maxRefsToUpdate = numberOfInvalidChanges + numberOfValidChanges;
+ BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
+ int expectedSecondRunResult =
+ numberOfInvalidChanges > maxRefsToUpdate ? numberOfInvalidChanges - maxRefsToUpdate : 0;
+ assertThat(secondRunResult.fixedRefDiff.keySet().size()).isEqualTo(expectedSecondRunResult);
+ }
+
+ @Test
public void fixAuthorIdent() throws Exception {
Change c = newChange();
Timestamp when = TimeUtil.nowTs();
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 9ebee9c..65b725b 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1726,6 +1726,8 @@
assertQuery(predicate + "\"2009-10-01 21:02:00\"", change1);
assertQuery(predicate + "2009-10-01", change1);
assertQuery(predicate + "2009-10-03", change2, change1);
+ assertQuery(predicate + "\"2009-09-30 21:00:00 -0000\"", change1);
+ assertQuery(predicate + "\"2009-10-02 03:00:00 -0000\"", change2, change1);
}
// Same test as above, but using filter code path.
@@ -1742,6 +1744,12 @@
assertQuery(makeIndexedPredicateFilterQuery(predicate + "\"2009-10-01 21:02:00\""), change1);
assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-01"), change1);
assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-03"), change2, change1);
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-09-30 21:00:00 -0000\""), change1);
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-10-02 03:00:00 -0000\""),
+ change2,
+ change1);
}
}
@@ -1763,6 +1771,8 @@
assertQuery(predicate + "\"2009-10-01 20:59:59 -0000\"", change2);
assertQuery(predicate + "2009-10-01", change2);
assertQuery(predicate + "2009-09-30", change2, change1);
+ assertQuery(predicate + "\"2009-09-30 21:00:00 -0000\"", change2, change1);
+ assertQuery(predicate + "\"2009-10-02 03:00:00 -0000\"", change2);
}
// Same test as above, but using filter code path.
@@ -1774,6 +1784,12 @@
makeIndexedPredicateFilterQuery(predicate + "\"2009-10-01 20:59:59 -0000\""), change2);
assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-01"), change2);
assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-09-30"), change2, change1);
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-09-30 21:00:00 -0000\""),
+ change2,
+ change1);
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-10-02 03:00:00 -0000\""), change2);
}
}
diff --git a/javatests/com/google/gerrit/testing/BUILD b/javatests/com/google/gerrit/testing/BUILD
index 5774707..9443b0d 100644
--- a/javatests/com/google/gerrit/testing/BUILD
+++ b/javatests/com/google/gerrit/testing/BUILD
@@ -7,6 +7,7 @@
deps = [
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:jgit",
"//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/testing/ConfigSuiteTest.java b/javatests/com/google/gerrit/testing/ConfigSuiteTest.java
new file mode 100644
index 0000000..1ec30da
--- /dev/null
+++ b/javatests/com/google/gerrit/testing/ConfigSuiteTest.java
@@ -0,0 +1,266 @@
+// 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.testing;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.verify;
+
+import com.google.gerrit.testing.ConfigSuite.AfterConfig;
+import com.google.gerrit.testing.ConfigSuite.BeforeConfig;
+import com.google.gerrit.testing.ConfigSuite.ConfigRule;
+import org.eclipse.jgit.lib.Config;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runner.Result;
+import org.junit.runner.RunWith;
+import org.junit.runner.notification.RunNotifier;
+import org.junit.runners.model.Statement;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ConfigSuiteTest {
+ @Mock private ConfigBasedTestListener configBasedTestListener;
+ private RunNotifier notifier;
+
+ @Before
+ public void setUp() {
+ notifier = new RunNotifier();
+ ConfigBasedTest.setConfigBasedTestListener(configBasedTestListener);
+ }
+
+ @After
+ public void tearDown() {
+ ConfigBasedTest.setConfigBasedTestListener(null);
+ }
+
+ @Test
+ public void methodsExecuteInCorrectOrder() throws Exception {
+ InOrder inOrder = Mockito.inOrder(configBasedTestListener);
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).classRuleExecuted();
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).beforeClassExecuted();
+ // default config
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).configRuleExectued();
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).beforeConfigExecuted();
+ inOrder.verify(configBasedTestListener, Mockito.times(2)).testExecuted(any(), any(), any());
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).afterConfigExecuted();
+ // first config
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).configRuleExectued();
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).beforeConfigExecuted();
+ inOrder.verify(configBasedTestListener, Mockito.times(2)).testExecuted(any(), any(), any());
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).afterConfigExecuted();
+ // second config
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).configRuleExectued();
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).beforeConfigExecuted();
+ inOrder.verify(configBasedTestListener, Mockito.times(2)).testExecuted(any(), any(), any());
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).afterConfigExecuted();
+
+ inOrder.verify(configBasedTestListener, Mockito.times(1)).afterClassExecuted();
+ }
+
+ @Test
+ public void beforeClassExecutedOnce() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ verify(configBasedTestListener, Mockito.times(1)).beforeClassExecuted();
+ }
+
+ @Test
+ public void afterClassExecutedOnce() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ verify(configBasedTestListener, Mockito.times(1)).afterClassExecuted();
+ }
+
+ @Test
+ public void classRuleExecutedOnlyOnce() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ verify(configBasedTestListener, Mockito.times(1)).classRuleExecuted();
+ }
+
+ @Test
+ public void beforeConfigExecutedForEachConfig() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ // default, firstConfig, secondConfig
+ verify(configBasedTestListener, Mockito.times(3)).beforeConfigExecuted();
+ }
+
+ @Test
+ public void afterConfigExecutedForEachConfig() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ // default, firstConfig, secondConfig
+ verify(configBasedTestListener, Mockito.times(3)).afterConfigExecuted();
+ }
+
+ @Test
+ public void configRuleExecutedForEachConfig() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ // default, firstConfig, secondConfig
+ verify(configBasedTestListener, Mockito.times(3)).afterConfigExecuted();
+ }
+
+ @Test
+ public void testsExecuteWithCorrectConfigAndName() throws Exception {
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ verify(configBasedTestListener, Mockito.times(6)).testExecuted(any(), any(), any());
+
+ verify(configBasedTestListener, Mockito.times(1)).testExecuted("test1", "default", null);
+ verify(configBasedTestListener, Mockito.times(1)).testExecuted("test2", "default", null);
+ verify(configBasedTestListener, Mockito.times(1))
+ .testExecuted("test1", "firstValue", "firstConfig");
+ verify(configBasedTestListener, Mockito.times(1))
+ .testExecuted("test2", "firstValue", "firstConfig");
+ verify(configBasedTestListener, Mockito.times(1))
+ .testExecuted("test1", "secondValue", "secondConfig");
+ verify(configBasedTestListener, Mockito.times(1))
+ .testExecuted("test2", "secondValue", "secondConfig");
+ }
+
+ @Test
+ public void testResultWasSuccessful() throws Exception {
+ Result result = new Result();
+ notifier.addListener(result.createListener());
+ new ConfigSuite(ConfigBasedTest.class).run(notifier);
+ assertThat(result.wasSuccessful()).isTrue();
+ }
+
+ @Test
+ public void failedTestResultNotMissed() throws Exception {
+ Result result = new Result();
+ notifier.addListener(result.createListener());
+ new ConfigSuite(FailedConfigBasedTest.class).run(notifier);
+ // 3 fails with 3 different configs
+ assertThat(result.wasSuccessful()).isFalse();
+ assertThat(result.getFailureCount()).isEqualTo(3);
+ }
+
+ interface ConfigBasedTestListener {
+ void beforeClassExecuted();
+
+ void afterClassExecuted();
+
+ void classRuleExecuted();
+
+ void configRuleExectued();
+
+ void testExecuted(String testName, String testValue, String configName);
+
+ void beforeConfigExecuted();
+
+ void afterConfigExecuted();
+ }
+
+ public static class ConfigBasedTest {
+ private static ConfigBasedTestListener listener;
+
+ public static void setConfigBasedTestListener(ConfigBasedTestListener listener) {
+ ConfigBasedTest.listener = listener;
+ }
+
+ @BeforeClass
+ public static void beforeClass() {
+ listener.beforeClassExecuted();
+ }
+
+ @AfterClass
+ public static void afterClass() {
+ listener.afterClassExecuted();
+ }
+
+ @ClassRule
+ public static TestRule classRule =
+ new TestRule() {
+ @Override
+ public Statement apply(Statement statement, Description description) {
+ return new Statement() {
+ @Override
+ public void evaluate() throws Throwable {
+ listener.classRuleExecuted();
+ statement.evaluate();
+ }
+ };
+ }
+ };
+
+ @ConfigRule
+ public static TestRule configRule =
+ new TestRule() {
+ @Override
+ public Statement apply(Statement statement, Description description) {
+ return new Statement() {
+ @Override
+ public void evaluate() throws Throwable {
+ listener.configRuleExectued();
+ statement.evaluate();
+ }
+ };
+ }
+ };
+
+ @BeforeConfig
+ public static void beforeConfig() {
+ listener.beforeConfigExecuted();
+ }
+
+ @AfterConfig
+ public static void afterConfig() {
+ listener.afterConfigExecuted();
+ }
+
+ @ConfigSuite.Config
+ public static Config firstConfig() {
+ Config cfg = new Config();
+ cfg.setString("gerrit", null, "testValue", "firstValue");
+ return cfg;
+ }
+
+ @ConfigSuite.Config
+ public static Config secondConfig() {
+ Config cfg = new Config();
+ cfg.setString("gerrit", null, "testValue", "secondValue");
+ return cfg;
+ }
+
+ @ConfigSuite.Parameter public Config config;
+ @ConfigSuite.Name public String name;
+
+ @Test
+ public void test1() {
+ String testValue = config.getString("gerrit", null, "testValue");
+ listener.testExecuted("test1", testValue != null ? testValue : "default", name);
+ }
+
+ @Test
+ public void test2() {
+ String testValue = config.getString("gerrit", null, "testValue");
+ listener.testExecuted("test2", testValue != null ? testValue : "default", name);
+ }
+ }
+
+ public static class FailedConfigBasedTest extends ConfigBasedTest {
+ @Test
+ public void failedTest() {
+ assertThat(true).isFalse();
+ }
+ }
+}
diff --git a/tools/BUILD b/tools/BUILD
index e44ae78..36507a9 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -240,8 +240,8 @@
"-Xep:InstanceOfAndCastMatchWrongType:ERROR",
"-Xep:InstantTemporalUnit:ERROR",
"-Xep:IntLongMath:ERROR",
- # "-Xep:InvalidBlockTag:WARN",
- "-Xep:InvalidInlineTag:WARN",
+ "-Xep:InvalidBlockTag:ERROR",
+ "-Xep:InvalidInlineTag:ERROR",
"-Xep:InvalidJavaTimeConstant:ERROR",
"-Xep:InvalidLink:ERROR",
# "-Xep:InvalidParam:WARN",
