Merge "Merge branch 'stable-3.2' into stable-3.3" into stable-3.3
diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt
index 4b1312a..c50a293 100644
--- a/Documentation/dev-e2e-tests.txt
+++ b/Documentation/dev-e2e-tests.txt
@@ -232,6 +232,17 @@
Scenario development is often done using locally running Gerrit systems under test, which are
sometimes dockerized.
+==== Number of users
+
+The `number_of_users` property can be used to scale scenario steps to run with the specified number
+of concurrent users. The value of this property remains `1` by default. For example, this sets the
+number of concurrent users to 10:
+
+* `-Dcom.google.gerrit.scenarios.number_of_users=10`
+
+This will make scenarios that support the `number_of_users` property to inject that many users
+concurrently for load testing.
+
== How to run tests
Run all tests:
diff --git a/e2e-tests/README b/e2e-tests/README
new file mode 100755
index 0000000..59e0fba
--- /dev/null
+++ b/e2e-tests/README
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# Example usage only-
+# 1. Optional: replace test@mail.com below with your own, reachable locally.
+# 2. Use the '>>' operator below instead to not overwrite your known_hosts; keep '>' otherwise.
+# 3. Note that appending as proposed above may potentially repeat the same line multiple times.
+# 4. Init your local Gerrit test site then start it; you may refer to [1] below.
+# 5. Set GIT_HTTP_PASSWORD below to yours, from [2].
+# 6. Change to this directory to execute ./README (this executable file) in its own terminal.
+# 7. Install sbt if missing, based on your operating system; re-run to compile.
+# 8. Optional: add the below generated (displayed) key to your local admin user [3].
+# 9. Otherwise keep the lines below that use your existing user ssh keys for admin testing.
+# 10. This script assumes the google-sourced version of the example json file [4].
+# 11. If running that scenario locally as below reports authentication failures, [4] may be a fork.
+# 12. Uncomment any one of the below sbt commands at will; you may add some locally.
+# 13. See [5] for how to start using JAVA_OPTS below; you may leave it empty for these sbt commands.
+# 14. You can initialize an IDE sbt (Scala) project from/in this root folder; see [6].
+#
+# [1] https://gerrit-review.googlesource.com/Documentation/dev-readme.html#init
+# [2] http://localhost:8080/settings/#HTTPCredentials
+# [3] http://localhost:8080/settings/#SSHKeys
+# [4] ./src/test/resources/data/com/google/gerrit/scenarios/CloneUsingBothProtocols.json
+# [5] https://gerrit-review.googlesource.com/Documentation/dev-e2e-tests.html#_environment_properties
+# [6] https://gerrit-review.googlesource.com/Documentation/dev-e2e-tests.html#_ide_intellij
+
+# DO NOT change this (assumed) directory; force-removed *recursively* below!
+gatlingGitKeys=/tmp/ssh-keys
+
+userSshDir=$HOME/.ssh
+
+# Comment this group of lines out if willing to generate other keys as below.
+rm -f $gatlingGitKeys
+ln -s "$userSshDir" $gatlingGitKeys
+
+# Comment this group of lines out if keys already generated, as either below or above.
+#rm -fr $gatlingGitKeys
+#mkdir $gatlingGitKeys
+#ssh-keygen -m PEM -t rsa -C "test@mail.com" -f $gatlingGitKeys/id_rsa
+
+ssh-keyscan -t rsa -p 29418 localhost > "$userSshDir"/known_hosts
+cat $gatlingGitKeys/id_rsa.pub
+
+export GIT_HTTP_USERNAME="admin"
+export GIT_HTTP_PASSWORD="TODO"
+export JAVA_OPTS="\
+"
+#-Dx=y \
+
+#sbt clean
+#sbt update
+sbt compile
+#sbt "gatling:testOnly com.google.gerrit.scenarios.CloneUsingBothProtocols"
+#sbt "gatling:lastReport"
diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch-body.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch-body.json
new file mode 100644
index 0000000..f69e575
--- /dev/null
+++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch-body.json
@@ -0,0 +1,3 @@
+{
+ "revision": "master"
+}
diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch.json
new file mode 100644
index 0000000..5459f11
--- /dev/null
+++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateBranch.json
@@ -0,0 +1,6 @@
+[
+ {
+ "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/projects/PROJECT/branches/",
+ "project": "PROJECT"
+ }
+]
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
index 08966a8..c283861 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
@@ -21,9 +21,9 @@
import scala.concurrent.duration._
class CloneUsingBothProtocols extends GitSimulation {
- private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+ private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
private val projectName = className
- private val duration = 2
+ private val duration = 2 * numberOfUsers
override def replaceOverride(in: String): String = {
replaceKeyWith("_project", projectName, in)
@@ -43,7 +43,7 @@
),
test.inject(
nothingFor(stepWaitTime(this) seconds),
- constantUsersPerSec(single) during (duration seconds)
+ constantUsersPerSec(numberOfUsers) during (duration seconds)
).protocols(gitProtocol),
deleteProject.test.inject(
nothingFor(stepWaitTime(deleteProject) + duration seconds),
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateBranch.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateBranch.scala
new file mode 100644
index 0000000..ccde633
--- /dev/null
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateBranch.scala
@@ -0,0 +1,56 @@
+// 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.scenarios
+
+import io.gatling.core.Predef.{atOnceUsers, _}
+import io.gatling.core.feeder.FeederBuilder
+import io.gatling.core.structure.ScenarioBuilder
+import io.gatling.http.Predef._
+
+import scala.concurrent.duration._
+
+class CreateBranch extends ProjectSimulation {
+ private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
+ private val branchIdKey = "branchId"
+ private var counter = 0
+
+ private val test: ScenarioBuilder = scenario(uniqueName)
+ .feed(data)
+ .exec(session => {
+ counter += 1
+ session.set(branchIdKey, "branch-" + counter)
+ })
+ .exec(http(uniqueName)
+ .post("${url}${" + branchIdKey + "}")
+ .body(ElFileBody(body)).asJson)
+
+ private val createProject = new CreateProject(projectName)
+ private val deleteProject = new DeleteProject(projectName)
+
+ setUp(
+ createProject.test.inject(
+ nothingFor(stepWaitTime(createProject) seconds),
+ atOnceUsers(single)
+ ),
+ test.inject(
+ nothingFor(stepWaitTime(this) seconds),
+ atOnceUsers(numberOfUsers)
+ ),
+ deleteProject.test.inject(
+ nothingFor(stepWaitTime(deleteProject) seconds),
+ atOnceUsers(single)
+ ),
+ ).protocols(httpProtocol)
+}
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateChange.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateChange.scala
index f3692a9..b0063cb 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateChange.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateChange.scala
@@ -19,12 +19,14 @@
import io.gatling.core.structure.ScenarioBuilder
import io.gatling.http.Predef._
+import scala.collection.mutable
import scala.concurrent.duration._
class CreateChange extends ProjectSimulation {
- private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+ private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
private val numberKey = "_number"
var number = 0
+ var numbers: mutable.Queue[Int] = mutable.Queue[Int]()
override def relativeRuntimeWeight = 2
@@ -40,6 +42,7 @@
.check(regex("\"" + numberKey + "\":(\\d+),").saveAs(numberKey)))
.exec(session => {
number = session(numberKey).as[Int]
+ numbers += number
session
})
@@ -54,11 +57,11 @@
),
test.inject(
nothingFor(stepWaitTime(this) seconds),
- atOnceUsers(single)
+ atOnceUsers(numberOfUsers)
),
deleteChange.test.inject(
nothingFor(stepWaitTime(deleteChange) seconds),
- atOnceUsers(single)
+ atOnceUsers(numberOfUsers)
),
deleteProject.test.inject(
nothingFor(stepWaitTime(deleteProject) seconds),
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteChange.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteChange.scala
index d832bde..e47108f 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteChange.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteChange.scala
@@ -20,7 +20,7 @@
import io.gatling.http.Predef.http
class DeleteChange extends GerritSimulation {
- private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+ private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
private var createChange: Option[CreateChange] = None
override def relativeRuntimeWeight = 2
@@ -34,7 +34,7 @@
.feed(data)
.exec(session => {
if (createChange.nonEmpty) {
- session.set("number", createChange.get.number)
+ session.set("number", createChange.get.numbers.dequeue())
} else {
session
}
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
index 7b31b3d..b11c87c 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
@@ -34,6 +34,7 @@
protected val uniqueName: String = className + "-" + hashCode()
protected val single = 1
+ val numberOfUsers: Int = replaceProperty("number_of_users", single).toInt
val replicationDelay: Int = replaceProperty("replication_delay", 15).toInt
private val powerFactor = replaceProperty("power_factor", 1.0).toDouble
protected val SecondsPerWeightUnit = 2
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 3b4aa01..ce5de3c 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -47,7 +47,6 @@
import static javax.servlet.http.HttpServletResponse.SC_PRECONDITION_FAILED;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.CharMatcher;
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
@@ -140,7 +139,6 @@
import com.google.gson.GsonBuilder;
import com.google.gson.JsonElement;
import com.google.gson.JsonParseException;
-import com.google.gson.JsonPrimitive;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.JsonWriter;
@@ -1863,10 +1861,6 @@
static long replyText(
@Nullable HttpServletRequest req, HttpServletResponse res, boolean allowTracing, String text)
throws IOException {
- if ((req == null || isRead(req)) && isMaybeHTML(text)) {
- return replyJson(
- req, res, allowTracing, ImmutableListMultimap.of("pp", "0"), new JsonPrimitive(text));
- }
if (!text.endsWith("\n")) {
text += "\n";
}
@@ -1876,10 +1870,6 @@
return replyBinaryResult(req, res, BinaryResult.create(text).setContentType(PLAIN_TEXT));
}
- private static boolean isMaybeHTML(String text) {
- return CharMatcher.anyOf("<&").matchesAnyOf(text);
- }
-
private static boolean acceptsGzip(HttpServletRequest req) {
if (req != null) {
String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING);
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index eea1052..46e8d33 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -22,7 +22,10 @@
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.config.UrlFormatter;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.inject.Singleton;
import java.io.IOException;
import java.security.SecureRandom;
@@ -34,6 +37,8 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -109,6 +114,37 @@
id);
}
+ /**
+ * Make sure that the change commit message has a correct footer.
+ *
+ * @param requireChangeId true if Change-Id is a mandatory footer for the project
+ * @param currentChangeId current Change-Id value before the commit message is updated
+ * @param newCommitMessage new commit message for the change
+ * @throws ResourceConflictException if the new commit message has a missing or invalid Change-Id
+ * @throws BadRequestException if the new commit message is null or empty
+ */
+ public static void ensureChangeIdIsCorrect(
+ boolean requireChangeId, String currentChangeId, String newCommitMessage)
+ throws ResourceConflictException, BadRequestException {
+ RevCommit revCommit =
+ RevCommit.parse(
+ Constants.encode("tree " + ObjectId.zeroId().name() + "\n\n" + newCommitMessage));
+
+ // Check that the commit message without footers is not empty
+ CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
+
+ List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
+ if (requireChangeId && changeIdFooters.isEmpty()) {
+ throw new ResourceConflictException("missing Change-Id footer");
+ }
+ if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
+ throw new ResourceConflictException("wrong Change-Id footer");
+ }
+ if (changeIdFooters.size() > 1) {
+ throw new ResourceConflictException("multiple Change-Id footers");
+ }
+ }
+
public static String status(Change c) {
return c != null ? c.getStatus().name().toLowerCase() : "deleted";
}
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 3bfabdd..bc905c2 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import com.google.common.base.Charsets;
+import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
@@ -367,8 +368,15 @@
PatchSet basePatchset = modificationTarget.getBasePatchset();
RevCommit basePatchsetCommit = NoteDbEdits.lookupCommit(repository, basePatchset.commitId());
+ boolean changeIdRequired =
+ projectCache
+ .get(notes.getChange().getProject())
+ .orElseThrow(illegalState(notes.getChange().getProject()))
+ .is(BooleanProjectConfig.REQUIRE_CHANGE_ID);
+ String currentChangeId = notes.getChange().getKey().get();
String newCommitMessage =
- createNewCommitMessage(editBehavior, commitModification, commitToModify);
+ createNewCommitMessage(
+ changeIdRequired, currentChangeId, editBehavior, commitModification, commitToModify);
newCommitMessage = editBehavior.mergeCommitMessageIfNecessary(newCommitMessage, commitToModify);
Optional<ChangeEdit> unmodifiedEdit =
@@ -464,8 +472,12 @@
}
private String createNewCommitMessage(
- EditBehavior editBehavior, CommitModification commitModification, RevCommit commitToModify)
- throws InvalidChangeOperationException, BadRequestException {
+ boolean requireChangeId,
+ String currentChangeId,
+ EditBehavior editBehavior,
+ CommitModification commitModification,
+ RevCommit commitToModify)
+ throws InvalidChangeOperationException, BadRequestException, ResourceConflictException {
if (!commitModification.newCommitMessage().isPresent()) {
return editBehavior.getUnmodifiedCommitMessage(commitToModify);
}
@@ -479,6 +491,8 @@
"New commit message cannot be same as existing commit message");
}
+ ChangeUtil.ensureChangeIdIsCorrect(requireChangeId, currentChangeId, newCommitMessage);
+
return newCommitMessage;
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 190deb5..37318d0 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -51,11 +50,9 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
-import java.util.List;
import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@@ -116,14 +113,13 @@
String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
ensureCanEditCommitMessage(resource.getNotes());
- sanitizedCommitMessage =
- ensureChangeIdIsCorrect(
- projectCache
- .get(resource.getProject())
- .orElseThrow(illegalState(resource.getProject()))
- .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
- resource.getChange().getKey().get(),
- sanitizedCommitMessage);
+ ChangeUtil.ensureChangeIdIsCorrect(
+ projectCache
+ .get(resource.getProject())
+ .orElseThrow(illegalState(resource.getProject()))
+ .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+ resource.getChange().getKey().get(),
+ sanitizedCommitMessage);
try (Repository repository = repositoryManager.openRepository(resource.getProject());
RevWalk revWalk = new RevWalk(repository);
@@ -204,33 +200,4 @@
throw new AuthException("modifying commit message not permitted", denied);
}
}
-
- private String ensureChangeIdIsCorrect(
- boolean requireChangeId, String currentChangeId, String newCommitMessage)
- throws ResourceConflictException, BadRequestException {
- RevCommit revCommit =
- RevCommit.parse(
- Constants.encode("tree " + ObjectId.zeroId().name() + "\n\n" + newCommitMessage));
-
- // Check that the commit message without footers is not empty
- CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
-
- List<String> changeIdFooters = ChangeUtil.getChangeIdsFromFooter(revCommit, urlFormatter.get());
- if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
- throw new ResourceConflictException("wrong Change-Id footer");
- }
-
- if (requireChangeId && revCommit.getFooterLines().isEmpty()) {
- // sanitization always adds '\n' at the end.
- newCommitMessage += "\n";
- }
-
- if (requireChangeId && changeIdFooters.isEmpty()) {
- newCommitMessage += FooterConstants.CHANGE_ID.getName() + ": " + currentChangeId + "\n";
- } else if (changeIdFooters.size() > 1) {
- throw new ResourceConflictException("multiple Change-Id footers");
- }
-
- return newCommitMessage;
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 18f1223..0c30ef5 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3667,16 +3667,6 @@
}
@Test
- public void changeCommitMessageWithNoChangeIdRetainsChangeID() throws Exception {
- PushOneCommit.Result r = createChange();
- assertThat(getCommitMessage(r.getChangeId()))
- .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
- gApi.changes().id(r.getChangeId()).setMessage("modified commit\n");
- assertThat(getCommitMessage(r.getChangeId()))
- .isEqualTo("modified commit\n\nChange-Id: " + r.getChangeId() + "\n");
- }
-
- @Test
public void changeCommitMessageNullNotAllowed() throws Exception {
PushOneCommit.Result r = createChange();
assertThat(getCommitMessage(r.getChangeId()))
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 82215b6..2976d78 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -470,7 +470,7 @@
gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
gApi.changes().id(changeId).edit().publish();
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
- gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+ gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
gApi.changes().id(changeId).edit().publish();
DiffInfo diffInfo =
@@ -492,7 +492,7 @@
gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
gApi.changes().id(changeId).edit().publish();
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
- gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+ gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
gApi.changes().id(changeId).edit().publish();
DiffInfo diffInfo =
@@ -510,7 +510,7 @@
gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
gApi.changes().id(changeId).edit().publish();
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
- gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+ gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
gApi.changes().id(changeId).edit().publish();
DiffInfo diffInfo =
@@ -2704,6 +2704,10 @@
assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
}
+ private String updatedCommitMessage() {
+ return "An unchanged patchset\n\nChange-Id: " + changeId;
+ }
+
private void assertDiffForNewFile(
PushOneCommit.Result pushResult, String path, String expectedContentSideB) throws Exception {
DiffInfo diff =
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index b855485..85a7b29 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -1046,7 +1046,8 @@
@Test
public void fixDoesNotModifyCommitMessageOfChangeEdit() throws Exception {
- String changeEditCommitMessage = "This is the commit message of the change edit.\n";
+ String changeEditCommitMessage =
+ "This is the commit message of the change edit.\n\nChange-Id: " + changeId + "\n";
gApi.changes().id(changeId).edit().modifyCommitMessage(changeEditCommitMessage);
fixReplacementInfo.path = FILE_NAME;
@@ -1065,7 +1066,8 @@
@Test
public void fixOnCommitMessageCanBeApplied() throws Exception {
// Set a dedicated commit message.
- String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n";
+ String footer = "\nChange-Id: " + changeId + "\n";
+ String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n" + footer;
gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
gApi.changes().id(changeId).edit().publish();
@@ -1080,13 +1082,15 @@
gApi.changes().id(changeId).current().applyFix(fixId);
String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
- assertThat(commitMessage).isEqualTo("Modified line\nLine 2 of commit message\n");
+ assertThat(commitMessage).isEqualTo("Modified line\nLine 2 of commit message\n" + footer);
}
@Test
public void fixOnHeaderPartOfCommitMessageCannotBeApplied() throws Exception {
// Set a dedicated commit message.
- String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n";
+ String footer = "Change-Id: " + changeId;
+ String originalCommitMessage =
+ "Line 1 of commit message\nLine 2 of commit message\n" + "\n" + footer + "\n";
gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
gApi.changes().id(changeId).edit().publish();
@@ -1108,8 +1112,9 @@
@Test
public void fixContainingSeveralModificationsOfCommitMessageCanBeApplied() throws Exception {
// Set a dedicated commit message.
+ String footer = "\nChange-Id: " + changeId + "\n";
String originalCommitMessage =
- "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n";
+ "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n" + footer;
gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
gApi.changes().id(changeId).edit().publish();
@@ -1135,13 +1140,14 @@
String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
assertThat(commitMessage)
- .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n");
+ .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n" + footer);
}
@Test
public void fixModifyingTheCommitMessageAndAFileCanBeApplied() throws Exception {
// Set a dedicated commit message.
- String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n";
+ String footer = "\nChange-Id: " + changeId + "\n";
+ String originalCommitMessage = "Line 1 of commit message\nLine 2 of commit message\n" + footer;
gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
gApi.changes().id(changeId).edit().publish();
@@ -1165,7 +1171,7 @@
gApi.changes().id(changeId).current().applyFix(fixId);
String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
- assertThat(commitMessage).isEqualTo("Modified line 1\nLine 2 of commit message\n");
+ assertThat(commitMessage).isEqualTo("Modified line 1\nLine 2 of commit message\n" + footer);
Optional<BinaryResult> file = gApi.changes().id(changeId).edit().getFile(FILE_NAME2);
BinaryResultSubject.assertThat(file)
.value()
@@ -1176,8 +1182,9 @@
@Test
public void twoFixesOnCommitMessageCanBeAppliedOneAfterTheOther() throws Exception {
// Set a dedicated commit message.
+ String footer = "\nChange-Id: " + changeId + "\n";
String originalCommitMessage =
- "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n";
+ "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n" + footer;
gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
gApi.changes().id(changeId).edit().publish();
@@ -1206,14 +1213,17 @@
String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
assertThat(commitMessage)
- .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n");
+ .isEqualTo("Modified line 1\nLine 2 of commit message\nModified line 3\n" + footer);
}
@Test
public void twoConflictingFixesOnCommitMessageCanNotBeAppliedOneAfterTheOther() throws Exception {
// Set a dedicated commit message.
+ String footer = "Change-Id: " + changeId;
String originalCommitMessage =
- "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n";
+ "Line 1 of commit message\nLine 2 of commit message\nLine 3 of commit message\n\n"
+ + footer
+ + "\n";
gApi.changes().id(changeId).edit().modifyCommitMessage(originalCommitMessage);
gApi.changes().id(changeId).edit().publish();
@@ -1379,8 +1389,10 @@
@Test
public void getFixPreviewForCommitMsg() throws Exception {
+ String footer = "Change-Id: " + changeId;
updateCommitMessage(
- changeId, "Commit title\n\nCommit message line 1\nLine 2\nLine 3\nLast line\n");
+ changeId,
+ "Commit title\n\nCommit message line 1\nLine 2\nLine 3\nLast line\n\n" + footer + "\n");
FixReplacementInfo commitMsgReplacement = new FixReplacementInfo();
commitMsgReplacement.path = Patch.COMMIT_MSG;
// The test assumes that the first 5 lines is a header.
@@ -1420,7 +1432,11 @@
.isEqualTo("Commit message line 1");
assertThat(diff).content().element(1).linesOfA().containsExactly("Line 2");
assertThat(diff).content().element(1).linesOfB().containsExactly("New content");
- assertThat(diff).content().element(2).commonLines().containsExactly("Line 3", "Last line", "");
+ assertThat(diff)
+ .content()
+ .element(2)
+ .commonLines()
+ .containsExactly("Line 3", "Last line", "", footer, "");
}
private void updateCommitMessage(String changeId, String newCommitMessage) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 5dbbe96..5808ea4 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -310,12 +310,14 @@
@Test
public void updateCommitMessageByEditingMagicCommitMsgFile() throws Exception {
createEmptyEditFor(changeId);
+ String updatedCommitMsg = "Foo Bar\n\nChange-Id: " + changeId + "\n";
gApi.changes()
.id(changeId)
.edit()
- .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create("Foo Bar".getBytes(UTF_8)));
+ .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create(updatedCommitMsg.getBytes(UTF_8)));
assertThat(getEdit(changeId)).isPresent();
- ensureSameBytes(getFileContentOfEdit(changeId, Patch.COMMIT_MSG), "Foo Bar\n".getBytes(UTF_8));
+ ensureSameBytes(
+ getFileContentOfEdit(changeId, Patch.COMMIT_MSG), updatedCommitMsg.getBytes(UTF_8));
}
@Test
@@ -346,6 +348,39 @@
}
@Test
+ public void updateMessageEditChangeIdShouldThrowResourceConflictException() throws Exception {
+ createEmptyEditFor(changeId);
+ String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () ->
+ gApi.changes()
+ .id(changeId)
+ .edit()
+ .modifyCommitMessage(commitMessage.replaceAll(changeId, changeId2)));
+ assertThat(thrown).hasMessageThat().isEqualTo("wrong Change-Id footer");
+ }
+
+ @Test
+ public void updateMessageEditRemoveChangeIdShouldThrowResourceConflictException()
+ throws Exception {
+ createEmptyEditFor(changeId);
+ String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () ->
+ gApi.changes()
+ .id(changeId)
+ .edit()
+ .modifyCommitMessage(commitMessage.replaceAll("(Change-Id:).*", "")));
+ assertThat(thrown).hasMessageThat().isEqualTo("missing Change-Id footer");
+ }
+
+ @Test
public void updateMessageNoChange() throws Exception {
createEmptyEditFor(changeId);
String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
