Refactor SvnInfo to make it testable; also makes the branch relative path to the root of the repository.

Before the branch would be set only to the tail, e.g. "1.6".
Now, the branch is set to "releases/1.6".

Review by: fabbott


git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@5007 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/build-tools/ant-gwt/build.xml b/build-tools/ant-gwt/build.xml
index e4c986d..68c30a7 100644
--- a/build-tools/ant-gwt/build.xml
+++ b/build-tools/ant-gwt/build.xml
@@ -12,6 +12,16 @@
     </gwt.javac>
   </target>
 
+  <target name="compile.tests" depends="build" description="Compiles the test code for this project">
+    <mkdir dir="${javac.junit.out}" />
+    <gwt.javac srcdir="test" destdir="${javac.junit.out}">
+      <classpath>
+        <pathelement location="${javac.out}" />
+        <pathelement location="${gwt.tools.lib}/junit/junit-3.8.1.jar" />
+      </classpath>
+    </gwt.javac>
+  </target>
+
   <target name="build" depends="compile" description="Packages this project into a jar">
     <mkdir dir="${gwt.build.lib}" />
     <gwt.jar destfile="${gwt.build.lib}/${ant.project.name}.jar">
@@ -20,6 +30,35 @@
     </gwt.jar>
   </target>
 
+  <target name="test" depends="build, compile.tests" description="Run unit tests for this project.">
+    <!-- TODO: refactor gwt.junit so it can be reused here -->
+    <taskdef name="junit" classname="org.apache.tools.ant.taskdefs.optional.junit.JUnitTask">
+      <classpath>
+        <pathelement location="${gwt.tools.lib}/junit/junit-3.8.1.jar" />
+        <pathelement location="${gwt.tools.antlib}/ant-junit-1.6.5.jar" />
+      </classpath>
+    </taskdef>
+  
+    <echo message="Writing test results to ${junit.out}/reports for ${test.cases}" />
+    <mkdir dir="${junit.out}/reports" />
+  
+    <echo message="${javac.out} ${javac.junit.out}" />
+    <junit dir="${junit.out}" fork="yes" printsummary="yes" haltonfailure="true">
+      <classpath>
+        <pathelement location="${javac.junit.out}" />
+        <pathelement location="${javac.out}" />
+        <pathelement location="${gwt.tools.lib}/junit/junit-3.8.1.jar" />
+      </classpath>
+  
+      <formatter type="plain" />
+      <formatter type="xml" />
+  
+      <batchtest todir="${junit.out}/reports">
+        <fileset dir="${javac.junit.out}" includes="**/*Test.class" />
+      </batchtest>
+    </junit>
+  </target>
+
   <target name="checkstyle" description="Static analysis of source">
     <gwt.checkstyle>
       <fileset dir="src" />
