Curiosly Correct

Experienced Java programmers will quickly recognize that the following is not how to check for String equality:

public class StringToConstantCompareDemo {
    public static final String SOME_CONSTANT = "Hello";

    public int compareStringToConstant(String input) {
        if (input == SOME_CONSTANT) {
            return 1;
        } else {
            return 0;
        }
    }
}

Instead this example checked if input is a reference to the same object as SOME_CONSTANT. Or as stated in the Java Language Specification:

“While == may be used to compare references of type String, such an equality test determines whether or not the two operands refer to the same String object. The result is false if the operands are distinct String objects, even if they contain the same sequence of characters. The contents of two strings s and t can be tested for equality by the method invocation s.equals(t).”

The code should have been:

public int compareStringToConstant(String input) {
    if (SOME_CONSTANT.equals(input)) {
        return 1;
    } else {
        return 0;
    }
}

Okay, let’s say we know not to put an == for String comparison. We sometimes make mistakes though. One way to protect yourself from mistakes is to write unit tests.

What might a unit test for this function look like?

public class StringCompareTest {
    @Test
    public void stringIsEqualsToHello() {
        StringToConstantCompareDemo demo = new StringToConstantCompareDemo();
        Assert.assertEquals(1, demo.compareStringToConstant(
                           StringToConstantCompareDemo.SOME_CONSTANT));
    }
}

If we ran this test, it would still pass with the == in the method. Why? Because we passed it a reference to SOME_CONSTANT. Let’s try again:

@Test
public void stringsAreEqual() {
    StringToConstantCompareDemo demo = new StringToConstantCompareDemo();
    Assert.assertEquals(1, demo.compareStringToConstant("Hello"));
}

Hmm…. it still passes. That seems a little odd. Let’s look at why.

During compilation, String constants of the same value are “interned”, or made to reference the same object in memory. See this section of the Java Language Specification for more details. This optimization helps preserve memory, but it foils the intent of our test case.

However, there’s one more thing we can try in our test case:

@Test
public void stringsAreEqual() {
    StringToConstantCompareDemo demo = new StringToConstantCompareDemo();
    Assert.assertEquals(1, demo.compareStringToConstant(new String("Hello")));
}

Now we’ve got it. With the == in the method, the test case fails and with .equals the test case passes.

The problem here is that if I knew about this behavior and thought to write new String(…) in my test case, I probably didn’t compare the String with == in the first place! Therefore, it’s unlikely that I would have ever found this bug with a unit test.

An alternative is to use a static code analyzer, such as PMD or FindBugs, which can locate this issue and many others. You shouldn’t expect a static analyzer to replace rigorous unit testing. They can only look for common programming mistakes that would be suspicious in any program. They cannot validate the logic of your code. However, unit testing and static analysis combined can detect a wider range of problems than either one alone.

Many use the Cargo Maven plugin to manage a web container during the build process. It allows you to install the container, deploy your application, start the container and then stop the container at some later point. Here are the relevant fragments of a typical Maven POM file which you might use to automatically start Apache Tomcat for running integrated tests:

[...]

<plugin>
  <groupId>org.codehaus.cargo</groupId>
  <artifactId>cargo-maven2-plugin</artifactId>
  <executions>
    <execution>
      <id>install</id>
      <goals>
        <goal>install</goal>
      </goals>
    </execution>
    <execution>
      <id>start-container</id>
      <phase>pre-integration-test</phase>
      <goals>
        <goal>start</goal>
      </goals>
      <configuration>
        <wait>false</wait>
      </configuration>
    </execution>
    <execution>
      <id>stop-container</id>
      <phase>post-integration-test</phase>
      <goals>
        <goal>stop</goal>
      </goals>
    </execution>
  </executions>
  <configuration>
    <container>
      <containerId>tomcat6x</containerId>
      <zipUrlInstaller>
        <url>${tomcat.distribution.url}</url>
        <installDir>${tomcat.install.dir}</installDir>
      </zipUrlInstaller>
    </container>
    <configuration>
      <home>${tomcat.home.dir}</home>
      <properties>
        <cargo.logging>high</cargo.logging>
        <cargo.servlet.port>8080</cargo.servlet.port>
        <cargo.tomcat.ajp.port>8009</cargo.tomcat.ajp.port>
      </properties>
    </configuration>
  </configuration>
