From 83ea0403b118c99a9986d1a7fc46247c644ea4b2 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 3 Feb 2014 09:21:51 +0100 Subject: [PATCH] Acceptance tests: Disable HTTPD per class or method annotation If HTTPD not used in tests, it doesn't make much sense to enable it. Using @NoHttpd on class or method it is now possible to disable HTTP entirely and is another small step making slow tests fast. With maturity of plugin API, more and more tests are going to move to plugin API driven tests from REST driven tests. Benchmark shows, that doing REST vs. plugin API comparisson: @Test public void rest() throws Exception { String changeId = createChange().getChangeId(); for (int i = 0; i < 1000; i++) { getRESTChange(changeId, ALL); } } @Test public void api() throws Exception { String changeId = createChange().getChangeId(); for (int i = 0; i < 1000; i++) { getPluginAPIChange(changeId, ALL); } } make on my machine ca. 2 sec. for plugin API vs. 10 sec. for REST: The whole Servlet/HTTP stack and Gson serialization and deserialization is out if the picture for API driven tests. Change-Id: If60170f06f466765d9602450043a1629a344d7ce --- .../gerrit/acceptance/AbstractDaemonTest.java | 13 +++++---- .../gerrit/acceptance/GerritServer.java | 4 ++- .../com/google/gerrit/acceptance/NoHttpd.java | 27 +++++++++++++++++++ .../acceptance/api/change/ChangeIT.java | 2 ++ .../acceptance/api/project/ProjectIT.java | 2 ++ .../acceptance/api/revision/RevisionIT.java | 2 ++ .../acceptance/git/DraftChangeBlockedIT.java | 2 ++ .../acceptance/git/SshPushForReviewIT.java | 3 +++ .../gerrit/acceptance/git/SubmitOnPushIT.java | 2 ++ .../rest/change/ListChangesOptionsIT.java | 2 ++ .../server/project/LabelTypeIT.java | 2 ++ .../java/com/google/gerrit/pgm/Daemon.java | 4 +++ 12 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/NoHttpd.java diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 2c9f27a47b..14571cd985 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -93,7 +93,9 @@ public abstract class AbstractDaemonTest { @Override public void evaluate() throws Throwable { boolean mem = description.getAnnotation(UseLocalDisk.class) == null; - beforeTest(config(description), mem); + boolean enableHttpd = description.getAnnotation(NoHttpd.class) == null + && description.getTestClass().getAnnotation(NoHttpd.class) == null; + beforeTest(config(description), mem, enableHttpd); base.evaluate(); afterTest(); } @@ -116,8 +118,8 @@ public abstract class AbstractDaemonTest { } } - private void beforeTest(Config cfg, boolean memory) throws Exception { - server = startServer(cfg, memory); + private void beforeTest(Config cfg, boolean memory, boolean enableHttpd) throws Exception { + server = startServer(cfg, memory, enableHttpd); server.getTestInjector().injectMembers(this); admin = accounts.admin(); user = accounts.user(); @@ -133,8 +135,9 @@ public abstract class AbstractDaemonTest { git = cloneProject(sshSession.getUrl() + "/" + project.get()); } - protected GerritServer startServer(Config cfg, boolean memory) throws Exception { - return GerritServer.start(cfg, memory); + protected GerritServer startServer(Config cfg, boolean memory, + boolean enableHttpd) throws Exception { + return GerritServer.start(cfg, memory, enableHttpd); } private void afterTest() throws Exception { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java index 272b5abc15..a8ce229b18 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java @@ -49,7 +49,8 @@ import java.util.concurrent.TimeUnit; public class GerritServer { /** Returns fully started Gerrit server */ - static GerritServer start(Config cfg, boolean memory) throws Exception { + static GerritServer start(Config cfg, boolean memory, boolean enableHttpd) + throws Exception { final CyclicBarrier serverStarted = new CyclicBarrier(2); final Daemon daemon = new Daemon(new Runnable() { public void run() { @@ -71,6 +72,7 @@ public class GerritServer { cfg.setBoolean("httpd", null, "requestLog", false); cfg.setBoolean("sshd", null, "requestLog", false); cfg.setBoolean("index", "lucene", "testInmemory", true); + daemon.setEnableHttpd(enableHttpd); daemon.setLuceneModule(new LuceneIndexModule( ChangeSchemas.getLatest().getVersion(), Runtime.getRuntime().availableProcessors(), null)); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/NoHttpd.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/NoHttpd.java new file mode 100644 index 0000000000..378439ce2d --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/NoHttpd.java @@ -0,0 +1,27 @@ +// Copyright (C) 2014 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; + +@Target({TYPE, METHOD}) +@Retention(RUNTIME) +public @interface NoHttpd { +} diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index 929df3f577..6dff20a219 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.api.change; import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.ReviewInput; @@ -30,6 +31,7 @@ import org.junit.Test; import java.io.IOException; +@NoHttpd public class ChangeIT extends AbstractDaemonTest { @Test diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java index c8064846cc..54afd0bde2 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/project/ProjectIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.project; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.restapi.RestApiException; @@ -23,6 +24,7 @@ import org.junit.Test; import java.io.IOException; +@NoHttpd public class ProjectIT extends AbstractDaemonTest { @Test diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 96ce57eb5e..72b7ff158d 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.revision; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.CherryPickInput; @@ -27,6 +28,7 @@ import org.junit.Test; import java.io.IOException; +@NoHttpd public class RevisionIT extends AbstractDaemonTest { @Test diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java index 20aebb6072..b69c26478b 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/DraftChangeBlockedIT.java @@ -18,6 +18,7 @@ import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.project.Util.grant; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.common.data.Permission; import com.google.gerrit.server.config.AllProjectsName; @@ -33,6 +34,7 @@ import org.junit.Test; import java.io.IOException; +@NoHttpd public class DraftChangeBlockedIT extends AbstractDaemonTest { @Inject diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SshPushForReviewIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SshPushForReviewIT.java index 5251d2d0e8..c037e76d66 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SshPushForReviewIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SshPushForReviewIT.java @@ -14,11 +14,14 @@ package com.google.gerrit.acceptance.git; +import com.google.gerrit.acceptance.NoHttpd; + import org.eclipse.jgit.api.errors.GitAPIException; import org.junit.Before; import java.io.IOException; +@NoHttpd public class SshPushForReviewIT extends AbstractPushForReview { @Before public void selectSshUrl() throws GitAPIException, IOException { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index f3509092e4..a73169dc18 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertTrue; 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.SshSession; import com.google.gerrit.common.data.AccessSection; @@ -62,6 +63,7 @@ import org.junit.Test; import java.io.IOException; +@NoHttpd public class SubmitOnPushIT extends AbstractDaemonTest { @Inject diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ListChangesOptionsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ListChangesOptionsIT.java index 0a6a3dab0f..97ae2fd312 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ListChangesOptionsIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ListChangesOptionsIT.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertNull; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.extensions.common.ChangeInfo; @@ -30,6 +31,7 @@ import org.junit.Test; import java.util.List; +@NoHttpd public class ListChangesOptionsIT extends AbstractDaemonTest { private String changeId; diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java index 56c9c21230..ce722faada 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java @@ -19,6 +19,7 @@ import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LAB import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.extensions.api.changes.ReviewInput; @@ -36,6 +37,7 @@ import org.eclipse.jgit.lib.Repository; import org.junit.Before; import org.junit.Test; +@NoHttpd public class LabelTypeIT extends AbstractDaemonTest { @Inject diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index aba492d35e..cac5a78239 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -155,6 +155,10 @@ public class Daemon extends SiteProgram { this.serverStarted = serverStarted; } + public void setEnableHttpd(boolean enable) { + httpd = enable; + } + @Override public int run() throws Exception { if (doInit) {