Add annotations for tests to use clock step and set timezone

Many of the acceptance test rely for their assertions on unique
timestamps. This is achieved by setting a clock step. E.g. a clock step
of one second means that two subsequent timestamps differ by 1 second.

Test classes set the clock step in a setup method that is annotated with
@Before or @BeforeClass, and then they reset the clock to use system
time in a cleanup method that is annotated with @After or @AfterClass.

On method level setting a clock step must have a finally block to unset
it when the test is done.

This results in a lot of boilerplate code and is also error-prone (e.g.
some tests didn't reset the clock to use system time after setting a
clock step).

The new UseClockStep annotation makes this easier and less error-prone.
If this annotation is set (either on class or method level)
AbstractDaemonTest takes care to set and unset clock steps.

By default @UseClockStep uses a clock step of 1 second, which is what
most tests use, but the clock step and the clock step unit can be
specified explicitly. In addition it's possible to request setting the
clock initially to Instant.EPOCH which some tests require.

If a test class uses a clock step single methods in this class can
override this setting by using the UseSystemTime annotation.

In addition this change adds a UseTimezone annotation that allows to set
the timezone for the test execution. Here too using the annotation
relieves the tests from caring to reset the timezone after the test is
done.

All tests that set a clock step or a timezone are adapted to use the new
annotations.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iff0429cedadaea8538fd5ab96417241762f5a588

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -128,6 +128,7 @@ import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.FakeEmailSender;
 import com.google.gerrit.testing.FakeEmailSender.Message;
 import com.google.gerrit.testing.SshMode;
+import com.google.gerrit.testing.TestTimeUtil;
 import com.google.gson.Gson;
 import com.google.inject.Inject;
 import com.google.inject.Module;
