Revision: 5364
Author: [email protected]
Date: Mon Apr 22 10:43:32 2013
Log: cleanup test flags
https://codereview.appspot.com/8912043
- We were using both caja.test.foo and test.foo properties. There
isn't really a reason to require a caja. prefix, so I'm renaming
them all to test.foo.
- I've renamed some of the existing test.foo properties for clarity.
- Added a typo check for test flags.
- test.filter.method is now really a glob. Before, 'x' would be
interpreted as /^.*x.*$/ rather than /^x$/.
R=kpreid2
http://code.google.com/p/google-caja/source/detail?r=5364
Added:
/trunk/tests/com/google/caja/util/TestFlag.java
/trunk/tests/com/google/caja/util/TestFlagTypoCheck.java
Modified:
/trunk/build.xml
/trunk/tests/com/google/caja/plugin/BrowserTestCase.java
/trunk/tests/com/google/caja/plugin/WebDriverHandle.java
/trunk/tests/com/google/caja/util/CajaTestCase.java
/trunk/tests/com/google/caja/util/LocalServer.java
/trunk/tests/com/google/caja/util/ThisHostName.java
=======================================
--- /dev/null
+++ /trunk/tests/com/google/caja/util/TestFlag.java Mon Apr 22 10:43:32 2013
@@ -0,0 +1,109 @@
+// Copyright (C) 2013 Google Inc.
+//
+// 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.caja.util;
+
+import java.io.FileDescriptor;
+import java.io.FileOutputStream;
+import java.io.PrintStream;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * See <a href="https://code.google.com/p/google-caja/wiki/CajaTesting";
+ * >CajaTesting</a>
+ */
+
+public enum TestFlag {
+ BROWSER(
+ "test.browser"),
+ BROWSER_CLOSE(
+ "test.browser.close"),
+ CHROME_BINARY(
+ "test.chrome.binary"),
+ DEBUG(
+ "test.debug"),
+ DEBUG_BROWSER(
+ "test.debug.browser"),
+ DEBUG_SERVER(
+ "test.debug.server"),
+ EXCLUDE(
+ "test.exclude"),
+ FAILURE_NOT_AN_OPTION(
+ "test.failureNotAnOption"),
+ FILTER(
+ "test.filter"),
+ FILTER_METHOD(
+ "test.filter.method"),
+ SERVER_HOSTNAME(
+ "test.server.hostname"),
+ SERVER_PORT(
+ "test.server.port"),
+ THREADS(
+ "test.threads"),
+ WEBDRIVER_URL(
+ "test.webdriver.url");
+
+ private static class Names {
+ private static final Set<String> set = new HashSet<String>();
+ }
+
+ private String name;
+
+ TestFlag(String name) {
+ this.name = name;
+ Names.set.add(name);
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public static Set<String> all() {
+ return Names.set;
+ }
+
+ public String getString(String dflt) {
+ return System.getProperty(name, dflt);
+ }
+
+ public int getInt(int dflt) {
+ String value = System.getProperty(name);
+ if (value == null || "".equals(value)) {
+ return dflt;
+ }
+ try {
+ return Integer.parseInt(value);
+ } catch (NumberFormatException e) {
+ throw error("Invalid value " + value + " for " + name);
+ }
+ }
+
+ public boolean truthy() {
+ String value = System.getProperty(name);
+ if (value == null) { return false; }
+ return "1".equals(value) || "true".equalsIgnoreCase(value)
+ || "y".equalsIgnoreCase(value) || "yes".equalsIgnoreCase(value);
+ }
+
+ private RuntimeException error(String message) {
+ // System.err is captured by ant-junit and goes to the test logs
+ // FileDescriptor.err is captured by ant and goes to stdout.
+ @SuppressWarnings("resource")
+ PrintStream err = new PrintStream(
+ new FileOutputStream(FileDescriptor.err), true);
+ err.println(message);
+ return new RuntimeException(message);
+ }
+}
=======================================
--- /dev/null
+++ /trunk/tests/com/google/caja/util/TestFlagTypoCheck.java Mon Apr 22
10:43:32 2013
@@ -0,0 +1,36 @@
+package com.google.caja.util;
+
+import java.util.Collections;
+import java.util.Properties;
+
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Task;
+
+public class TestFlagTypoCheck extends Task {
+ @Override
+ public void execute() throws BuildException {
+ StringBuilder err = new StringBuilder();
+ Properties props = System.getProperties();
+ for (Object key : Collections.list(props.propertyNames())) {
+ check(key, err);
+ }
+ for (Object key : getProject().getProperties().keySet()) {
+ check(key, err);
+ }
+ if (err.length() != 0) {
+ throw new BuildException(err.toString());
+ }
+ }
+
+ private void check(Object key, StringBuilder err) {
+ if (key instanceof String) {
+ String name = (String) key;
+ if (name.startsWith("caja.test.")) {
+ err.append("Obsolete property " + name + "\n");
+ }
+ if (name.startsWith("test.") && !TestFlag.all().contains(name)) {
+ err.append("Unknown property " + name + "\n");
+ }
+ }
+ }
+}
=======================================
--- /trunk/build.xml Thu Apr 11 19:45:31 2013
+++ /trunk/build.xml Mon Apr 22 10:43:32 2013
@@ -47,7 +47,7 @@
<property name="tests.caja" location="${tests}/com/google/caja"/>
<property name="test.exclude" value="--none--"/>
<property name="test.filter" value="*Test"/>
- <property name="test.method.filter" value="*"/>
+ <property name="test.filter.method" value="*"/>
<property name="third_party" location="third_party"/>
<property name="demos" location="${src.caja}/demos"/>
<property name="precajolesrc" value="${third_party}/precajole"/>
@@ -200,8 +200,8 @@
<target name="brserve"
description="Runs just the local HTTP server used by browser tests.">
<antcall target="runtests">
- <param name="caja.test.serverOnly" value="true"/>
- <!-- Run an arbitrary test to trigger the serverOnly logic -->
+ <param name="test.debug.server" value="true"/>
+ <!-- Run an arbitrary test to trigger the test.debug logic -->
<param name="test.filter" value="Es5BrowserTest"/>
</antcall>
</target>
@@ -209,8 +209,8 @@
<target name="brserve+"
description="... and start a test browser.">
<antcall target="runtests">
- <param name="caja.test.startAndWait" value="true"/>
- <!-- Run an arbitrary test to trigger the startAndWait logic -->
+ <param name="test.debug.browser" value="true"/>
+ <!-- Run an arbitrary test to trigger the test.debug logic -->
<param name="test.filter" value="Es5BrowserTest"/>
</antcall>
</target>
@@ -270,8 +270,11 @@
<property name="testonly.user.region" value="TR"/>
<propertyset id="test.propertyset">
+ <!-- caja.test.* is obsolete, but we propagate them so the
+ test runner can warn the user -->
<propertyref prefix="caja.test."/>
<propertyref prefix="emma."/>
+ <propertyref prefix="junit.seed"/>
<propertyref prefix="test."/>
<propertyref prefix="testonly."/>
<propertyref prefix="webdriver."/>
@@ -353,7 +356,20 @@
<exec executable="tools/codecheck.sh" failonerror="true"/>
</target>
+ <target name="RuntestsTypoCheck">
+ <javac destdir="${testlib}" debug="true" target="1.5" source="1.5">
+ <src path="${tests}"/>
+ <classpath refid="classpath.tests"/>
+ <include name="**/TestFlagTypoCheck.java"/>
+ </javac>
+ <taskdef name="testflagtypocheck"
+ classname="com.google.caja.util.TestFlagTypoCheck"
+ classpathref="classpath.tests" />
+ <testflagtypocheck/>
+ </target>
+
<target name="RuntestsRun">
+ <antcall target="RuntestsTypoCheck"/>
<antcall target="RuntestsCodeCheck"/>
<condition property="jvmarg"
value="-Xdebug -Xmx2G
-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=9999" else="">
@@ -414,9 +430,9 @@
<istrue value="${runtests.success}"/>
<equals arg1="${test.exclude}" arg2="--none--"/>
<equals arg1="${test.filter}" arg2="*Test"/>
- <equals arg1="${test.method.filter}" arg2="*"/>
- <isfalse value="${caja.test.serverOnly}"/>
- <isfalse value="${caja.test.startAndWait}"/>
+ <equals arg1="${test.filter.method}" arg2="*"/>
+ <isfalse value="${test.debug.server}"/>
+ <isfalse value="${test.debug.browser}"/>
</and>
</condition>
<fail message=
=======================================
--- /trunk/tests/com/google/caja/plugin/BrowserTestCase.java Mon Apr 15
14:18:10 2013
+++ /trunk/tests/com/google/caja/plugin/BrowserTestCase.java Mon Apr 22
10:43:32 2013
@@ -19,6 +19,8 @@
import com.google.caja.reporting.MessageQueue;
import com.google.caja.util.CajaTestCase;
import com.google.caja.util.LocalServer;
+import com.google.caja.util.TestFlag;
+import com.google.caja.util.ThisHostName;
import java.io.FileDescriptor;
import java.io.FileOutputStream;
@@ -39,57 +41,16 @@
* <a href="http://code.google.com/p/google-caja/wiki/CajaTesting";
* >CajaTesting wiki page</a>
* <p>
- * Useful system properties:
- * <dl>
- * <dt>caja.test.browser</dt>
- * <dd>Which browser driver to use. Default is "firefox".</dd>
- *
- * <dt>caja.test.browserPath</dt>
- * <dd>Override location of browser executable. Currently only
- * for Chrome (sets chrome.binary for webdriver).</dd>
- *
- * <dt>caja.test.closeBrowser</dt>
- * <dd>When true, always close browser when done. Normally when a browser
- * test fails, we try to keep the browser open so the error can be
- * manually inspected. This flag disables that.</dd>
- *
- * <dt>caja.test.remote</dt>
- * <dd>URL of a remote webdriver, which should usually be something like
- * "http://hostname:4444/wd/hub";. If unset, use a local webdriver.</dd>
- *
- * <dt>caja.test.serverOnly</dt>
- * <dd>When true, start server and wait</dd>
- *
- * <dt>caja.test.serverPort</dt>
- * <dd>What port to use for the localhost webserver. Default is 8000 when
- * using one of the manual testing options (serverOnly or startAndWait).
- * Otherwise, default is 0 (which chooses any available port).</dd>
- *
- * <dt>caja.test.startAndWait</dt>
- * <dd>When true, start server and browser and wait</dd>
- *
- * <dt>caja.test.thishostname</dt>
- * <dd>Hostname that a remote browser should use to contact the
- * localhost server. If unset, guesses a non-loopback hostname.</dd>
- * </dl>
- * <p>
* Type parameter D is for data passed in to subclass overrides of
driveBrowser.
*
* @author [email protected] (Ziqing Mao)
* @author [email protected] (Kevin Reid)
*/
public abstract class BrowserTestCase<D> extends CajaTestCase {
- // TODO(felix8a): gather flags
- private static final String CLOSE_BROWSER = "caja.test.closeBrowser";
- private static final String REMOTE = "caja.test.remote";
- private static final String SERVER_ONLY = "caja.test.serverOnly";
- private static final String SERVER_PORT = "caja.test.serverPort";
- private static final String START_AND_WAIT = "caja.test.startAndWait";
-
// We acquire a WebDriverHandle on construction because the test runner
// constructs all the TestCase objects before running any of them, and
// we want WebDriverHandle's refcount to stay nonzero as long as
possible.
- private WebDriverHandle wdh = new WebDriverHandle();
+ private final WebDriverHandle wdh = new WebDriverHandle();
protected String testBuildVersion = null;
@@ -167,12 +128,12 @@
protected String runBrowserTest(String pageName, D data,
String... params) throws Exception {
- int serverPort = intProp(SERVER_PORT, 0);
+ int serverPort = TestFlag.SERVER_PORT.getInt(0);
- if (flag(SERVER_ONLY) || flag(START_AND_WAIT)) {
+ if (TestFlag.DEBUG_BROWSER.truthy() || TestFlag.DEBUG_SERVER.truthy())
{
pageName = "test-index.html";
params = null;
- serverPort = intProp(SERVER_PORT, 8000);
+ serverPort = TestFlag.SERVER_PORT.getInt(8000);
}
String result = "";
@@ -185,9 +146,13 @@
throw e;
}
- String localhost = "localhost";
- if (System.getProperty(REMOTE) != null) {
- localhost = localServer.hostname();
+ String localhost = TestFlag.SERVER_HOSTNAME.getString(null);
+ if (localhost == null) {
+ if (TestFlag.WEBDRIVER_URL.truthy()) {
+ localhost = ThisHostName.value();
+ } else {
+ localhost = "localhost";
+ }
}
String page = "http://"; + localhost + ":" + localServer.getPort()
+ "/ant-testlib/com/google/caja/plugin/" + pageName;
@@ -196,12 +161,12 @@
}
getErr().println("- Try " + page);
- if (flag(SERVER_ONLY)) {
+ if (TestFlag.DEBUG_SERVER.truthy()) {
Thread.currentThread().join();
}
WebDriver driver = wdh.makeWindow();
driver.get(page);
- if (flag(START_AND_WAIT)) {
+ if (TestFlag.DEBUG_BROWSER.truthy()) {
Thread.currentThread().join();
}
@@ -210,7 +175,7 @@
} finally {
localServer.stop();
// It's helpful for debugging to keep failed windows open.
- if (!passed && !isKnownFailure() && !flag(CLOSE_BROWSER)) {
+ if (!passed && !isKnownFailure()
&& !TestFlag.BROWSER_CLOSE.truthy()) {
wdh.keepOpen();
} else {
wdh.closeWindow();
@@ -218,25 +183,6 @@
}
return result;
}
-
- static protected boolean flag(String name) {
- String value = System.getProperty(name);
- return value != null && !"".equals(value) && !"0".equals(value)
- && !"false".equalsIgnoreCase(value);
- }
-
- static protected int intProp(String name, int dflt) {
- String value = System.getProperty(name);
- if (value == null || "".equals(value)) {
- return dflt;
- }
- try {
- return Integer.parseInt(value);
- } catch (NumberFormatException e) {
- getErr().println("Invalid value " + value + " for " + name);
- throw e;
- }
- }
protected String runTestDriver(
String testDriver, boolean es5, String... params)
=======================================
--- /trunk/tests/com/google/caja/plugin/WebDriverHandle.java Mon Apr 15
15:07:57 2013
+++ /trunk/tests/com/google/caja/plugin/WebDriverHandle.java Mon Apr 22
10:43:32 2013
@@ -30,6 +30,8 @@
import org.openqa.selenium.remote.DesiredCapabilities;
import org.openqa.selenium.remote.RemoteWebDriver;
+import com.google.caja.util.TestFlag;
+
/**
* Manages WebDriver instances.
*
@@ -50,20 +52,12 @@
*/
class WebDriverHandle {
- // TODO(felix8a): gather flags
- private static final String BROWSER = "caja.test.browser";
- private static final String BROWSER_PATH = "caja.test.browserPath";
- private static final String REMOTE = "caja.test.remote";
-
private static RemoteWebDriver driver = null;
private static int refCount = 0;
private static String firstWindow = null;
private static int windowSeq = 1;
private static int keptWindows = 0;
- private final static String browserType =
- System.getProperty(BROWSER, "firefox");
-
WebDriverHandle() {
refCount += 1;
}
@@ -80,7 +74,7 @@
// ignore
}
}
- JavascriptExecutor jsexec = (JavascriptExecutor) driver;
+ JavascriptExecutor jsexec = driver;
String name = "cajatestwin" + (windowSeq++);
Boolean result = (Boolean) jsexec.executeScript(
"return !!window.open('', '" + name + "')");
@@ -112,7 +106,7 @@
}
String getBrowserType() {
- return browserType;
+ return TestFlag.BROWSER.getString("firefox");
}
void closeWindow() {
@@ -146,22 +140,24 @@
}
private RemoteWebDriver makeDriver() {
- String browserPath = System.getProperty(BROWSER_PATH);
- String remote = System.getProperty(REMOTE, "");
DesiredCapabilities dc = new DesiredCapabilities();
- if (!"".equals(remote)) {
+
+ String chrome = TestFlag.CHROME_BINARY.getString(null);
+ if (chrome != null) {
+ dc.setCapability("chrome.binary", chrome);
+ }
+
+ String browserType = getBrowserType();
+ String webdriver = TestFlag.WEBDRIVER_URL.getString("");
+ if (!"".equals(webdriver)) {
dc.setBrowserName(browserType);
dc.setJavascriptEnabled(true);
try {
- return new RemoteWebDriver(new URL(remote), dc);
+ return new RemoteWebDriver(new URL(webdriver), dc);
} catch (MalformedURLException e) {
throw new RuntimeException(e);
}
- }
- if ("chrome".equals(browserType)) {
- if (browserPath != null) {
- dc.setCapability("chrome.binary", browserPath);
- }
+ } else if ("chrome".equals(browserType)) {
return new ChromeDriver(dc);
} else if ("firefox".equals(browserType)) {
return new FirefoxDriver();
=======================================
--- /trunk/tests/com/google/caja/util/CajaTestCase.java Tue Jan 22 15:38:01
2013
+++ /trunk/tests/com/google/caja/util/CajaTestCase.java Mon Apr 22 10:43:32
2013
@@ -496,14 +496,14 @@
@Override
protected void runTest() throws Throwable {
// Support filtering of test methods via the Java system property
- // "test.method.filter". This can be used in conjunction with
+ // "test.filter.method". This can be used in conjunction with
// "test.filter".
- String filterGlob = System.getProperty("test.method.filter");
+ String filterGlob = System.getProperty("test.filter.method");
if (filterGlob != null) {
// TODO: Maybe move globToPattern into util.
Pattern methodFilter = Pattern.compile(
AllTests.globToPattern(filterGlob), Pattern.DOTALL);
- if (!methodFilter.matcher(getName()).find()) {
+ if (!methodFilter.matcher(getName()).matches()) {
System.err.println("Skipping " + getName());
return;
}
=======================================
--- /trunk/tests/com/google/caja/util/LocalServer.java Mon Apr 8 18:32:43
2013
+++ /trunk/tests/com/google/caja/util/LocalServer.java Mon Apr 22 10:43:32
2013
@@ -43,10 +43,6 @@
public LocalServer(ConfigureContextCallback contextCallback) {
this.contextCallback = contextCallback;
}
-
- public String hostname() {
- return ThisHostName.value();
- }
public int getPort() {
return server.getConnectors()[0].getLocalPort();
=======================================
--- /trunk/tests/com/google/caja/util/ThisHostName.java Fri Nov 23 09:27:38
2012
+++ /trunk/tests/com/google/caja/util/ThisHostName.java Mon Apr 22 10:43:32
2013
@@ -27,9 +27,6 @@
/**
* Try to return a non-loopback hostname for this host,
* which other hosts can use to contact it.
- *
- * System property "caja.test.thishostname" will override
- * the auto-discovery.
*/
public static String value() {
if (_value == null) {
@@ -39,11 +36,6 @@
}
private static String computeValue() {
- String prop = System.getProperty("caja.test.thishostname");
- if (prop != null) {
- return prop;
- }
-
InetAddress localhost;
try {
localhost = InetAddress.getLocalHost();
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
