Merge branch 'stable-3.1' into stable-3.2
* stable-3.1:
e2e-tests: Add a CreateBranch scenario
Adapt CreateChange and DeleteChange to perform load testing
Adapt CloneUsingBothProtocols to perform load testing
Set version to 3.1.13-SNAPSHOT
Set version to 3.0.17-SNAPSHOT
Set version to 2.15.23-SNAPSHOT
Set version to 2.16.27-SNAPSHOT
Set version to 3.1.12
Set version to 3.0.16
Set version to 2.16.26
Do not remove Change-Id footer in test
Use Change-Id footer for updating commit msg in RevisionDiffIT
Avoid magic values in RevisionDiffIT
Set version to 2.15.22
Update git submodules
Update git submodules
Move ensureChangeIdIsCorrect from PutMessage to ChangeUtil
Avoid creating HTTP Sessions for Git-over-HTTP
Avoid creating HTTP Sessions for Git-over-HTTP
Avoid creating HTTP Sessions for Git-over-HTTP
Avoid creating HTTP Sessions for Git-over-HTTP
e2e-tests: Init executable README for local setup testing
Update git submodules
Disallow editing the Change-Id during inline edits
Fix badly formatted error message shown in error dialog
ForRef#check should permit internal users to read all refs
Change-Id: I8f786bc8ad477005d06e3aa3c231822e37f0ee2c
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 a166d97..673695b6 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -19,17 +19,25 @@
import com.google.common.collect.Ordering;
import com.google.common.io.BaseEncoding;
+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.util.CommitMessageUtil;
import com.google.inject.Singleton;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.Collection;
+import java.util.List;
import java.util.Random;
import java.util.Set;
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;
@Singleton
public class ChangeUtil {
@@ -102,6 +110,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 48683ea..2f04cae 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.collect.ImmutableList;
+import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
@@ -210,7 +211,8 @@
* @throws PermissionBackendException
* @throws BadRequestException if the commit message is malformed
*/
- public void modifyMessage(Repository repository, ChangeNotes notes, String newCommitMessage)
+ public void modifyMessage(
+ Repository repository, Project.NameKey project, ChangeNotes notes, String newCommitMessage)
throws AuthException, IOException, UnchangedCommitMessageException,
PermissionBackendException, BadRequestException, ResourceConflictException {
assertCanEdit(notes);
@@ -232,6 +234,14 @@
ObjectId newEditCommit =
createCommit(repository, basePatchSetCommit, baseTree, newCommitMessage, nowTimestamp);
+ ChangeUtil.ensureChangeIdIsCorrect(
+ projectCache
+ .get(project)
+ .orElseThrow(illegalState(project))
+ .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+ notes.getChange().getKey().get(),
+ newCommitMessage);
+
if (optionalChangeEdit.isPresent()) {
updateEdit(
notes.getProjectName(),
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index 9a25f52..d510fce 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -481,7 +481,7 @@
Project.NameKey project = rsrc.getProject();
try (Repository repository = repositoryManager.openRepository(project)) {
- editModifier.modifyMessage(repository, rsrc.getNotes(), input.message);
+ editModifier.modifyMessage(repository, project, rsrc.getNotes(), input.message);
} catch (UnchangedCommitMessageException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 2e9d21a..1ed7fd7 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;
@@ -49,11 +48,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;
@@ -111,14 +108,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);
@@ -199,33 +195,4 @@
throw new AuthException("modifying commit message not permitted", denied);
}
}
-
- private static 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 = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
- 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 bf77b91..d25e6541f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4072,16 +4072,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 717d3cc..a53f298 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -429,7 +429,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 =
@@ -451,7 +451,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 =
@@ -469,7 +469,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 =
@@ -2737,6 +2737,10 @@
assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
}
+ private String updatedCommitMessage() {
+ return "An unchanged patchset\n\nChange-Id: " + changeId;
+ }
+
private static CommentInput createCommentInput(
int startLine, int startCharacter, int endLine, int endCharacter, String message) {
CommentInput comment = new CommentInput();
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 0b8f441..0c98d32 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -935,7 +935,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;
@@ -1090,8 +1091,13 @@
@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.
@@ -1131,7 +1137,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 f0bb201..d7cc338 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();