@@ -143,6 +144,8 @@ import java.nio.file.FileSystem;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.sql.Timestamp;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -285,6 +288,7 @@ public abstract class AbstractDaemonTest {
 
   private ProjectResetter resetter;
   private List<Repository> toClose;
+  private String systemTimeZone;
 
   @Before
   public void clearSender() {
@@ -435,6 +439,37 @@ public abstract class AbstractDaemonTest {
     if (!classDesc.skipProjectClone()) {
       testRepo = cloneProject(project, getCloneAsAccount(description));
     }
+
+    // Set the clock step last, so that the test setup isn't consuming any timestamps after the
+    // clock has been set.
+    setTimeSettings(classDesc.useSystemTime(), classDesc.useClockStep(), classDesc.useTimezone());
+    setTimeSettings(
+        methodDesc.useSystemTime(), methodDesc.useClockStep(), methodDesc.useTimezone());
+  }
+
+  private void setTimeSettings(
+      boolean useSystemTime,
+      @Nullable UseClockStep useClockStep,
+      @Nullable UseTimezone useTimezone) {
+    if (useSystemTime) {
+      TestTimeUtil.useSystemTime();
+    } else if (useClockStep != null) {
+      TestTimeUtil.resetWithClockStep(useClockStep.clockStep(), useClockStep.clockStepUnit());
+      if (useClockStep.startAtEpoch()) {
+        TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
+      }
+    }
+    if (useTimezone != null) {
+      systemTimeZone = System.setProperty("user.timezone", useTimezone.timezone());
+    }
+  }
+
+  private void resetTimeSettings() {
+    TestTimeUtil.useSystemTime();
+    if (systemTimeZone != null) {
+      System.setProperty("user.timezone", systemTimeZone);
+      systemTimeZone = null;
+    }
   }
 
   /** Override to bind an additional Guice module */
@@ -562,6 +597,7 @@ public abstract class AbstractDaemonTest {
       repo.close();
     }
     closeSsh();
+    resetTimeSettings();
     if (server != commonServer) {
       server.close();
       server = null;

java/com/google/gerrit/acceptance/GerritServer.java

@@ -110,6 +110,9 @@ public class GerritServer implements AutoCloseable {
           has(Sandboxed.class, testDesc.getTestClass()),
           has(SkipProjectClone.class, testDesc.getTestClass()),
           has(UseSsh.class, testDesc.getTestClass()),
+          false, // @UseSystemTime is only valid on methods.
+          get(UseClockStep.class, testDesc.getTestClass()),
+          get(UseTimezone.class, testDesc.getTestClass()),
           null, // @GerritConfig is only valid on methods.
           null, // @GerritConfigs is only valid on methods.
           null, // @GlobalPluginConfig is only valid on methods.
@@ -119,6 +122,15 @@ public class GerritServer implements AutoCloseable {
 
     public static Description forTestMethod(
         org.junit.runner.Description testDesc, String configName) {
+      UseClockStep useClockStep = testDesc.getAnnotation(UseClockStep.class);
+      if (testDesc.getAnnotation(UseSystemTime.class) == null && useClockStep == null) {
+        // Only read the UseClockStep from the class if on method level neither @UseSystemTime nor
+        // @UseClockStep have been used.
+        // If the method defines @UseSystemTime or @UseClockStep it should overwrite @UseClockStep
+        // on class level.
+        useClockStep = get(UseClockStep.class, testDesc.getTestClass());
+      }
+
       return new AutoValue_GerritServer_Description(
           testDesc,
           configName,
@@ -133,6 +145,11 @@ public class GerritServer implements AutoCloseable {
               || has(SkipProjectClone.class, testDesc.getTestClass()),
           testDesc.getAnnotation(UseSsh.class) != null
               || has(UseSsh.class, testDesc.getTestClass()),
+          testDesc.getAnnotation(UseSystemTime.class) != null,
+          useClockStep,
+          testDesc.getAnnotation(UseTimezone.class) != null
+              ? testDesc.getAnnotation(UseTimezone.class)
+              : get(UseTimezone.class, testDesc.getTestClass()),
           testDesc.getAnnotation(GerritConfig.class),
           testDesc.getAnnotation(GerritConfigs.class),
           testDesc.getAnnotation(GlobalPluginConfig.class),
@@ -149,6 +166,16 @@ public class GerritServer implements AutoCloseable {
       return false;
     }
 
+    @Nullable
+    private static <T extends Annotation> T get(Class<T> annotation, Class<?> clazz) {
+      for (; clazz != null; clazz = clazz.getSuperclass()) {
+        if (clazz.getAnnotation(annotation) != null) {
+          return clazz.getAnnotation(annotation);
+        }
+      }
+      return null;
+    }
+
     private static Level getLogLevelThresholdAnnotation(org.junit.runner.Description testDesc) {
       LogThreshold logLevelThreshold = testDesc.getTestClass().getAnnotation(LogThreshold.class);
       if (logLevelThreshold == null) {
@@ -176,6 +203,14 @@ public class GerritServer implements AutoCloseable {
       return useSshAnnotation() && SshMode.useSsh();
     }
 
+    abstract boolean useSystemTime();
+
+    @Nullable
+    abstract UseClockStep useClockStep();
+
+    @Nullable
+    abstract UseTimezone useTimezone();
+
     @Nullable
     abstract GerritConfig config();
 
@@ -191,12 +226,15 @@ public class GerritServer implements AutoCloseable {
     abstract Level logLevelThreshold();
 
     private void checkValidAnnotations() {
+      if (useClockStep() != null && useSystemTime()) {
+        throw new IllegalStateException("Use either @UseClockStep or @UseSystemTime, not both");
+      }
       if (configs() != null && config() != null) {
-        throw new IllegalStateException("Use either @GerritConfigs or @GerritConfig not both");
+        throw new IllegalStateException("Use either @GerritConfigs or @GerritConfig, not both");
       }
       if (pluginConfigs() != null && pluginConfig() != null) {
         throw new IllegalStateException(
-            "Use either @GlobalPluginConfig or @GlobalPluginConfigs not both");
+            "Use either @GlobalPluginConfig or @GlobalPluginConfigs, not both");
       }
       if ((pluginConfigs() != null || pluginConfig() != null) && memory()) {
         throw new IllegalStateException("Must use @UseLocalDisk with @GlobalPluginConfig(s)");

java/com/google/gerrit/acceptance/UseClockStep.java

@@ -0,0 +1,42 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Annotation to use a clock step for the execution of acceptance tests (the test class must inherit
+ * from {@link AbstractDaemonTest}).
+ *
+ * <p>Annotations on method level override annotations on class level.
+ */
+@Target({TYPE, METHOD})
+@Retention(RUNTIME)
+public @interface UseClockStep {
+  /** Amount to increment clock by on each lookup. */
+  long clockStep() default 1L;
+
+  /** Time unit for {@link #clockStep()}. */
+  TimeUnit clockStepUnit() default TimeUnit.SECONDS;
+
+  /** Whether the clock should initially be set to {@link java.time.Instant#EPOCH}. */
+  boolean startAtEpoch() default false;
+}

java/com/google/gerrit/acceptance/UseSystemTime.java

@@ -0,0 +1,35 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation to use the system time for the execution of acceptance tests (the test class must
+ * inherit from {@link AbstractDaemonTest}).
+ *
+ * <p>Can only be applied on method level, since using system time on class level is the default if
+ * {@link UseClockStep} is not used.
+ *
+ * <p>Intended to be used to use system time for single tests when the test class is annotated with
+ * {@link UseClockStep}.
+ */
+@Target(METHOD)
+@Retention(RUNTIME)
+public @interface UseSystemTime {}

java/com/google/gerrit/acceptance/UseTimezone.java

@@ -0,0 +1,35 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation to set a timezone for the execution of acceptance tests (the test class must inherit
+ * from {@link AbstractDaemonTest}).
+ *
+ * <p>Annotations on method level override annotations on class level.
+ */
+@Target({TYPE, METHOD})
+@Retention(RUNTIME)
+public @interface UseTimezone {
+  /** The timezone that should be used for the test, e.g. "US/Eastern". */
+  String timezone();
+}

javatests/com/google/gerrit/acceptance/annotation/BUILD

@@ -4,4 +4,5 @@ acceptance_tests(
     srcs = glob(["*.java"]),
     group = "annotation",
     labels = ["annotation"],
+    deps = ["//java/com/google/gerrit/server/util/time"],
 )

javatests/com/google/gerrit/acceptance/annotation/UseClockStepTest.java

@@ -0,0 +1,57 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.annotation;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.server.util.time.TimeUtil;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
+import org.junit.Test;
+
+public class UseClockStepTest extends AbstractDaemonTest {
+  @Test
+  @UseClockStep
+  public void useClockStepWithDefaults() {
+    long firstTimestamp = TimeUtil.nowMs();
+    long secondTimestamp = TimeUtil.nowMs();
+    assertThat(secondTimestamp - firstTimestamp).isEqualTo(1000);
+  }
+
+  @Test
+  @UseClockStep(clockStepUnit = TimeUnit.MINUTES)
+  public void useClockStepWithTimeUnit() {
+    long firstTimestamp = TimeUtil.nowMs();
+    long secondTimestamp = TimeUtil.nowMs();
+    assertThat(secondTimestamp - firstTimestamp).isEqualTo(60 * 1000);
+  }
+
+  @Test
+  @UseClockStep(clockStep = 5)
+  public void useClockStepWithClockStep() {
+    long firstTimestamp = TimeUtil.nowMs();
+    long secondTimestamp = TimeUtil.nowMs();
+    assertThat(secondTimestamp - firstTimestamp).isEqualTo(5 * 1000);
+  }
+
+  @Test
+  @UseClockStep(startAtEpoch = true)
+  public void useClockStepWithStartAtEpoch() {
+    assertThat(TimeUtil.nowTs()).isEqualTo(Timestamp.from(Instant.EPOCH));
+  }
+}

javatests/com/google/gerrit/acceptance/annotation/UseSystemTimeTest.java

@@ -0,0 +1,34 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.annotation;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.UseSystemTime;
+import com.google.gerrit.server.util.time.TimeUtil;
+import org.junit.Test;
+
+@UseClockStep
+public class UseSystemTimeTest extends AbstractDaemonTest {
+  @Test
+  @UseSystemTime
+  public void useSystemTimeAlthoughClassIsAnnotatedWithUseClockStep() {
+    long firstTimestamp = TimeUtil.nowMs();
+    long secondTimestamp = TimeUtil.nowMs();
+    assertThat(secondTimestamp - firstTimestamp).isLessThan(1000);
+  }
+}

javatests/com/google/gerrit/acceptance/annotation/UseTimezoneTest.java

@@ -0,0 +1,35 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.annotation;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.UseTimezone;
+import org.junit.Test;
+
+public class UseTimezoneTest extends AbstractDaemonTest {
+  @Test
+  @UseTimezone(timezone = "US/Eastern")
+  public void usEastern() {
+    assertThat(System.getProperty("user.timezone")).isEqualTo("US/Eastern");
+  }
+
+  @Test
+  @UseTimezone(timezone = "UTC")
+  public void utc() {
+    assertThat(System.getProperty("user.timezone")).isEqualTo("UTC");
+  }
+}

javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -61,6 +61,7 @@ import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
@@ -143,7 +144,6 @@ import com.google.gerrit.server.validators.AccountActivationValidationListener;
 import com.google.gerrit.server.validators.ValidationException;
 import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.name.Named;
@@ -419,36 +419,32 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void createAtomically() throws Exception {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
+    Account.Id accountId = Account.id(seq.nextAccountId());
-    try {
+    String fullName = "Foo";
-      Account.Id accountId = Account.id(seq.nextAccountId());
+    ExternalId extId = ExternalId.createEmail(accountId, "foo@example.com");
-      String fullName = "Foo";
+    AccountState accountState =
-      ExternalId extId = ExternalId.createEmail(accountId, "foo@example.com");
+        accountsUpdateProvider
-      AccountState accountState =
+            .get()
-          accountsUpdateProvider
+            .insert(
-              .get()
+                "Create Account Atomically",
-              .insert(
+                accountId,
-                  "Create Account Atomically",
+                u -> u.setFullName(fullName).addExternalId(extId));
-                  accountId,
+    assertThat(accountState.getAccount().fullName()).isEqualTo(fullName);
-                  u -> u.setFullName(fullName).addExternalId(extId));
-      assertThat(accountState.getAccount().fullName()).isEqualTo(fullName);
 
-      AccountInfo info = gApi.accounts().id(accountId.get()).get();
+    AccountInfo info = gApi.accounts().id(accountId.get()).get();
-      assertThat(info.name).isEqualTo(fullName);
+    assertThat(info.name).isEqualTo(fullName);
 
-      List<EmailInfo> emails = gApi.accounts().id(accountId.get()).getEmails();
+    List<EmailInfo> emails = gApi.accounts().id(accountId.get()).getEmails();
-      assertThat(emails.stream().map(e -> e.email).collect(toSet())).containsExactly(extId.email());
+    assertThat(emails.stream().map(e -> e.email).collect(toSet())).containsExactly(extId.email());
 
-      RevCommit commitUserBranch =
+    RevCommit commitUserBranch =
-          projectOperations.project(allUsers).getHead(RefNames.refsUsers(accountId));
+        projectOperations.project(allUsers).getHead(RefNames.refsUsers(accountId));
-      RevCommit commitRefsMetaExternalIds =
+    RevCommit commitRefsMetaExternalIds =
-          projectOperations.project(allUsers).getHead(RefNames.REFS_EXTERNAL_IDS);
+        projectOperations.project(allUsers).getHead(RefNames.REFS_EXTERNAL_IDS);
-      assertThat(commitUserBranch.getCommitTime())
+    assertThat(commitUserBranch.getCommitTime())
-          .isEqualTo(commitRefsMetaExternalIds.getCommitTime());
+        .isEqualTo(commitRefsMetaExternalIds.getCommitTime());
-    } finally {
-      TestTimeUtil.useSystemTime();
-    }
   }
 
   @Test
@@ -2906,9 +2902,9 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void deleteAllDraftComments() throws Exception {
     try {
-      TestTimeUtil.resetWithClockStep(1, SECONDS);
       Project.NameKey project2 = projectOperations.newProject().create();
       PushOneCommit.Result r1 = createChange();
 

javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java

@@ -18,12 +18,12 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.RawInputUtil;
@@ -51,15 +51,13 @@ import com.google.gerrit.server.git.meta.MetaDataUpdate;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.project.ProjectConfig;
 import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.util.List;
 import org.eclipse.jgit.lib.Config;
-import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
+@UseClockStep
 public class AgreementsIT extends AbstractDaemonTest {
   private ContributorAgreement caAutoVerify;
   private ContributorAgreement caNoAutoVerify;
@@ -112,16 +110,6 @@ public class AgreementsIT extends AbstractDaemonTest {
     return cfg;
   }
 
-  @BeforeClass
-  public static void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @AfterClass
-  public static void restoreTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @Before
   public void setUp() throws Exception {
     caAutoVerify = configureContributorAgreement(true);

javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java

@@ -20,7 +20,6 @@ import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static java.util.concurrent.TimeUnit.HOURS;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.collect.ImmutableList;
@@ -28,6 +27,7 @@ import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.data.Permission;
@@ -113,10 +113,9 @@ public class AbandonIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   @GerritConfig(name = "changeCleanup.abandonAfter", value = "1w")
   public void abandonInactiveOpenChanges() throws Exception {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-
     // create 2 changes which will be abandoned ...
     int id1 = createChange().getChange().getId().get();
     int id2 = createChange().getChange().getId().get();

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -58,7 +58,6 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static com.google.gerrit.truth.CacheStatsSubject.assertThat;
 import static com.google.gerrit.truth.CacheStatsSubject.cloneStats;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
@@ -78,6 +77,8 @@ import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.UseTimezone;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -182,7 +183,6 @@ import com.google.gerrit.server.update.BatchUpdateOp;
 import com.google.gerrit.server.update.ChangeContext;
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
 import com.google.inject.name.Named;
@@ -213,8 +213,8 @@ import org.junit.Before;
 import org.junit.Test;
 
 @NoHttpd
+@UseTimezone(timezone = "US/Eastern")
 public class ChangeIT extends AbstractDaemonTest {
-  private String systemTimeZone;
 
   @Inject private AccountOperations accountOperations;
   @Inject private ChangeIndexCollection changeIndexCollection;
@@ -241,17 +241,6 @@ public class ChangeIT extends AbstractDaemonTest {
   private ChangeIndexedCounter changeIndexedCounter;
   private RegistrationHandle changeIndexedCounterHandle;
 
-  @Before
-  public void setTimeForTesting() {
-    systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
-  }
-
-  @After
-  public void resetTime() {
-    TestTimeUtil.useSystemTime();
-    System.setProperty("user.timezone", systemTimeZone);
-  }
-
   @Before
   public void addChangeIndexedCounter() {
     changeIndexedCounter = new ChangeIndexedCounter();
@@ -1896,6 +1885,7 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void addReviewer() throws Exception {
     testAddReviewerViaPostReview(
         (changeId, reviewer) -> {
@@ -1906,6 +1896,7 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void addReviewerViaPostReview() throws Exception {
     testAddReviewerViaPostReview(
         (changeId, reviewer) -> {
@@ -1918,7 +1909,6 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   private void testAddReviewerViaPostReview(AddReviewerCaller addReviewer) throws Exception {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
     PushOneCommit.Result r = createChange();
     ChangeResource rsrc = parseResource(r);
     String oldETag = rsrc.getETag();
@@ -2028,8 +2018,8 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void addReviewerThatIsNotPerfectMatch() throws Exception {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
     PushOneCommit.Result r = createChange();
     ChangeResource rsrc = parseResource(r);
     String oldETag = rsrc.getETag();
@@ -2077,8 +2067,8 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void addGroupAsReviewersWhenANotPerfectMatchedUserExists() throws Exception {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
     PushOneCommit.Result r = createChange();
     ChangeResource rsrc = parseResource(r);
     String oldETag = rsrc.getETag();
@@ -2138,8 +2128,8 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  @UseClockStep
   public void addSelfAsReviewer() throws Exception {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
     PushOneCommit.Result r = createChange();
     ChangeResource rsrc = parseResource(r);
     String oldETag = rsrc.getETag();

javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -45,6 +45,7 @@ import com.google.gerrit.acceptance.ProjectResetter;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -93,7 +94,6 @@ import com.google.gerrit.server.notedb.Sequences;
 import com.google.gerrit.server.util.MagicBranch;
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.GerritJUnit.ThrowingRunnable;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.lang.annotation.Retention;
@@ -104,7 +104,6 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
@@ -121,10 +120,10 @@ import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.PushResult;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 @NoHttpd
+@UseClockStep
 public class GroupsIT extends AbstractDaemonTest {
   @Inject @ServerInitiated private GroupsUpdate groupsUpdate;
   @Inject private AccountOperations accountOperations;
@@ -140,16 +139,6 @@ public class GroupsIT extends AbstractDaemonTest {
   @Inject private Sequences seq;
   @Inject private StalenessChecker stalenessChecker;
 
-  @Before
-  public void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
-  }
-
-  @After
-  public void resetTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @After
   public void consistencyCheck() throws Exception {
     if (description.getAnnotation(IgnoreGroupInconsistencies.class) == null) {

javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -27,7 +27,6 @@ import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.collect.ImmutableList;
@@ -37,6 +36,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.RawInputUtil;
@@ -65,7 +65,6 @@ import com.google.gerrit.server.project.testing.TestLabels;
 import com.google.gerrit.server.restapi.change.ChangeEdits.EditMessage;
 import com.google.gerrit.server.restapi.change.ChangeEdits.Post;
 import com.google.gerrit.server.restapi.change.ChangeEdits.Put;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
 import com.google.inject.Inject;
@@ -81,11 +80,10 @@ import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
+@UseClockStep
 public class ChangeEditIT extends AbstractDaemonTest {
 
   private static final String FILE_NAME = "foo";
@@ -103,16 +101,6 @@ public class ChangeEditIT extends AbstractDaemonTest {
   private String changeId2;
   private PatchSet ps;
 
-  @BeforeClass
-  public static void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @AfterClass
-  public static void restoreTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @Before
   public void setUp() throws Exception {
     changeId = newChange(admin.newIdent());

javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -41,7 +41,6 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS
 import static com.google.gerrit.server.project.testing.TestLabels.label;
 import static com.google.gerrit.server.project.testing.TestLabels.value;
 import static java.util.Comparator.comparing;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 
@@ -58,6 +57,7 @@ import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.SkipProjectClone;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.data.GlobalCapability;
@@ -105,7 +105,6 @@ import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.project.testing.TestLabels;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -134,12 +133,11 @@ import org.eclipse.jgit.transport.PushResult;
 import org.eclipse.jgit.transport.RefSpec;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.junit.After;
-import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 @SkipProjectClone
+@UseClockStep
 public abstract class AbstractPushForReview extends AbstractDaemonTest {
   protected enum Protocol {
     // Only test protocols which are actually served by the Gerrit server, since each separate test
@@ -160,16 +158,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
 
   @Inject private DynamicSet<CommitValidationListener> commitValidators;
 
-  @BeforeClass
-  public static void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @AfterClass
-  public static void restoreTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @Before
   public void setUpPatchSetLock() throws Exception {
     try (ProjectConfigUpdate u = updateProject(project)) {

javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java

@@ -16,16 +16,15 @@ package com.google.gerrit.acceptance.git;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
@@ -480,127 +479,107 @@ public class SubmoduleSubscriptionsIT extends AbstractSubmoduleSubscription {
   }
 
   @Test
+  @UseClockStep
   public void superRepoCommitHasSameAuthorAsSubmoduleCommit() throws Exception {
     // Make sure that the commit is created at an earlier timestamp than the submit timestamp.
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
+    allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
-    try {
+    createSubmoduleSubscription(superRepo, "master", subKey, "master");
-      allowMatchingSubmoduleSubscription(
-          subKey, "refs/heads/master", superKey, "refs/heads/master");
-      createSubmoduleSubscription(superRepo, "master", subKey, "master");
 
-      PushOneCommit.Result pushResult =
+    PushOneCommit.Result pushResult =
-          createChange(subRepo, "refs/heads/master", "Change", "a.txt", "some content", null);
+        createChange(subRepo, "refs/heads/master", "Change", "a.txt", "some content", null);
-      approve(pushResult.getChangeId());
+    approve(pushResult.getChangeId());
-      gApi.changes().id(pushResult.getChangeId()).current().submit();
+    gApi.changes().id(pushResult.getChangeId()).current().submit();
 
-      // Expect that the author name/email is preserved for the superRepo commit, but a new author
+    // Expect that the author name/email is preserved for the superRepo commit, but a new author
-      // timestamp is used.
+    // timestamp is used.
-      PersonIdent authorIdent = getAuthor(superRepo, "master");
+    PersonIdent authorIdent = getAuthor(superRepo, "master");
-      assertThat(authorIdent.getName()).isEqualTo(admin.fullName());
+    assertThat(authorIdent.getName()).isEqualTo(admin.fullName());
-      assertThat(authorIdent.getEmailAddress()).isEqualTo(admin.email());
+    assertThat(authorIdent.getEmailAddress()).isEqualTo(admin.email());
-      assertThat(authorIdent.getWhen())
+    assertThat(authorIdent.getWhen())
-          .isGreaterThan(pushResult.getCommit().getAuthorIdent().getWhen());
+        .isGreaterThan(pushResult.getCommit().getAuthorIdent().getWhen());
-    } finally {
-      TestTimeUtil.useSystemTime();
-    }
   }
 
   @Test
+  @UseClockStep
   public void superRepoCommitHasSameAuthorAsSubmoduleCommits() throws Exception {
     assume().that(isSubmitWholeTopicEnabled()).isTrue();
 
-    // Make sure that the commits are created at different timestamps and that the submit timestamp
+    Project.NameKey proj2 = createProjectForPush(getSubmitType());
-    // is afterwards.
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-    try {
 
-      Project.NameKey proj2 = createProjectForPush(getSubmitType());
+    TestRepository<?> subRepo2 = cloneProject(proj2);
+    allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
+    allowMatchingSubmoduleSubscription(proj2, "refs/heads/master", superKey, "refs/heads/master");
 
-      TestRepository<?> subRepo2 = cloneProject(proj2);
+    Config config = new Config();
-      allowMatchingSubmoduleSubscription(
+    prepareSubmoduleConfigEntry(config, subKey, subKey, "master");
-          subKey, "refs/heads/master", superKey, "refs/heads/master");
+    prepareSubmoduleConfigEntry(config, proj2, proj2, "master");
-      allowMatchingSubmoduleSubscription(proj2, "refs/heads/master", superKey, "refs/heads/master");
+    pushSubmoduleConfig(superRepo, "master", config);
 
-      Config config = new Config();
+    String topic = "foo";
-      prepareSubmoduleConfigEntry(config, subKey, subKey, "master");
-      prepareSubmoduleConfigEntry(config, proj2, proj2, "master");
-      pushSubmoduleConfig(superRepo, "master", config);
 
-      String topic = "foo";
+    PushOneCommit.Result pushResult1 =
+        createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic);
+    approve(pushResult1.getChangeId());
 
-      PushOneCommit.Result pushResult1 =
+    PushOneCommit.Result pushResult2 =
-          createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic);
+        createChange(subRepo2, "refs/heads/master", "Change 2", "b.txt", "other content", topic);
-      approve(pushResult1.getChangeId());
+    approve(pushResult2.getChangeId());
 
-      PushOneCommit.Result pushResult2 =
+    // Submit the topic, 2 changes with the same author.
-          createChange(subRepo2, "refs/heads/master", "Change 2", "b.txt", "other content", topic);
+    gApi.changes().id(pushResult1.getChangeId()).current().submit();
-      approve(pushResult2.getChangeId());
 
-      // Submit the topic, 2 changes with the same author.
+    // Expect that the author name/email is preserved for the superRepo commit, but a new author
-      gApi.changes().id(pushResult1.getChangeId()).current().submit();
+    // timestamp is used.
-
+    PersonIdent authorIdent = getAuthor(superRepo, "master");
-      // Expect that the author name/email is preserved for the superRepo commit, but a new author
+    assertThat(authorIdent.getName()).isEqualTo(admin.fullName());
-      // timestamp is used.
+    assertThat(authorIdent.getEmailAddress()).isEqualTo(admin.email());
-      PersonIdent authorIdent = getAuthor(superRepo, "master");
+    assertThat(authorIdent.getWhen())
-      assertThat(authorIdent.getName()).isEqualTo(admin.fullName());
+        .isGreaterThan(pushResult1.getCommit().getAuthorIdent().getWhen());
-      assertThat(authorIdent.getEmailAddress()).isEqualTo(admin.email());
+    assertThat(authorIdent.getWhen())
-      assertThat(authorIdent.getWhen())
+        .isGreaterThan(pushResult2.getCommit().getAuthorIdent().getWhen());
-          .isGreaterThan(pushResult1.getCommit().getAuthorIdent().getWhen());
-      assertThat(authorIdent.getWhen())
-          .isGreaterThan(pushResult2.getCommit().getAuthorIdent().getWhen());
-    } finally {
-      TestTimeUtil.useSystemTime();
-    }
   }
 
   @Test
+  @UseClockStep
   public void superRepoCommitHasGerritAsAuthorIfAuthorsOfSubmoduleCommitsDiffer() throws Exception {
     assume().that(isSubmitWholeTopicEnabled()).isTrue();
 
-    // Make sure that the commits are created at different timestamps and that the submit timestamp
+    Project.NameKey proj2 = createProjectForPush(getSubmitType());
-    // is afterwards.
+    TestRepository<InMemoryRepository> repo2 = cloneProject(proj2, user);
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-    try {
-      Project.NameKey proj2 = createProjectForPush(getSubmitType());
-      TestRepository<InMemoryRepository> repo2 = cloneProject(proj2, user);
 
-      allowMatchingSubmoduleSubscription(
+    allowMatchingSubmoduleSubscription(subKey, "refs/heads/master", superKey, "refs/heads/master");
-          subKey, "refs/heads/master", superKey, "refs/heads/master");
+    allowMatchingSubmoduleSubscription(proj2, "refs/heads/master", superKey, "refs/heads/master");
-      allowMatchingSubmoduleSubscription(proj2, "refs/heads/master", superKey, "refs/heads/master");
 
-      Config config = new Config();
+    Config config = new Config();
-      prepareSubmoduleConfigEntry(config, subKey, subKey, "master");
+    prepareSubmoduleConfigEntry(config, subKey, subKey, "master");
-      prepareSubmoduleConfigEntry(config, proj2, proj2, "master");
+    prepareSubmoduleConfigEntry(config, proj2, proj2, "master");
-      pushSubmoduleConfig(superRepo, "master", config);
+    pushSubmoduleConfig(superRepo, "master", config);
 
-      String topic = "foo";
+    String topic = "foo";
 
-      // Create change as admin.
+    // Create change as admin.
-      PushOneCommit.Result pushResult1 =
+    PushOneCommit.Result pushResult1 =
-          createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic);
+        createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic);
-      approve(pushResult1.getChangeId());
+    approve(pushResult1.getChangeId());
 
-      // Create change as user.
+    // Create change as user.
-      PushOneCommit push =
+    PushOneCommit push =
-          pushFactory.create(user.newIdent(), repo2, "Change 2", "b.txt", "other content");
+        pushFactory.create(user.newIdent(), repo2, "Change 2", "b.txt", "other content");
-      PushOneCommit.Result pushResult2 = push.to("refs/for/master/" + name(topic));
+    PushOneCommit.Result pushResult2 = push.to("refs/for/master/" + name(topic));
-      approve(pushResult2.getChangeId());
+    approve(pushResult2.getChangeId());
 
-      // Submit the topic, 2 changes with the different author.
+    // Submit the topic, 2 changes with the different author.
-      gApi.changes().id(pushResult1.getChangeId()).current().submit();
+    gApi.changes().id(pushResult1.getChangeId()).current().submit();
 
-      // Expect that the Gerrit server identity is chosen as author for the superRepo commit and a
+    // Expect that the Gerrit server identity is chosen as author for the superRepo commit and a
-      // new author timestamp is used.
+    // new author timestamp is used.
-      PersonIdent authorIdent = getAuthor(superRepo, "master");
+    PersonIdent authorIdent = getAuthor(superRepo, "master");
-      assertThat(authorIdent.getName()).isEqualTo(serverIdent.get().getName());
+    assertThat(authorIdent.getName()).isEqualTo(serverIdent.get().getName());
-      assertThat(authorIdent.getEmailAddress()).isEqualTo(serverIdent.get().getEmailAddress());
+    assertThat(authorIdent.getEmailAddress()).isEqualTo(serverIdent.get().getEmailAddress());
-      assertThat(authorIdent.getWhen())
+    assertThat(authorIdent.getWhen())
-          .isGreaterThan(pushResult1.getCommit().getAuthorIdent().getWhen());
+        .isGreaterThan(pushResult1.getCommit().getAuthorIdent().getWhen());
-      assertThat(authorIdent.getWhen())
+    assertThat(authorIdent.getWhen())
-          .isGreaterThan(pushResult2.getCommit().getAuthorIdent().getWhen());
+        .isGreaterThan(pushResult2.getCommit().getAuthorIdent().getWhen());
-    } finally {
-      TestTimeUtil.useSystemTime();
-    }
   }
 
   @Test

javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java

@@ -29,7 +29,6 @@ import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static java.util.Objects.requireNonNull;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.lib.Constants.EMPTY_TREE_ID;
 import static org.mockito.Mockito.atLeast;
@@ -49,6 +48,8 @@ import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.UseTimezone;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.Nullable;
@@ -95,7 +96,6 @@ import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.server.validators.ValidationException;
 import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.GerritJUnit.ThrowingRunnable;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -119,10 +119,11 @@ import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.transport.RefSpec;
 import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 @NoHttpd
+@UseClockStep
+@UseTimezone(timezone = "US/Eastern")
 public abstract class AbstractSubmit extends AbstractDaemonTest {
   @ConfigSuite.Config
   public static Config submitWholeTopicEnabled() {
@@ -138,19 +139,6 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
   @Inject private Submit submitHandler;
 
   private RegistrationHandle onSubmitValidatorHandle;
-  private String systemTimeZone;
-
-  @Before
-  public void setTimeForTesting() {
-    systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @After
-  public void resetTime() {
-    TestTimeUtil.useSystemTime();
-    System.setProperty("user.timezone", systemTimeZone);
-  }
 
   @After
   public void removeOnSubmitValidator() {

javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java

@@ -19,12 +19,12 @@ import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.a
 import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.data.Permission;
@@ -35,30 +35,18 @@ import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.account.AccountResolver.UnresolvableAccountException;
 import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.util.Iterator;
 import java.util.List;
 import org.eclipse.jgit.transport.RefSpec;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 @NoHttpd
+@UseClockStep
 public class AssigneeIT extends AbstractDaemonTest {
   @Inject private ProjectOperations projectOperations;
   @Inject private RequestScopeOperations requestScopeOperations;
 
-  @BeforeClass
-  public static void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @AfterClass
-  public static void restoreTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @Test
   public void getNoAssignee() throws Exception {
     PushOneCommit.Result r = createChange();

javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java

@@ -23,7 +23,6 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.parseCommitMessageRange;
 import static com.google.gerrit.server.restapi.change.DeleteChangeMessage.createNewChangeMessage;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toSet;
 import static org.eclipse.jgit.util.RawParseUtils.decode;
 
@@ -32,6 +31,8 @@ import com.google.common.collect.Lists;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.UseTimezone;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.data.GlobalCapability;
@@ -45,7 +46,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
@@ -54,30 +54,16 @@ import java.util.List;
 import java.util.Optional;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.util.RawParseUtils;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+@UseClockStep
+@UseTimezone(timezone = "US/Eastern")
 @RunWith(ConfigSuite.class)
 public class ChangeMessagesIT extends AbstractDaemonTest {
   @Inject private ProjectOperations projectOperations;
   @Inject private RequestScopeOperations requestScopeOperations;
 
-  private String systemTimeZone;
-
-  @Before
-  public void setTimeForTesting() {
-    systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @After
-  public void resetTime() {
-    TestTimeUtil.useSystemTime();
-    System.setProperty("user.timezone", systemTimeZone);
-  }
-
   @Test
   public void messagesNotReturnedByDefault() throws Exception {
     String changeId = createChange().getChangeId();

javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

@@ -20,7 +20,6 @@ import static com.google.gerrit.common.data.Permission.READ;
 import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG;
 
 import com.google.common.base.Strings;
@@ -29,6 +28,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.PushOneCommit.Result;
 import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.extensions.api.changes.ChangeApi;
@@ -51,7 +51,6 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.submit.ChangeAlreadyMergedException;
 import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.util.List;
 import java.util.Map;
@@ -61,24 +60,13 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.RefSpec;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
+@UseClockStep
 public class CreateChangeIT extends AbstractDaemonTest {
   @Inject private ProjectOperations projectOperations;
   @Inject private RequestScopeOperations requestScopeOperations;
 
-  @BeforeClass
-  public static void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @AfterClass
-  public static void restoreTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @Test
   public void createEmptyChange_MissingBranch() throws Exception {
     ChangeInput ci = new ChangeInput();

javatests/com/google/gerrit/acceptance/rest/change/HashtagsIT.java

@@ -20,7 +20,6 @@ import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.a
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static java.util.Objects.requireNonNull;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
@@ -28,6 +27,7 @@ import com.google.common.truth.IterableSubject;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.data.Permission;
@@ -35,26 +35,14 @@ import com.google.gerrit.extensions.api.changes.HashtagsInput;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 @NoHttpd
+@UseClockStep
 public class HashtagsIT extends AbstractDaemonTest {
   @Inject private ProjectOperations projectOperations;
 
-  @BeforeClass
-  public static void setTimeForTesting() {
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @AfterClass
-  public static void restoreTime() {
-    TestTimeUtil.useSystemTime();
-  }
-
   @Inject private RequestScopeOperations requestScopeOperations;
 
   @Test

javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java

@@ -20,7 +20,6 @@ import static com.google.gerrit.acceptance.GitUtil.assertPushOk;
 import static com.google.gerrit.acceptance.GitUtil.pushHead;
 import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
 import static com.google.gerrit.extensions.common.testing.EditInfoSubject.assertThat;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
@@ -29,6 +28,8 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.UseTimezone;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -52,7 +53,6 @@ import com.google.gerrit.server.update.BatchUpdateOp;
 import com.google.gerrit.server.update.ChangeContext;
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.ConfigSuite;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -64,11 +64,11 @@ import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 @NoHttpd
+@UseClockStep
+@UseTimezone(timezone = "US/Eastern")
 public class GetRelatedIT extends AbstractDaemonTest {
   private static final int MAX_TERMS = 10;
 
@@ -84,20 +84,6 @@ public class GetRelatedIT extends AbstractDaemonTest {
   @Inject private ProjectOperations projectOperations;
   @Inject private RequestScopeOperations requestScopeOperations;
 
-  private String systemTimeZone;
-
-  @Before
-  public void setTimeForTesting() {
-    systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @After
-  public void resetTime() {
-    TestTimeUtil.useSystemTime();
-    System.setProperty("user.timezone", systemTimeZone);
-  }
-
   @Inject private IndexConfig indexConfig;
   @Inject private ChangesCollection changes;
 

javatests/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java

@@ -15,11 +15,12 @@
 package com.google.gerrit.acceptance.server.mail;
 
 import static com.google.common.truth.Truth.assertThat;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.acceptance.UseTimezone;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
@@ -27,7 +28,6 @@ import com.google.gerrit.mail.EmailHeader;
 import com.google.gerrit.mail.MailProcessingUtil;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.testing.FakeEmailSender;
-import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.sql.Timestamp;
 import java.time.ZoneId;
@@ -37,28 +37,14 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 /** Tests the presence of required metadata in email headers, text and html. */
+@UseClockStep
+@UseTimezone(timezone = "US/Eastern")
 public class MailMetadataIT extends AbstractDaemonTest {
   @Inject private RequestScopeOperations requestScopeOperations;
 
-  private String systemTimeZone;
-
-  @Before
-  public void setTimeForTesting() {
-    systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
-    TestTimeUtil.resetWithClockStep(1, SECONDS);
-  }
-
-  @After
-  public void resetTime() {
-    TestTimeUtil.useSystemTime();
-    System.setProperty("user.timezone", systemTimeZone);
-  }
-
   @Test
   public void metadataOnNewChange() throws Exception {
     PushOneCommit.Result newChange = createChange();