diff --git a/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/CommandRunner.java b/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/CommandRunner.java
new file mode 100644
index 0000000..ff47993
--- /dev/null
+++ b/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/CommandRunner.java
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2009 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.gwt.ant.taskdefs;
+
+import org.apache.tools.ant.BuildException;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.LineNumberReader;
+
+/**
+ * Utility class to run external commands.
+ */
+class CommandRunner {
+
+  /**
+   * Returns the output of running a command as a string. Will fail if the
+   * invoked process returns a non-zero status code.
+   */
+  public static String getCommandOutput(File workDir, String... cmd) {
+    Process process = runCommandIgnoringErr(workDir, cmd);
+    StringBuilder output = new StringBuilder();
+    LineNumberReader lnr = new LineNumberReader(new InputStreamReader(
+        process.getInputStream()));
+    try {
+      for (String line = lnr.readLine(); line != null; line = lnr.readLine()) {
+        output.append(line);
+        output.append('\n');
+      }
+      int statusCode = process.waitFor();
+      if (statusCode != 0) {
+        throw new BuildException("Non-zero status code result (" + statusCode
+            + ") running command: " + makeCmdString(cmd));
+      }
+      return output.toString();
+    } catch (IOException e) {
+      throw new BuildException("Unable to read command: " + makeCmdString(cmd),
+          e);
+    } catch (InterruptedException e) {
+      throw new BuildException("Interrupted waiting for command: "
+          + makeCmdString(cmd), e);
+    }
+  }
+
+  /**
+   * Runs the specified command and returns the {@link Process}. The caller
+   * must handle both the output and error streams to avoid blocking the
+   * underlying process.
+   */
+  public static Process runCommand(File workDir, String... cmd) {
+    ProcessBuilder pb = new ProcessBuilder(cmd);
+    pb.directory(workDir);
+    try {
+      return pb.start();
+    } catch (IOException e) {
+      throw new BuildException("Unable to launch command: "
+          + makeCmdString(cmd), e);
+    }
+  }
+
+  /**
+   * Runs the specified command and returns the {@link Process}. The resulting
+   * process's error stream will be continually drained in a daemon thread to
+   * prevent the underlying process from blocking. The caller must handle both
+   * output stream to avoid blocking the underlying process.
+   */
+  public static Process runCommandIgnoringErr(File workDir, String... cmd) {
+    final Process process = runCommand(workDir, cmd);
+    // Consume error output on another thread to avoid blocking.
+    Thread errThread = new Thread(new Runnable() {
+      public void run() {
+        InputStream errorStream = process.getErrorStream();
+        try {
+          byte[] buf = new byte[8192];
+          int read;
+          do {
+            read = errorStream.read(buf);
+          } while (read >= 0);
+        } catch (IOException e) {
+        } finally {
+          try {
+            errorStream.close();
+          } catch (IOException e) {
+          }
+        }
+      }
+    });
+    errThread.setDaemon(true);
+    errThread.start();
+    return process;
+  }
+
+  /**
+   * Turns a command array into a printable string.
+   */
+  static String makeCmdString(String... cmd) {
+    StringBuilder sb = new StringBuilder();
+    for (String arg : cmd) {
+      if (sb.length() > 0) {
+        sb.append(' ');
+      }
+      sb.append(arg);
+    }
+    String cmdString = sb.toString();
+    return cmdString;
+  }
+}
diff --git a/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/SvnInfo.java b/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/SvnInfo.java
index f326367..706fc54 100644
--- a/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/SvnInfo.java
+++ b/build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/SvnInfo.java
@@ -20,8 +20,8 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.LineNumberReader;
+import java.io.StringReader;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -32,9 +32,137 @@
  */
 public class SvnInfo extends Task {
 
-  // URL line from svn info output, selecting the very last term of the URL as
-  // the branch specifier
-  private static final String URL_REGEX = "\\s*URL:\\s*https?://.*/([^/]*)\\s*";
+  /**
+   * Structured svn info.
+   */
+  static class Info {
+    /**
+     * The relative path of this svn working copy within the root of the remote
+     * repository. That is, if the root is "http://example.com/svn" and the
+     * working copy URL is "http://example.com/svn/tags/w00t", this will be set
+     * to "tags/w00t".
+     */
+    public final String branch;
+
+    /**
+     * The revision of this working copy. Initially set to the value parsed from
+     * "svn info" given a more detailed value via "svnversion".
+     */
+    public String revision;
+
+    public Info(String branch, String revision) {
+      this.branch = branch;
+      this.revision = revision;
+    }
+  }
+
+  /**
+   * A regex that matches a URL.
+   */
+  static final String URL_REGEX = "\\w+://\\S*";
+
+  /**
+   * A pattern that matches the URL line in svn info output.
+   */
+  private static final Pattern BRANCH_PATTERN = Pattern.compile("\\s*URL:\\s*("
+      + URL_REGEX + ")\\s*");
+
+  /**
+   * A pattern that matches the Revision line in svn info output.
+   */
+  private static final Pattern REVISION_PATTERN = Pattern.compile("\\s*Revision:\\s*(\\d+)\\s*");
+
+  /**
+   * A pattern that matches the Repository Root line in svn info output.
+   */
+  private static final Pattern ROOT_PATTERN = Pattern.compile("\\s*Repository Root:\\s*("
+      + URL_REGEX + ")\\s*");
+
+  /**
+   * Runs "svn info", returning the output as a string.
+   */
+  static String getSvnInfo(File dir) {
+    String output = CommandRunner.getCommandOutput(dir, "svn", "info");
+    if (output.length() == 0) {
+      throw new BuildException("svn info didn't give any answer");
+    }
+    return output;
+  }
+
+  /**
+   * Runs "svnversion", returning the output as a string.
+   */
+  static String getSvnVersion(File dir) {
+    String output = CommandRunner.getCommandOutput(dir, "svnversion", ".");
+    output = output.trim();
+    if (output.length() == 0) {
+      throw new BuildException("svnversion didn't give any answer");
+    }
+    return output;
+  }
+
+  /**
+   * Returns <code>true</code> if the specified directory looks like an svn
+   * working copy.
+   */
+  static boolean looksLikeSvn(File dir) {
+    return new File(dir, ".svn").isDirectory();
+  }
+
+  /**
+   * Parses the output of running "svn info".
+   */
+  static Info parseInfo(String svnInfo) {
+    String rootUrl = null;
+    String branchUrl = null;
+    String revision = null;
+    LineNumberReader lnr = new LineNumberReader(new StringReader(svnInfo));
+    try {
+      for (String line = lnr.readLine(); line != null; line = lnr.readLine()) {
+        Matcher m;
+        if ((m = ROOT_PATTERN.matcher(line)) != null && m.matches()) {
+          rootUrl = m.group(1);
+        } else if ((m = BRANCH_PATTERN.matcher(line)) != null && m.matches()) {
+          branchUrl = m.group(1);
+        } else if ((m = REVISION_PATTERN.matcher(line)) != null && m.matches()) {
+          revision = m.group(1);
+        }
+      }
+    } catch (IOException e) {
+      throw new BuildException("Should never happen", e);
+    }
+
+    if (rootUrl == null) {
+      throw new BuildException("svn info didn't get root URL: " + svnInfo);
+    }
+    if (branchUrl == null) {
+      throw new BuildException("svn info didn't get branch URL: " + svnInfo);
+    }
+    if (revision == null) {
+      throw new BuildException("svn info didn't get revision: " + svnInfo);
+    }
+    rootUrl = removeTrailingSlash(rootUrl);
+    branchUrl = removeTrailingSlash(branchUrl);
+    if (!branchUrl.startsWith(rootUrl)) {
+      throw new BuildException("branch URL (" + branchUrl + ") and root URL ("
+          + rootUrl + ") did not match");
+    }
+
+    String branch;
+    if (branchUrl.length() == rootUrl.length()) {
+      branch = "";
+    } else {
+      branch = branchUrl.substring(rootUrl.length() + 1);
+    }
+    return new Info(branch, revision);
+  }
+
+  static String removeTrailingSlash(String url) {
+    if (url.endsWith("/")) {
+      return url.substring(0, url.length() - 1);
+    }
+    return url;
+  }
 
   private String fileprop;
 
@@ -59,24 +187,21 @@
     if (!workDirFile.isDirectory()) {
       throw new BuildException(workdir + " is not a directory");
     }
-    
-    String branch;
-    String revision;
 
-    File svnDirFile = new File(workdir, ".svn");
-    if (!svnDirFile.exists()) {
-      // This is not svn workdir. We can't guess the version... 
-      branch = "unknown";
-      revision = "unknown";
+    Info info;
+    if (!looksLikeSvn(workDirFile)) {
+      info = new Info("unknown", "unknown");
     } else {
-      branch = getSvnBranch(workDirFile);
-      revision = getSvnVersion(workDirFile);
+      info = parseInfo(getSvnInfo(workDirFile));
+
+      // Use svnversion to get a more exact revision string.
+      info.revision = getSvnVersion(workDirFile);
     }
 
-    getProject().setNewProperty(outprop, branch + "@" + revision);
+    getProject().setNewProperty(outprop, info.branch + "@" + info.revision);
     if (fileprop != null) {
       getProject().setNewProperty(fileprop,
-          branch + "-" + revision.replaceAll(":", "-"));
+          info.branch + "-" + info.revision.replaceAll(":", "-"));
     }
   }
 
