From 063e0fd4fd688c1963e5e0402bee1d27b57db009 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 30 Jan 2014 16:04:47 -0800 Subject: [PATCH] Run acceptance tests with different configs using @Parameterized This is a simpler approach as it doesn't require generating extra test targets for Buck, nor rebuilding test jars to get everything working in Eclipse. There are enough tests running at any given time with Buck that having each acceptance test take twice as long should maintain adequate parallelism. Change-Id: I109acaf6b957e8f9dd621e98abe46a75f78cdc32 --- gerrit-acceptance-tests/BUCK | 11 ----- .../gerrit/acceptance/AbstractDaemonTest.java | 43 ++++++++++++++++--- .../acceptance/ConfigAnnotationParser.java | 24 ----------- gerrit-acceptance-tests/tests.defs | 40 ++++++----------- 4 files changed, 48 insertions(+), 70 deletions(-) diff --git a/gerrit-acceptance-tests/BUCK b/gerrit-acceptance-tests/BUCK index 0fc7fbc1f1..3c267209f2 100644 --- a/gerrit-acceptance-tests/BUCK +++ b/gerrit-acceptance-tests/BUCK @@ -40,14 +40,3 @@ 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 2c69dd947a..0675428f76 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 @@ -14,6 +14,7 @@ package com.google.gerrit.acceptance; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gerrit.acceptance.GitUtil.initSsh; import static org.junit.Assert.assertEquals; @@ -25,16 +26,46 @@ import com.google.gson.Gson; import com.google.inject.Inject; import org.apache.http.HttpStatus; -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.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; import org.junit.runners.model.Statement; import java.io.IOException; +import java.util.Arrays; +@RunWith(Parameterized.class) public abstract class AbstractDaemonTest { + private static class NamedConfig extends Config { + private final String name; + + private NamedConfig(String name) { + this.name = checkNotNull(name); + } + + @Override + public String toString() { + return name; + } + } + + @Parameters(name = "{0}") + public static Iterable configs() { + Config defaultConfig = new NamedConfig("default"); + + return Arrays.asList(new Object[][]{ + {defaultConfig}, + }); + } + + @Parameter + public Config baseConfig; + @Inject protected AccountCreator accounts; @@ -58,20 +89,18 @@ public abstract class AbstractDaemonTest { } }; - private static Config config(Description description) - throws IOException, ConfigInvalidException { - Config base = ConfigAnnotationParser.parseFromSystemProperty(); + private Config config(Description description) { 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(base, cfgs); + return ConfigAnnotationParser.parse(baseConfig, cfgs); } else if (cfg != null) { - return ConfigAnnotationParser.parse(base, cfg); + return ConfigAnnotationParser.parse(baseConfig, cfg); } else { - return base; + return baseConfig; } } 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 a1285b54a1..41df3e6f9d 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,40 +14,16 @@ package com.google.gerrit.acceptance; -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 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; diff --git a/gerrit-acceptance-tests/tests.defs b/gerrit-acceptance-tests/tests.defs index c682e7b48e..3c8b4d5484 100644 --- a/gerrit-acceptance-tests/tests.defs +++ b/gerrit-acceptance-tests/tests.defs @@ -1,36 +1,20 @@ def acceptance_tests( srcs, deps = [], - configs = [], vm_args = ['-Xmx256m']): for j in srcs: - for config in [None] + configs: - name = j[:-len('.java')] + 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', + ], labels = [ 'acceptance', 'slow', - ] - 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, - ) + ], + vm_args = vm_args, + )