Allow comments created via the test API to have a specific creation time

For some tests, the exact creation time of a comment matters. In the
past, we used workarounds like TestTimeUtil to fix issues with timing
in tests. Tests using the test API can do better, though. As it's an
API specific for tests, we can support the specification of
creation/update times even though we wouldn't do so in public APIs.

We decided to not use Timestamp as type in the API as:
1) It's a mutable type which should not be used in an AutoValue.
2) It's much more inconvenient to specify and less readable/descriptive
than the types of Java 8's date/time API.

Change-Id: Ib026677ce3cfa6ddc248d40e8b429c298c8ccf88
This commit is contained in:
Alice Kober-Sotzek
2020-09-15 12:51:29 +02:00
parent c5a486c2d5
commit 163adfb3e7
3 changed files with 137 additions and 1 deletions

View File

@@ -186,11 +186,13 @@ public class PerPatchsetOperationsImpl implements PerPatchsetOperations {
short side = commentCreation.side().orElse(CommentSide.PATCHSET_COMMIT).getNumericSide();
Boolean unresolved = commentCreation.unresolved().orElse(null);
String parentUuid = commentCreation.parentUuid().orElse(null);
Timestamp createdOn =
commentCreation.createdOn().map(Timestamp::from).orElse(context.getWhen());
HumanComment newComment =
commentsUtil.newHumanComment(
context.getNotes(),
context.getUser(),
context.getWhen(),
createdOn,
filePath,
patchsetId,
side,

View File

@@ -21,6 +21,9 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Patch;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.util.Optional;
/**
@@ -49,6 +52,8 @@ public abstract class TestCommentCreation {
public abstract Optional<Account.Id> author();
public abstract Optional<Instant> createdOn();
abstract Comment.Status status();
abstract ThrowingFunction<TestCommentCreation, String> commentCreator();
@@ -174,6 +179,22 @@ public abstract class TestCommentCreation {
/** Author of the comment. Must be an existing user account. */
public abstract Builder author(Account.Id accountId);
/**
* Creation time of the comment. Like {@link #createdOn(Instant)} but with an arbitrary, fixed
* time zone (-> deterministic test execution).
*/
public Builder createdOn(LocalDateTime createdOn) {
// We don't care about the exact time zone in most tests, just that it's fixed so that tests
// are deterministic.
return createdOn(createdOn.atZone(ZoneOffset.UTC).toInstant());
}
/**
* Creation time of the comment. This may also lie in the past or future. Comments stored in
* NoteDb support only second precision.
*/
public abstract Builder createdOn(Instant createdOn);
/**
* Status of the comment. Hidden in the API surface. Use {@link
* PerPatchsetOperations#newComment()} or {@link PerPatchsetOperations#newDraftComment()}

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance.testsuite.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThatList;
import static com.google.gerrit.extensions.common.testing.RobotCommentInfoSubject.assertThat;
@@ -33,6 +34,12 @@ import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.Month;
import java.time.ZoneOffset;
import java.util.List;
import org.junit.Test;
@@ -314,6 +321,59 @@ public class PatchsetOperationsImplTest extends AbstractDaemonTest {
assertThat(comment).author().id().isEqualTo(accountId.get());
}
@Test
public void commentIsCreatedWithSpecifiedCreationTime() throws Exception {
Change.Id changeId = changeOperations.newChange().create();
// Don't use nanos. NoteDb supports only second precision.
Instant creationTime =
LocalDateTime.of(2020, Month.SEPTEMBER, 15, 12, 10, 43).atZone(ZoneOffset.UTC).toInstant();
String commentUuid =
changeOperations
.change(changeId)
.currentPatchset()
.newComment()
.createdOn(creationTime)
.create();
Timestamp creationTimestamp = Timestamp.from(creationTime);
CommentInfo comment = getCommentFromServer(changeId, commentUuid);
assertThat(comment).updated().isEqualTo(creationTimestamp);
}
@Test
public void zoneOfCreationDateCanBeOmitted() throws Exception {
Change.Id changeId = changeOperations.newChange().create();
// As we don't care about the exact time zone internally used as a default, do a relative test
// so that we don't need to assert on exact instants in time. For a relative test, we need two
// comments whose creation date should be exactly the specified amount apart.
// Don't use nanos or millis. NoteDb supports only second precision.
LocalDateTime creationTime1 = LocalDateTime.of(2020, Month.SEPTEMBER, 15, 12, 10, 43);
LocalDateTime creationTime2 = creationTime1.plusMinutes(10);
String commentUuid1 =
changeOperations
.change(changeId)
.currentPatchset()
.newComment()
.createdOn(creationTime1)
.create();
String commentUuid2 =
changeOperations
.change(changeId)
.currentPatchset()
.newComment()
.createdOn(creationTime2)
.create();
CommentInfo comment1 = getCommentFromServer(changeId, commentUuid1);
Instant comment1Creation = comment1.updated.toInstant();
CommentInfo comment2 = getCommentFromServer(changeId, commentUuid2);
Instant comment2Creation = comment2.updated.toInstant();
Duration commentCreationDifference = Duration.between(comment1Creation, comment2Creation);
assertThat(commentCreationDifference).isEqualTo(Duration.ofMinutes(10));
}
@Test
public void draftCommentCanBeCreatedWithoutSpecifyingAnyParameters() throws Exception {
Change.Id changeId = changeOperations.newChange().create();
@@ -625,6 +685,59 @@ public class PatchsetOperationsImplTest extends AbstractDaemonTest {
assertThatList(comments).comparingElementsUsing(hasUuid()).containsExactly(commentUuid);
}
@Test
public void draftCommentIsCreatedWithSpecifiedCreationTime() throws Exception {
Change.Id changeId = changeOperations.newChange().create();
// Don't use nanos. NoteDb supports only second precision.
Instant creationTime =
LocalDateTime.of(2020, Month.SEPTEMBER, 15, 12, 10, 43).atZone(ZoneOffset.UTC).toInstant();
String commentUuid =
changeOperations
.change(changeId)
.currentPatchset()
.newDraftComment()
.createdOn(creationTime)
.create();
Timestamp creationTimestamp = Timestamp.from(creationTime);
CommentInfo comment = getDraftCommentFromServer(changeId, commentUuid);
assertThat(comment).updated().isEqualTo(creationTimestamp);
}
@Test
public void zoneOfCreationDateOfDraftCommentCanBeOmitted() throws Exception {
Change.Id changeId = changeOperations.newChange().create();
// As we don't care about the exact time zone internally used as a default, do a relative test
// so that we don't need to assert on exact instants in time. For a relative test, we need two
// comments whose creation date should be exactly the specified amount apart.
// Don't use nanos or millis. NoteDb supports only second precision.
LocalDateTime creationTime1 = LocalDateTime.of(2020, Month.SEPTEMBER, 15, 12, 10, 43);
LocalDateTime creationTime2 = creationTime1.plusMinutes(10);
String commentUuid1 =
changeOperations
.change(changeId)
.currentPatchset()
.newDraftComment()
.createdOn(creationTime1)
.create();
String commentUuid2 =
changeOperations
.change(changeId)
.currentPatchset()
.newDraftComment()
.createdOn(creationTime2)
.create();
CommentInfo comment1 = getDraftCommentFromServer(changeId, commentUuid1);
Instant comment1Creation = comment1.updated.toInstant();
CommentInfo comment2 = getDraftCommentFromServer(changeId, commentUuid2);
Instant comment2Creation = comment2.updated.toInstant();
Duration commentCreationDifference = Duration.between(comment1Creation, comment2Creation);
assertThat(commentCreationDifference).isEqualTo(Duration.ofMinutes(10));
}
@Test
public void noDraftCommentsAreCreatedOnCreationOfPublishedComment() throws Exception {
Change.Id changeId = changeOperations.newChange().create();