Use TestProtocol to clone in-process for AbstractDaemonTest

This new JGit feature associates a particular request object with a
URL and processes all git protocol requests in-memory in a separate
thread. For acceptance tests, this request object also describes a
request scope context, such that each connection runs in its own
connection thread with its own Guice request context.

The TestProtocol instance uses custom Upload/ReceivePackFactory
implementations that parallel the implementations within the HTTP and
SSH code paths. On the one hand, this is yet another implementation
that must be modified when changing the git filter stack, but on the
other hand, there were already multiple stacks to begin with. In this
case, I think the performance improvement is also worth the cost.

With this change, single acceptance test methods can run in about
100ms even on my Macbook Air. Startup cost is still relatively high
due to server initialization, but this change opens the door to
reducing that well by removing HTTP and SSH servers from tests
entirely.

Change-Id: Icddd7d2ac448ea3f59909f8ef61f95feb1d67b64

gerrit-acceptance-tests/BUCK

@@ -35,6 +35,7 @@ java_library(
     '//lib/log:log4j',
     '//lib/guice:guice',
     '//lib/guice:guice-assistedinject',
+    '//lib/guice:guice-servlet',
     '//lib/jgit:jgit',
     '//lib/jgit:junit',
     '//lib/mina:sshd',

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.acceptance;
 
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
 import static com.google.gerrit.acceptance.GitUtil.initSsh;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.Util.block;
@@ -66,6 +65,8 @@ import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.Transport;
 import org.junit.AfterClass;
 import org.junit.Rule;
 import org.junit.rules.TestRule;
@@ -74,6 +75,7 @@ import org.junit.runner.RunWith;
 import org.junit.runners.model.Statement;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -132,6 +134,9 @@ public abstract class AbstractDaemonTest {
   @Inject
   protected @GerritServerConfig Config cfg;
 
+  @Inject
+  private InProcessProtocol inProcessProtocol;
+
   protected TestRepository<InMemoryRepository> testRepo;
   protected GerritServer server;
   protected TestAccount admin;
@@ -143,6 +148,7 @@ public abstract class AbstractDaemonTest {
   protected Project.NameKey project;
 
   private String resourcePrefix;
+  private List<Repository> toClose;
 
   @Rule
   public TestRule testRunner = new TestRule() {
@@ -207,6 +213,8 @@ public abstract class AbstractDaemonTest {
     }
 
     server.getTestInjector().injectMembers(this);
+    Transport.register(inProcessProtocol);
+    toClose = Collections.synchronizedList(new ArrayList<Repository>());
     admin = accounts.admin();
     user = accounts.user();
     adminSession = new RestSession(server, admin);
@@ -222,7 +230,12 @@ public abstract class AbstractDaemonTest {
         + description.getMethodName() + "_").replaceAll("");
 
     project = createProject(projectInput(description));
-    testRepo = cloneProject(project, sshSession);
+    testRepo = cloneProject(project, getCloneAsAccount(description));
+  }
+
+  private TestAccount getCloneAsAccount(Description description) {
+    TestProjectInput ann = description.getAnnotation(TestProjectInput.class);
+    return accounts.get(ann != null ? ann.cloneAs() : "admin");
   }
 
   private ProjectInput projectInput(Description description) {
@@ -306,7 +319,26 @@ public abstract class AbstractDaemonTest {
     // Default implementation does nothing.
   }
 
+  protected TestRepository<InMemoryRepository> cloneProject(Project.NameKey p)
+      throws Exception {
+    return cloneProject(p, admin);
+  }
+
+  protected TestRepository<InMemoryRepository> cloneProject(Project.NameKey p,
+      TestAccount testAccount) throws Exception {
+    InProcessProtocol.Context ctx = new InProcessProtocol.Context(
+        reviewDbProvider, identifiedUserFactory, testAccount.getId(), p);
+    Repository repo = repoManager.openRepository(p);
+    toClose.add(repo);
+    return GitUtil.cloneProject(
+        p, inProcessProtocol.register(ctx, repo).toString());
+  }
+
   private void afterTest() throws Exception {
+    Transport.unregister(inProcessProtocol);
+    for (Repository repo : toClose) {
+      repo.close();
+    }
     db.close();
     sshSession.close();
     if (server != commonServer) {
@@ -334,7 +366,9 @@ public abstract class AbstractDaemonTest {
 
   protected PushOneCommit.Result createChange() throws Exception {
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
-    return push.to("refs/for/master");
+    PushOneCommit.Result result = push.to("refs/for/master");
+    result.assertOkStatus();
+    return result;
   }
 
   private static final List<Character> RANDOM =

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AccountCreator.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.acceptance;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountExternalId;
@@ -150,6 +152,12 @@ public class AccountCreator {
     return create("user2", "user2@example.com", "User2");
   }
 
+  public TestAccount get(String username) {
+    return checkNotNull(
+        accounts.get(username),
+        "No TestAccount created for %s", username);
+  }
+
   private AccountExternalId.Key getEmailKey(String email) {
     return new AccountExternalId.Key(AccountExternalId.SCHEME_MAILTO, email);
   }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java

@@ -23,7 +23,10 @@ import com.google.gerrit.pgm.Daemon;
 import com.google.gerrit.pgm.Init;
 import com.google.gerrit.server.config.FactoryModule;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.AsyncReceiveCommits;
+import com.google.gerrit.server.git.SubmoduleOp;
 import com.google.gerrit.server.index.ChangeSchemas;
+import com.google.gerrit.server.ssh.NoSshModule;
 import com.google.gerrit.server.util.SocketUtil;
 import com.google.gerrit.testutil.TempFileUtil;
 import com.google.inject.Injector;
@@ -198,6 +201,10 @@ public class GerritServer {
       protected void configure() {
         bind(AccountCreator.class);
         factory(PushOneCommit.Factory.class);
+        factory(SubmoduleOp.Factory.class);
+        install(InProcessProtocol.module());
+        install(new NoSshModule());
+        install(new AsyncReceiveCommits.Module());
       }
     };
     return sysInjector.createChildInjector(module);

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/InProcessProtocol.java

@@ -0,0 +1,352 @@
+// Copyright (C) 2015 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 com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.InProcessProtocol.Context;
+import com.google.gerrit.common.data.Capable;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AccessPath;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.RemotePeer;
+import com.google.gerrit.server.RequestCleanup;
+import com.google.gerrit.server.config.GerritRequestModule;
+import com.google.gerrit.server.config.RequestScopedReviewDbProvider;
+import com.google.gerrit.server.git.AsyncReceiveCommits;
+import com.google.gerrit.server.git.ChangeCache;
+import com.google.gerrit.server.git.ReceiveCommits;
+import com.google.gerrit.server.git.ReceivePackInitializer;
+import com.google.gerrit.server.git.TagCache;
+import com.google.gerrit.server.git.TransferConfig;
+import com.google.gerrit.server.git.VisibleRefFilter;
+import com.google.gerrit.server.git.validators.UploadValidators;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.RequestScopePropagator;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestScopePropagator;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Key;
+import com.google.inject.Module;
+import com.google.inject.OutOfScopeException;
+import com.google.inject.Provider;
+import com.google.inject.Provides;
+import com.google.inject.Scope;
+import com.google.inject.servlet.RequestScoped;
+import com.google.inject.util.Providers;
+
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.PostReceiveHook;
+import org.eclipse.jgit.transport.PostReceiveHookChain;
+import org.eclipse.jgit.transport.PreUploadHook;
+import org.eclipse.jgit.transport.PreUploadHookChain;
+import org.eclipse.jgit.transport.ReceivePack;
+import org.eclipse.jgit.transport.TestProtocol;
+import org.eclipse.jgit.transport.UploadPack;
+import org.eclipse.jgit.transport.resolver.ReceivePackFactory;
+import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
+import org.eclipse.jgit.transport.resolver.UploadPackFactory;
+
+import java.io.IOException;
+import java.net.SocketAddress;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+class InProcessProtocol extends TestProtocol<Context> {
+  static Module module() {
+    return new AbstractModule() {
+      @Override
+      public void configure() {
+        install(new GerritRequestModule());
+        bind(RequestScopePropagator.class).to(Propagator.class);
+        bindScope(RequestScoped.class, InProcessProtocol.REQUEST);
+      }
+
+      @Provides
+      @RemotePeer
+      SocketAddress getSocketAddress() {
+        // TODO(dborowitz): Could potentially fake this with thread ID or
+        // something.
+        throw new OutOfScopeException("No remote peer in acceptance tests");
+      }
+    };
+  }
+
+  private static final Scope REQUEST = new Scope() {
+    @Override
+    public <T> Provider<T> scope(final Key<T> key, final Provider<T> creator) {
+      return new Provider<T>() {
+        @Override
+        public T get() {
+          Context ctx = current.get();
+          if (ctx == null) {
+            throw new OutOfScopeException("Not in TestProtocol scope");
+          }
+          return ctx.get(key, creator);
+        }
+
+        @Override
+        public String toString() {
+          return String.format("%s[%s]", creator, REQUEST);
+        }
+      };
+    }
+
+    @Override
+    public String toString() {
+      return "InProcessProtocol.REQUEST";
+    }
+  };
+
+  private static class Propagator
+      extends ThreadLocalRequestScopePropagator<Context> {
+    @Inject
+    Propagator(ThreadLocalRequestContext local,
+        Provider<RequestScopedReviewDbProvider> dbProviderProvider) {
+      super(REQUEST, current, local, dbProviderProvider);
+    }
+
+    @Override
+    protected Context continuingContext(Context ctx) {
+      return ctx.newContinuingContext();
+    }
+  }
+
+  private static final ThreadLocal<Context> current = new ThreadLocal<>();
+
+  // TODO(dborowitz): Merge this with AcceptanceTestRequestScope.
+  /**
+   * Multi-purpose session/context object.
+   * <p>
+   * Confusingly, Gerrit has two ideas of what a "context" object is:
+   * one for Guice {@link RequestScoped}, and one for its own simplified
+   * version of request scoping using {@link ThreadLocalRequestContext}.
+   * This class provides both, in essence just delegating the {@code
+   * ThreadLocalRequestContext} scoping to the Guice scoping mechanism.
+   * <p>
+   * It is also used as the session type for {@code UploadPackFactory} and
+   * {@code ReceivePackFactory}, since, after all, it encapsulates all the
+   * information about a single request.
+   */
+  static class Context implements RequestContext {
+    private static final Key<RequestScopedReviewDbProvider> DB_KEY =
+        Key.get(RequestScopedReviewDbProvider.class);
+    private static final Key<RequestCleanup> RC_KEY =
+        Key.get(RequestCleanup.class);
+    private static final Key<CurrentUser> USER_KEY = Key.get(CurrentUser.class);
+
+    private final SchemaFactory<ReviewDb> schemaFactory;
+    private final IdentifiedUser.GenericFactory userFactory;
+    private final Account.Id accountId;
+    private final Project.NameKey project;
+    private final RequestCleanup cleanup;
+    private final Map<Key<?>, Object> map;
+
+    Context(SchemaFactory<ReviewDb> schemaFactory,
+        IdentifiedUser.GenericFactory userFactory,
+        Account.Id accountId,
+        Project.NameKey project) {
+      this.schemaFactory = schemaFactory;
+      this.userFactory = userFactory;
+      this.accountId = accountId;
+      this.project = project;
+      map = new HashMap<>();
+      cleanup = new RequestCleanup();
+      map.put(DB_KEY,
+          new RequestScopedReviewDbProvider(
+            schemaFactory, Providers.of(cleanup)));
+      map.put(RC_KEY, cleanup);
+
+      IdentifiedUser user = userFactory.create(accountId);
+      user.setAccessPath(AccessPath.GIT);
+      map.put(USER_KEY, user);
+    }
+
+    private Context newContinuingContext() {
+      return new Context(schemaFactory, userFactory, accountId, project);
+    }
+
+    @Override
+    public CurrentUser getCurrentUser() {
+      return get(USER_KEY, null);
+    }
+
+    @Override
+    public Provider<ReviewDb> getReviewDbProvider() {
+      return get(DB_KEY, null);
+    }
+
+    private synchronized <T> T get(Key<T> key, Provider<T> creator) {
+      @SuppressWarnings("unchecked")
+      T t = (T) map.get(key);
+      if (t == null) {
+        t = creator.get();
+        map.put(key, t);
+      }
+      return t;
+    }
+  }
+
+  private static class Upload implements UploadPackFactory<Context> {
+    private final Provider<ReviewDb> dbProvider;
+    private final Provider<CurrentUser> userProvider;
+    private final TagCache tagCache;
+    private final ChangeCache changeCache;
+    private final ProjectControl.GenericFactory projectControlFactory;
+    private final TransferConfig transferConfig;
+    private final DynamicSet<PreUploadHook> preUploadHooks;
+    private final UploadValidators.Factory uploadValidatorsFactory;
+    private final ThreadLocalRequestContext threadContext;
+
+    @Inject
+    Upload(
+        Provider<ReviewDb> dbProvider,
+        Provider<CurrentUser> userProvider,
+        TagCache tagCache,
+        ChangeCache changeCache,
+        ProjectControl.GenericFactory projectControlFactory,
+        TransferConfig transferConfig,
+        DynamicSet<PreUploadHook> preUploadHooks,
+        UploadValidators.Factory uploadValidatorsFactory,
+        ThreadLocalRequestContext threadContext) {
+      this.dbProvider = dbProvider;
+      this.userProvider = userProvider;
+      this.tagCache = tagCache;
+      this.changeCache = changeCache;
+      this.projectControlFactory = projectControlFactory;
+      this.transferConfig = transferConfig;
+      this.preUploadHooks = preUploadHooks;
+      this.uploadValidatorsFactory = uploadValidatorsFactory;
+      this.threadContext = threadContext;
+    }
+
+    @Override
+    public UploadPack create(Context req, final Repository repo)
+        throws ServiceNotAuthorizedException {
+      // Set the request context, but don't bother unsetting, since we don't
+      // have an easy way to run code when this instance is done being used.
+      // Each operation is run in its own thread, so we don't need to recover
+      // its original context anyway.
+      threadContext.setContext(req);
+      current.set(req);
+      try {
+        ProjectControl ctl = projectControlFactory.controlFor(
+            req.project, userProvider.get());
+        if (!ctl.canRunUploadPack()) {
+          throw new ServiceNotAuthorizedException();
+        }
+
+        UploadPack up = new UploadPack(repo);
+        up.setPackConfig(transferConfig.getPackConfig());
+        up.setTimeout(transferConfig.getTimeout());
+
+        if (!ctl.allRefsAreVisible()) {
+          up.setAdvertiseRefsHook(new VisibleRefFilter(
+              tagCache, changeCache, repo, ctl, dbProvider.get(), true));
+        }
+        List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
+        hooks.add(uploadValidatorsFactory.create(
+            ctl.getProject(), repo, "localhost-test"));
+        up.setPreUploadHook(PreUploadHookChain.newChain(hooks));
+        return up;
+      } catch (NoSuchProjectException | IOException e) {
+        throw new RuntimeException(e);
+      }
+    }
+  }
+
+  private static class Receive implements ReceivePackFactory<Context> {
+    private final Provider<CurrentUser> userProvider;
+    private final ProjectControl.GenericFactory projectControlFactory;
+    private final AsyncReceiveCommits.Factory factory;
+    private final TransferConfig config;
+    private final DynamicSet<ReceivePackInitializer> receivePackInitializers;
+    private final DynamicSet<PostReceiveHook> postReceiveHooks;
+    private final ThreadLocalRequestContext threadContext;
+
+    @Inject
+    Receive(
+        Provider<CurrentUser> userProvider,
+        ProjectControl.GenericFactory projectControlFactory,
+        AsyncReceiveCommits.Factory factory,
+        TransferConfig config,
+        DynamicSet<ReceivePackInitializer> receivePackInitializers,
+        DynamicSet<PostReceiveHook> postReceiveHooks,
+        ThreadLocalRequestContext threadContext) {
+      this.userProvider = userProvider;
+      this.projectControlFactory = projectControlFactory;
+      this.factory = factory;
+      this.config = config;
+      this.receivePackInitializers = receivePackInitializers;
+      this.postReceiveHooks = postReceiveHooks;
+      this.threadContext = threadContext;
+    }
+
+    @Override
+    public ReceivePack create(final Context req, Repository db)
+        throws ServiceNotAuthorizedException {
+      // Set the request context, but don't bother unsetting, since we don't
+      // have an easy way to run code when this instance is done being used.
+      // Each operation is run in its own thread, so we don't need to recover
+      // its original context anyway.
+      threadContext.setContext(req);
+      current.set(req);
+      try {
+        ProjectControl ctl =
+            projectControlFactory.controlFor(req.project, userProvider.get());
+        if (!ctl.canRunReceivePack()) {
+          throw new ServiceNotAuthorizedException();
+        }
+
+        ReceiveCommits rc = factory.create(ctl, db).getReceiveCommits();
+        ReceivePack rp = rc.getReceivePack();
+
+        Capable r = rc.canUpload();
+        if (r != Capable.OK) {
+          throw new ServiceNotAuthorizedException();
+        }
+
+        IdentifiedUser user = (IdentifiedUser) ctl.getCurrentUser();
+        rp.setRefLogIdent(user.newRefLogIdent());
+        rp.setTimeout(config.getTimeout());
+        rp.setMaxObjectSizeLimit(config.getMaxObjectSizeLimit());
+
+        for (ReceivePackInitializer initializer : receivePackInitializers) {
+          initializer.init(ctl.getProject().getNameKey(), rp);
+        }
+
+        rp.setPostReceiveHook(PostReceiveHookChain.newChain(
+            Lists.newArrayList(postReceiveHooks)));
+        return rp;
+      } catch (NoSuchProjectException | IOException e) {
+        throw new RuntimeException(e);
+      }
+    }
+  }
+
+  @Inject
+  InProcessProtocol(Upload uploadPackFactory,
+      Receive receivePackFactory) {
+    super(uploadPackFactory, receivePackFactory);
+  }
+}

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/TestProjectInput.java

@@ -26,6 +26,8 @@ import java.lang.annotation.Target;
 @Target({METHOD})
 @Retention(RUNTIME)
 public @interface TestProjectInput {
+  // Fields from ProjectInput for creating the project.
+
   String parent() default "";
   boolean createEmptyCommit() default true;
   String description() default "";
@@ -38,4 +40,10 @@ public @interface TestProjectInput {
   InheritableBoolean useSignedOffBy() default InheritableBoolean.INHERIT;
   InheritableBoolean useContentMerge() default InheritableBoolean.INHERIT;
   InheritableBoolean requireChangeId() default InheritableBoolean.INHERIT;
+
+
+  // Fields specific to acceptance test behavior.
+
+  /** Username to use for initial clone, passed to {@link AccountCreator}. */
+  String cloneAs() default "admin";
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -16,7 +16,6 @@ package com.google.gerrit.acceptance.edit;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -270,7 +269,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
   @TestProjectInput(createEmptyCommit = false)
   public void updateRootCommitMessage() throws Exception {
     // Re-clone empty repo; TestRepository doesn't let us reset to unborn head.
-    testRepo = cloneProject(project, sshSession);
+    testRepo = cloneProject(project);
     changeId = newChange(admin.getIdent());
     change = getChange(changeId);
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -16,10 +16,10 @@ package com.google.gerrit.acceptance.git;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.extensions.common.ChangeInfo;
@@ -46,6 +46,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
   private NotesMigration notesMigration;
 
   protected enum Protocol {
+    // TODO(dborowitz): TEST.
     SSH, HTTP
   }
 
@@ -68,7 +69,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
       default:
         throw new IllegalArgumentException("unexpected protocol: " + p);
     }
-    testRepo = cloneProject(project, url + "/" + project.get());
+    testRepo = GitUtil.cloneProject(project, url + "/" + project.get());
   }
 
   @Test

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeOwnerIT.java

@@ -15,16 +15,14 @@
 package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
-import static com.google.gerrit.acceptance.GitUtil.initSsh;
 import static com.google.gerrit.common.data.Permission.LABEL;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.RestSession;
-import com.google.gerrit.acceptance.SshSession;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRule;
@@ -49,28 +47,27 @@ public class ChangeOwnerIT extends AbstractDaemonTest {
 
   @Before
   public void setUp() throws Exception {
+    setApiUser(user);
     sessionOwner = new RestSession(server, user);
-    SshSession sshSession = new SshSession(server, user);
-    initSsh(user);
-    sshSession.open();
-    testRepo = cloneProject(project, sshSession);
-    sshSession.close();
     user2 = accounts.user2();
     sessionDev = new RestSession(server, user2);
   }
 
   @Test
+  @TestProjectInput(cloneAs = "user")
   public void testChangeOwner_OwnerACLNotGranted() throws Exception {
     approve(sessionOwner, createMyChange(), HttpStatus.SC_FORBIDDEN);
   }
 
   @Test
+  @TestProjectInput(cloneAs = "user")
   public void testChangeOwner_OwnerACLGranted() throws Exception {
     grantApproveToChangeOwner();
     approve(sessionOwner, createMyChange(), HttpStatus.SC_OK);
   }
 
   @Test
+  @TestProjectInput(cloneAs = "user")
   public void testChangeOwner_NotOwnerACLGranted() throws Exception {
     grantApproveToChangeOwner();
     approve(sessionDev, createMyChange(), HttpStatus.SC_FORBIDDEN);

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.acceptance.rest.project;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
 import static com.google.gerrit.acceptance.GitUtil.fetch;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -75,7 +74,7 @@ public class ProjectLevelConfigIT extends AbstractDaemonTest {
         .assertOkStatus();
 
     Project.NameKey childProject = createProject("child", project);
-    TestRepository<?> childTestRepo = cloneProject(childProject, sshSession);
+    TestRepository<?> childTestRepo = cloneProject(childProject);
     fetch(childTestRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
     childTestRepo.reset("refs/heads/config");
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/JschVerifyFalseBugIT.java

@@ -15,9 +15,9 @@
 package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.acceptance.GitUtil.cloneProject;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.reviewdb.client.Project;
 
@@ -51,7 +51,7 @@ public class JschVerifyFalseBugIT extends AbstractDaemonTest {
         for (int i = 1; i < 100; i++) {
           String p = "p" + i;
           createProject(p);
-          cloneProject(new Project.NameKey(p), sshSession);
+          GitUtil.cloneProject(new Project.NameKey(p), sshSession);
         }
         return null;
       }