From 88da34f7fc0ef5d7b2bcf080b12a13bf1f4999d3 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 15 Jan 2014 14:24:33 -0800 Subject: [PATCH] Support acceptance tests running with different configs Store a library of configs to be used as the base gerrit.config during tests. Specify at a per-acceptance_tests-rule level which of these configs to generate rules for, passing in the config file via a system property in a new rule named based on the config file name. Include two additional labels for each test, "acceptance_config" and "config_", for including and excluding specific configs or all non-default configs. This preserves the behavior of running a single test class per VM, at the cost of making it harder to run tests with nonstandard configs from Eclipse. For that, developers can create custom run configurations with different VM args passing the appropriate -D. Change-Id: If29bcba26ac271479a63da836008b0f6608a1b6c --- gerrit-acceptance-tests/BUCK | 13 +++++- .../gerrit/acceptance/AbstractDaemonTest.java | 13 ++++-- .../acceptance/ConfigAnnotationParser.java | 43 ++++++++++++++----- .../gerrit/acceptance/GerritServer.java | 4 +- gerrit-acceptance-tests/tests.defs | 40 +++++++++++------ tools/eclipse/BUCK | 1 + 6 files changed, 83 insertions(+), 31 deletions(-) diff --git a/gerrit-acceptance-tests/BUCK b/gerrit-acceptance-tests/BUCK index 756450f3db..0fc7fbc1f1 100644 --- a/gerrit-acceptance-tests/BUCK +++ b/gerrit-acceptance-tests/BUCK @@ -1,5 +1,3 @@ -include_defs('//gerrit-acceptance-tests/tests.defs') - java_library( name = 'lib', srcs = glob(['src/test/java/com/google/gerrit/acceptance/*.java']), @@ -42,3 +40,14 @@ java_library( '//gerrit-acceptance-tests/...', ], ) + +java_library( + name = 'configs', + resources = glob([ + 'src/test/resources/com/google/gerrit/acceptance/config/*.config', + ]), + visibility = [ + '//tools/eclipse:classpath', + '//gerrit-acceptance-tests/...', + ], +) 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 8a06f15031..010589786c 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 @@ -17,12 +17,15 @@ package com.google.gerrit.acceptance; import com.google.gerrit.server.OutputFormat; import com.google.gson.Gson; +import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.junit.Rule; import org.junit.rules.TestRule; import org.junit.runner.Description; import org.junit.runners.model.Statement; +import java.io.IOException; + public abstract class AbstractDaemonTest { protected GerritServer server; @@ -42,18 +45,20 @@ public abstract class AbstractDaemonTest { } }; - private static Config config(Description description) { + private static Config config(Description description) + throws IOException, ConfigInvalidException { + Config base = ConfigAnnotationParser.parseFromSystemProperty(); GerritConfigs cfgs = description.getAnnotation(GerritConfigs.class); GerritConfig cfg = description.getAnnotation(GerritConfig.class); if (cfgs != null && cfg != null) { throw new IllegalStateException("Use either @GerritConfigs or @GerritConfig not both"); } if (cfgs != null) { - return ConfigAnnotationParser.parse(cfgs); + return ConfigAnnotationParser.parse(base, cfgs); } else if (cfg != null) { - return ConfigAnnotationParser.parse(cfg); + return ConfigAnnotationParser.parse(base, cfg); } else { - return null; + return base; } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ConfigAnnotationParser.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ConfigAnnotationParser.java index cf60fb444a..a1285b54a1 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ConfigAnnotationParser.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ConfigAnnotationParser.java @@ -14,36 +14,59 @@ package com.google.gerrit.acceptance; -import com.google.common.base.Splitter; -import com.google.common.collect.Lists; +import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.base.Charsets; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.Lists; +import com.google.common.io.Resources; + +import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; +import java.io.IOException; +import java.net.URL; import java.util.ArrayList; class ConfigAnnotationParser { - + private static final String CONFIG_PKG = + "com.google.gerrit.acceptance.config."; + private static final String CONFIG_DIR = "/" + CONFIG_PKG.replace('.', '/'); private static Splitter splitter = Splitter.on(".").trimResults(); - static Config parse(GerritConfigs annotation) { + static Config parseFromSystemProperty() + throws ConfigInvalidException, IOException { + Config cfg = new Config(); + String name = System.getProperty(CONFIG_PKG + "BaseConfig"); + if (!Strings.isNullOrEmpty(name)) { + String resource = CONFIG_DIR + name + ".config"; + URL url = checkNotNull(ConfigAnnotationParser.class.getResource(resource), + "test config resource not found: %s", resource); + cfg.fromText(Resources.toString(url, Charsets.UTF_8)); + } + return cfg; + } + + static Config parse(Config base, GerritConfigs annotation) { if (annotation == null) { return null; } - Config cfg = new Config(); + Config cfg = new Config(base); for (GerritConfig c : annotation.value()) { - parse(cfg, c); + parseAnnotation(cfg, c); } return cfg; } - static Config parse(GerritConfig annotation) { - Config cfg = new Config(); - parse(cfg, annotation); + static Config parse(Config base, GerritConfig annotation) { + Config cfg = new Config(base); + parseAnnotation(cfg, annotation); return cfg; } - static private void parse(Config cfg, GerritConfig c) { + static private void parseAnnotation(Config cfg, GerritConfig c) { ArrayList l = Lists.newArrayList(splitter.split(c.name())); if (l.size() == 2) { cfg.setString(l.get(0), null, l.get(1), c.value()); 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 6caa2a62a4..272b5abc15 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,7 @@ import java.util.concurrent.TimeUnit; public class GerritServer { /** Returns fully started Gerrit server */ - static GerritServer start(Config base, boolean memory) throws Exception { + static GerritServer start(Config cfg, boolean memory) throws Exception { final CyclicBarrier serverStarted = new CyclicBarrier(2); final Daemon daemon = new Daemon(new Runnable() { public void run() { @@ -67,7 +67,6 @@ public class GerritServer { ExecutorService daemonService = null; if (memory) { site = null; - Config cfg = base != null ? base : new Config(); mergeTestConfig(cfg); cfg.setBoolean("httpd", null, "requestLog", false); cfg.setBoolean("sshd", null, "requestLog", false); @@ -79,7 +78,6 @@ public class GerritServer { new InMemoryTestingDatabaseModule(cfg))); daemon.start(); } else { - Config cfg = base != null ? base : new Config(); site = initSite(cfg); daemonService = Executors.newSingleThreadExecutor(); daemonService.submit(new Callable() { diff --git a/gerrit-acceptance-tests/tests.defs b/gerrit-acceptance-tests/tests.defs index 3c8b4d5484..c682e7b48e 100644 --- a/gerrit-acceptance-tests/tests.defs +++ b/gerrit-acceptance-tests/tests.defs @@ -1,20 +1,36 @@ def acceptance_tests( srcs, deps = [], + configs = [], vm_args = ['-Xmx256m']): for j in srcs: - java_test( - name = j[:-len('.java')], - srcs = [j], - deps = ['//gerrit-acceptance-tests:lib'] + deps, - source_under_test = [ - '//gerrit-httpd:httpd', - '//gerrit-sshd:sshd', - '//gerrit-server:server', - ], + for config in [None] + configs: + name = j[:-len('.java')] labels = [ 'acceptance', 'slow', - ], - vm_args = vm_args, - ) + ] + curr_vm_args = list(vm_args) + + if config: + curr_vm_args.append( + '-Dcom.google.gerrit.acceptance.config.BaseConfig=' + config) + name += '_' + config + labels.append('acceptance_config') + labels.append('config_' + config) + + java_test( + name = name, + srcs = [j], + deps = deps + [ + '//gerrit-acceptance-tests:configs', + '//gerrit-acceptance-tests:lib', + ], + source_under_test = [ + '//gerrit-httpd:httpd', + '//gerrit-sshd:sshd', + '//gerrit-server:server', + ], + labels = labels, + vm_args = curr_vm_args, + ) diff --git a/tools/eclipse/BUCK b/tools/eclipse/BUCK index c60d9f2ac2..c2a8fa8a62 100644 --- a/tools/eclipse/BUCK +++ b/tools/eclipse/BUCK @@ -3,6 +3,7 @@ include_defs('//tools/build.defs') java_library( name = 'classpath', deps = LIBS + PGMLIBS + [ + '//gerrit-acceptance-tests:configs', '//gerrit-acceptance-tests:lib', '//gerrit-gwtdebug:gwtdebug', '//gerrit-gwtui:ui_module',