@@ -107,86 +232,4 @@
   public void setOutputProperty(String propname) {
     outprop = propname;
   }
-
-  private String getSvnBranch(File workdir) {
-    String branchName = null;
-
-    LineNumberReader svnout = runCommand(workdir, "svn", "info");
-    try {
-      String line = svnout.readLine();
-
-      Pattern urlRegex = Pattern.compile(URL_REGEX);
-      while (line != null) {
-        Matcher m = urlRegex.matcher(line);
-
-        if (m.matches()) {
-          branchName = m.group(1);
-          if (branchName == null || "".equals(branchName)) {
-            throw new BuildException(
-                "svn info didn't get branch from URL line " + line);
-          }
-          break;
-        }
-        line = svnout.readLine();
-      }
-    } catch (IOException e) {
-      throw new BuildException(
-          "<svninfo> cannot read svn info's output stream", e);
-    }
-    return branchName;
-  }
-
-  private String getSvnVersion(File workdir) {
-    String line = null;
-
-    LineNumberReader svnout = runCommand(workdir, "svnversion", ".");
-    try {
-      line = svnout.readLine();
-    } catch (IOException e) {
-      throw new BuildException(
-          "<svninfo> cannot read svnversion's output stream", e);
-    }
-    if (line == null || "".equals(line)) {
-      throw new BuildException("svnversion didn't give any answer");
-    }
-    return line;
-  }
-
-  private LineNumberReader runCommand(File workdir, String... cmd) {
-    String cmdString = "";
-    for (String arg : cmd) {
-      cmdString = cmdString + arg + " ";
-    }
-    cmdString = cmdString.substring(0, cmdString.length() - 1);
-
-    ProcessBuilder svnPb = new ProcessBuilder(cmd);
-    svnPb.directory(workdir);
-    Process svnproc;
-    try {
-      svnproc = svnPb.start();
-    } catch (IOException e) {
-      throw new BuildException("cannot launch command " + cmdString, e);
-    }
-
-    LineNumberReader svnerr = new LineNumberReader(new InputStreamReader(
-        svnproc.getErrorStream()));
-    try {
-      String line = svnerr.readLine();
-      String errorText = "";
-      if (line != null) {
-        while (line != null) {
-          errorText = errorText + "  " + line + "\n";
-          line = svnerr.readLine();
-        }
-        throw new BuildException(cmdString + " returned error output:\n"
-            + errorText);
-      }
-    } catch (IOException e) {
-      throw new BuildException("cannot read error stream from " + cmdString, e);
-    }
-
-    LineNumberReader svnout = new LineNumberReader(new InputStreamReader(
-        svnproc.getInputStream()));
-    return svnout;
-  }
 }