</plugin>
<plugin>
  <groupId>org.codehaus.mojo</groupId>
  <artifactId>failsafe-maven-plugin</artifactId>
  <version>2.4.3-alpha-1</version>
  <configuration>
    <includes>
      <include>**/*Test*.java</include>
    </includes>
  </configuration>
  <executions>
    <execution>
      <goals>
        <goal>integration-test</goal>
        <goal>verify</goal>
      </goals>
    </execution>
  </executions>
</plugin>

[...]

Let’s suppose that your application publishes MBeans for JMX . You may have integration test cases which connect to your application, running in Tomcat, through JMX to read the MBean properties. Therefore, you need to tell Tomcat that it should listen for JMX connections. This is done by modifying the Cargo configuration as follows:

<plugin>
  <groupId>org.codehaus.cargo</groupId>
  <artifactId>cargo-maven2-plugin</artifactId>
  <executions>
    <execution>
      <id>install</id>
      <goals>
        <goal>install</goal>
      </goals>
    </execution>
    <execution>
      <id>start-container</id>
      <phase>pre-integration-test</phase>
      <goals>
        <goal>start</goal>
      </goals>
      <configuration>
        <wait>false</wait>
      </configuration>
    </execution>
    <execution>
      <id>stop-container</id>
      <phase>post-integration-test</phase>
      <goals>
        <goal>stop</goal>
      </goals>
    </execution>
  </executions>
  <configuration>
    <container>
      <containerId>tomcat6x</containerId>
      <zipUrlInstaller>
        <url>${tomcat.distribution.url}</url>
        <installDir>${tomcat.install.dir}</installDir>
      </zipUrlInstaller>
      <systemProperties>
        <java.rmi.server.hostname>127.0.0.1</java.rmi.server.hostname>
        <com.sun.management.jmxremote.port>10600</com.sun.management.jmxremote.port>
        <com.sun.management.jmxremote.ssl>false</com.sun.management.jmxremote.ssl>
        <com.sun.management.jmxremote.authenticate>false</com.sun.management.jmxremote.authenticate>
      </systemProperties>
    </container>
    <configuration>
      <home>${tomcat.home.dir}</home>
      <properties>
        <cargo.logging>high</cargo.logging>
        <cargo.servlet.port>8080</cargo.servlet.port>
        <cargo.tomcat.ajp.port>8009</cargo.tomcat.ajp.port>
      </properties>
    </configuration>
  </configuration>
</plugin>

Note that we’ve added system properties to the container to tell how JMX clients can connect. If you run this however, you will be bit by the following exception when Cargo attempts to stop Tomcat:

[INFO] [cargo:stop {execution: stop-container}]
[INFO] [talledLocalContainer] Tomcat 6.x is stopping...
[WARNING] [talledLocalContainer] Error: Exception thrown by the agent : java.rmi.server.ExportException: Port already in use: 10600; nested
exception is:
[WARNING] [talledLocalContainer]        java.net.BindException: Address already in use: JVM_Bind
[INFO] ------------------------------------------------------------------------
[ERROR] FATAL ERROR
[INFO] ------------------------------------------------------------------------
[INFO] Failed to stop the Tomcat 6.x container.
Deployable [http://localhost:8080/cargocpc/index.html] failed to finish undeploying within the timeout period [120000]. The Deployable state
 is thus unknown.
[INFO] ------------------------------------------------------------------------
[INFO] Trace
org.codehaus.cargo.container.ContainerException: Failed to stop the Tomcat 6.x container.
   at org.codehaus.cargo.container.spi.AbstractLocalContainer.stop(AbstractLocalContainer.java:214)
   at org.codehaus.cargo.maven2.ContainerStopMojo.doExecute(ContainerStopMojo.java:49)
   at org.codehaus.cargo.maven2.AbstractCargoMojo.execute(AbstractCargoMojo.java:268)
   at org.apache.maven.plugin.DefaultPluginManager.executeMojo(DefaultPluginManager.java:483)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoals(DefaultLifecycleExecutor.java:678)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoalWithLifecycle(DefaultLifecycleExecutor.java:540)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoal(DefaultLifecycleExecutor.java:519)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoalAndHandleFailures(DefaultLifecycleExecutor.java:371)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeTaskSegments(DefaultLifecycleExecutor.java:332)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.execute(DefaultLifecycleExecutor.java:181)
   at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:356)
   at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:137)
   at org.apache.maven.cli.MavenCli.main(MavenCli.java:356)
   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
   at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
   at java.lang.reflect.Method.invoke(Method.java:597)
   at org.codehaus.classworlds.Launcher.launchEnhanced(Launcher.java:315)
   at org.codehaus.classworlds.Launcher.launch(Launcher.java:255)
   at org.codehaus.classworlds.Launcher.mainWithExitCode(Launcher.java:430)
   at org.codehaus.classworlds.Launcher.main(Launcher.java:375)
Caused by: org.codehaus.cargo.container.ContainerException: Deployable [http://localhost:8080/cargocpc/index.html] failed to finish undeploy
ing within the timeout period [120000]. The Deployable state is thus unknown.
   at org.codehaus.cargo.container.spi.deployer.DeployerWatchdog.watch(DeployerWatchdog.java:111)
   at org.codehaus.cargo.container.spi.AbstractLocalContainer.waitForCompletion(AbstractLocalContainer.java:239)
   at org.codehaus.cargo.container.spi.AbstractLocalContainer.stop(AbstractLocalContainer.java:208)
   ... 20 more
org.codehaus.cargo.container.ContainerException: Deployable [http://localhost:8080/cargocpc/index.html] failed to finish undeploying within
the timeout period [120000]. The Deployable state is thus unknown.
   at org.codehaus.cargo.container.spi.deployer.DeployerWatchdog.watch(DeployerWatchdog.java:111)
   at org.codehaus.cargo.container.spi.AbstractLocalContainer.waitForCompletion(AbstractLocalContainer.java:239)
   at org.codehaus.cargo.container.spi.AbstractLocalContainer.stop(AbstractLocalContainer.java:208)
   at org.codehaus.cargo.maven2.ContainerStopMojo.doExecute(ContainerStopMojo.java:49)
   at org.codehaus.cargo.maven2.AbstractCargoMojo.execute(AbstractCargoMojo.java:268)
   at org.apache.maven.plugin.DefaultPluginManager.executeMojo(DefaultPluginManager.java:483)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoals(DefaultLifecycleExecutor.java:678)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoalWithLifecycle(DefaultLifecycleExecutor.java:540)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoal(DefaultLifecycleExecutor.java:519)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeGoalAndHandleFailures(DefaultLifecycleExecutor.java:371)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.executeTaskSegments(DefaultLifecycleExecutor.java:332)
   at org.apache.maven.lifecycle.DefaultLifecycleExecutor.execute(DefaultLifecycleExecutor.java:181)
   at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:356)
   at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:137)
   at org.apache.maven.cli.MavenCli.main(MavenCli.java:356)
   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
   at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
   at java.lang.reflect.Method.invoke(Method.java:597)
   at org.codehaus.classworlds.Launcher.launchEnhanced(Launcher.java:315)
   at org.codehaus.classworlds.Launcher.launch(Launcher.java:255)
   at org.codehaus.classworlds.Launcher.mainWithExitCode(Launcher.java:430)
   at org.codehaus.classworlds.Launcher.main(Launcher.java:375)
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2 minutes 29 seconds
[INFO] Finished at: Mon Jan 10 14:55:57 EST 2011
[INFO] Final Memory: 16M/371M
[INFO] ------------------------------------------------------------------------

This is because when Cargo attempts to stop Tomcat it launches a new JVM. This new JVM is configured with the same system properties as were used to start Tomcat. Which means it tries to start a second JVM with the same JMX ports.

In order to fix this problem, I’m going to take out the command to stop Tomcat from Cargo and then add an Ant task that closes Tomcat:

<plugin>
  <groupId>org.codehaus.cargo</groupId>
  <artifactId>cargo-maven2-plugin</artifactId>
  <executions>
    <execution>
      <id>install</id>
      <goals>
        <goal>install</goal>
      </goals>
    </execution>
    <execution>
      <id>start-container</id>
      <phase>pre-integration-test</phase>
      <goals>
        <goal>start</goal>
      </goals>
      <configuration>
        <wait>false</wait>
      </configuration>
    </execution>
    <!-- This is where I intentionally removed the Cargo stop configuration -->
  </executions>

  [...]

<!-- This is the new Ant task for stopping Tomcat -->
<plugin>
  <artifactId>maven-antrun-plugin</artifactId>
  <version>1.6</version>
  <executions>
    <execution>
      <phase>post-integration-test</phase>
      <configuration>
        <target>
          <java classname="org.apache.catalina.startup.Bootstrap"
            classpath="
              ${tomcat.install.dir}/apache-tomcat-6.0.26/apache-tomcat-6.0.26/lib/*;
              ${tomcat.install.dir}/apache-tomcat-6.0.26/apache-tomcat-6.0.26/bin/bootstrap.jar;
              ${java.home}/lib/tools.jar"
            fork="true"
            dir="${tomcat.install.dir}/apache-tomcat-6.0.26/apache-tomcat-6.0.26/bin">
            <sysproperty key="catalina.home"
               value="${tomcat.install.dir}/apache-tomcat-6.0.26/apache-tomcat-6.0.26" />
            <sysproperty key="catalina.base"
               value="${tomcat.home.dir}" />
            <sysproperty key="java.io.tmpdir"
               value="${tomcat.home.dir}/temp" />
            <arg value="stop" />
          </java>
          <sleep seconds="10" />
        </target>
      </configuration>
      <goals>
        <goal>run</goal>
      </goals>
    </execution>
  </executions>
</plugin>

You might have noticed that there’s an Ant task in there to sleep. I was doing something immediately after this in my build which needed Tomcat to be fully shutdown and putting a wait in there seemed to help. Ten seconds may actually be a bit excessive, and you might not need it at all.