diff --git a/build-tools/ant-gwt/test/com/google/gwt/ant/taskdefs/CommandRunnerTest.java b/build-tools/ant-gwt/test/com/google/gwt/ant/taskdefs/CommandRunnerTest.java
new file mode 100644
index 0000000..89ad78d
--- /dev/null
+++ b/build-tools/ant-gwt/test/com/google/gwt/ant/taskdefs/CommandRunnerTest.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2009 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.gwt.ant.taskdefs;
+
+import junit.framework.TestCase;
+
+import java.io.File;
+
+/**
+ * Tests for {@link CommandRunner}.
+ */
+public class CommandRunnerTest extends TestCase {
+
+  /**
+   * Test that "java -help" runs successfully.
+   */
+  public void testGetCommandOutput() {
+    String output = CommandRunner.getCommandOutput(new File("."), new File(
+        System.getProperty("java.home"), "bin/java").getAbsolutePath(), "-help");
+    assertNotNull(output);
+  }
+
+  /**
+   * Test that a command array is correctly turned into a printable string.
+   */
+  public void testMakeCmdString() {
+    assertEquals("java", CommandRunner.makeCmdString("java"));
+    assertEquals("java -version", CommandRunner.makeCmdString("java",
+        "-version"));
+  }
+
+}
diff --git a/build-tools/ant-gwt/test/com/google/gwt/ant/taskdefs/SvnInfoTest.java b/build-tools/ant-gwt/test/com/google/gwt/ant/taskdefs/SvnInfoTest.java
new file mode 100644
index 0000000..02ccdb2
--- /dev/null
+++ b/build-tools/ant-gwt/test/com/google/gwt/ant/taskdefs/SvnInfoTest.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright 2009 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.gwt.ant.taskdefs;
+
+import com.google.gwt.ant.taskdefs.SvnInfo.Info;
+
+import junit.framework.TestCase;
+
+import java.io.File;
+
+/**
+ * Tests for {@link SvnInfo}.
+ */
+public class SvnInfoTest extends TestCase {
+
+  /**
+   * The current directory.
+   */
+  private static final File dir = new File(".");
+
+  /**
+   * If this is an svn working copy, just verify that "svn info" succeeds and
+   * returns something.
+   */
+  public void testGetSvnInfo() {
+    if (SvnInfo.looksLikeSvn(dir)) {
+      String info = SvnInfo.getSvnInfo(dir);
+      assertNotNull(info);
+      assertTrue(!"".equals(info));
+    }
+  }
+
+  /**
+   * If this is an svn working copy, just verify that "svnversion" succeeds and
+   * returns something.
+   */
+  public void testGetSvnVersion() {
+    if (SvnInfo.looksLikeSvn(dir)) {
+      String version = SvnInfo.getSvnVersion(dir);
+      assertNotNull(version);
+      assertTrue(!"".equals(version));
+    }
+  }
+
+  /**
+   * Test that the correct info is parsed out of a canned result.
+   */
+  public void testParseInfo() {
+    String svnInfo = "Path: .\n" + "URL: http://example.com/svn/tags/w00t\n"
+        + "Repository Root: http://example.com/svn\n"
+        + "Repository UUID: 00000000-0000-0000-0000-000000000000\n"
+        + "Revision: 9999\n" + "Node Kind: directory\n" + "Schedule: normal\n"
+        + "Last Changed Author: foo@example.com\n" + "Last Changed Rev: 8888\n"
+        + "Last Changed Date: 2009-01-01 00:00:00 +0000 (Thu, 01 Jan 2009)\n";
+    Info info = SvnInfo.parseInfo(svnInfo);
+    assertEquals("tags/w00t", info.branch);
+    assertEquals("9999", info.revision);
+  }
+
+  /**
+   * Test that trailing slashes are removed correctly.
+   */
+  public void testRemoveTrailingSlash() {
+    assertEquals("http://example.com/svn",
+        SvnInfo.removeTrailingSlash("http://example.com/svn"));
+    assertEquals("http://example.com/svn",
+        SvnInfo.removeTrailingSlash("http://example.com/svn/"));
+  }
+}
diff --git a/eclipse/build-tools/ant-gwt/.classpath b/eclipse/build-tools/ant-gwt/.classpath
index bf8a224..56c3270 100644
--- a/eclipse/build-tools/ant-gwt/.classpath
+++ b/eclipse/build-tools/ant-gwt/.classpath
@@ -1,8 +1,9 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <classpath>
 	<classpathentry kind="src" path="core/src"/>
+	<classpathentry kind="src" path="core/test"/>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
 	<classpathentry kind="var" path="GWT_TOOLS/lib/apache/ant-1.6.5.jar" sourcepath="/GWT_TOOLS/lib/apache/ant-1.6.5-src.zip"/>
-	<classpathentry kind="var" path="GWT_TOOLS/lib/tomcat/ant-launcher-1.6.5.jar"/>
+	<classpathentry exported="true" kind="var" path="GWT_TOOLS/lib/junit/junit-3.8.1.jar" sourcepath="/GWT_TOOLS/lib/junit/junit-3.8.1-src.zip"/>
 	<classpathentry kind="output" path="bin"/>
 </classpath>