TestEndpointReachable: incorporate review comments PR #142

test-framework/src/main/java/org/apache/brooklyn/test/framework/TestEffectorImpl.java

@@ -101,7 +101,7 @@ public String get() {
      * {@inheritDoc}
      */
     public void stop() {
-        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
+        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPED);
         sensors().set(SERVICE_UP, false);
     }
 

test-framework/src/main/java/org/apache/brooklyn/test/framework/TestEndpointReachable.java

@@ -51,12 +51,7 @@
  * the target entity):
  * <pre>
  * {@code
- * services:
- * - type: MyEntityUnderTest
- *   id: entity-under-test
- * - type: org.apache.brooklyn.test.framework.TestCase
- *   name: Tests
- *   brooklyn.children:
+ *   ...
  *   - type: org.apache.brooklyn.test.framework.TestEndpointReachable
  *     name: Endpoint reachable
  *     brooklyn.config:
@@ -69,6 +64,24 @@
  *         - $brooklyn:entity("entity-under-test").attributeWhenReady("https.port")
  * }
  * </pre>
+ * 
+ * One can also assert that the given endpoint is not reachable. Here the timeout means that at 
+ * some point within this timeout period, we expect the endpoint to become unreachable. As soon
+ * as it is unreachable, we return:
+ * 
+ * <pre>
+ * {@code
+ *   ...
+ *   - type: org.apache.brooklyn.test.framework.TestEndpointReachable
+ *     name: Endpoint reachable
+ *     brooklyn.config:
+ *       targetId: entity-under-test
+ *       timeout: 2m
+ *       endpointSensor: datastore.url
+ *       assertions:
+ *         reachable: false
+ * }
+ * </pre>
  */
 @ImplementedBy(value = TestEndpointReachableImpl.class)
 public interface TestEndpointReachable extends BaseTest {
@@ -81,4 +94,10 @@ public interface TestEndpointReachable extends BaseTest {
             Object.class,
             "endpointSensor", 
             "Sensor (or name of sensor) on target that advertises the endpoint (to test for tcp-reachability); mutually exclusive with 'endpoint'");
+
+    /**
+     * A key within the assertions map, to say whether we should assert that the endpoint is reachable or not reachable.
+     * The value in the map should be a boolean. If absent, defaults to true.
+     */
+    public static final String REACHABLE_KEY = "reachable";
 }

test-framework/src/main/java/org/apache/brooklyn/test/framework/TestEndpointReachableImpl.java

@@ -25,6 +25,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.location.Location;
@@ -34,19 +35,21 @@
 import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.test.Asserts;
-import org.apache.brooklyn.test.framework.TestFrameworkAssertions.AssertionSupport;
-import org.apache.brooklyn.test.framework.TestHttpCall.HttpAssertionTarget;
+import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.http.HttpTool;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.net.Networking;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Objects;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import com.google.common.net.HostAndPort;
 
 /**
@@ -65,6 +68,8 @@ public void start(Collection<? extends Location> locations) {
         final String endpoint = getConfig(ENDPOINT);
         final Object endpointSensor = getConfig(ENDPOINT_SENSOR);
         final Duration timeout = getConfig(TIMEOUT);
+        final List<Map<String, Object>> assertions = getAssertions(this, ASSERTIONS);
+
         final Entity target = resolveTarget();
 
         if (endpoint == null && endpointSensor == null) {
@@ -101,7 +106,7 @@ public HostAndPort get() {
                 public void run() {
                     HostAndPort val = supplier.get();
                     Asserts.assertNotNull(val);
-                    Asserts.assertTrue(Networking.isReachable(val), val+" not reachable");
+                    assertSucceeds(assertions, val);
                 }});
             sensors().set(Attributes.SERVICE_UP, true);
             ServiceStateLogic.setExpectedState(this, Lifecycle.RUNNING);
@@ -114,6 +119,44 @@ public void run() {
         }
     }
 
+    protected void assertSucceeds(List<Map<String, Object>> assertions, HostAndPort endpoint) {
+        Maybe<Object> checkReachableMaybe = getOnlyAssertionsValue(assertions, REACHABLE_KEY);
+        boolean checkReachable = checkReachableMaybe.isAbsentOrNull() || Boolean.TRUE.equals(TypeCoercions.coerce(checkReachableMaybe.get(), Boolean.class));
+        boolean reachable = Networking.isReachable(endpoint);
+        Asserts.assertEquals(reachable, checkReachable, endpoint+" "+(reachable ? "" : "not ")+"reachable");
+    }
+
+    /**
+     * Finds the value for the given key in one of the maps (or {@link Maybe#absent()} if not found).
+     * 
+     * @throws IllegalArgumentException if multiple conflicts values for the key, or if there are other (unexpected) keys.
+     */
+    protected Maybe<Object> getOnlyAssertionsValue(List<Map<String, Object>> assertions, String key) {
+        Maybe<Object> result = Maybe.absent();
+        Set<String> keys = Sets.newLinkedHashSet();
+        boolean foundConflictingDuplicate = false;
+        if (assertions != null) {
+            for (Map<String, Object> assertionMap : assertions) {
+                if (assertionMap.containsKey(key)) {
+                    Object val = assertionMap.get(REACHABLE_KEY);
+                    if (result.isPresent() && !Objects.equal(result.get(), val)) {
+                        foundConflictingDuplicate = true;
+                    } else {
+                        result = Maybe.of(val);
+                    }
+                }
+                keys.addAll(assertionMap.keySet());
+            }
+        }
+        Set<String> unhandledKeys = Sets.difference(keys, ImmutableSet.of(key));
+        if (foundConflictingDuplicate) {
+            throw new IllegalArgumentException("Multiple conflicting values for assertion '"+key+"' in "+this);
+        } else if (unhandledKeys.size() > 0) {
+            throw new IllegalArgumentException("Unknown assertions "+unhandledKeys+" in "+this);
+        }
+        return result;
+    }
+
     protected HostAndPort toHostAndPort(Object endpoint) {
         if (endpoint == null) {
             throw new IllegalArgumentException(String.format("The entity [%s] has no endpoint", getClass().getName()));
@@ -164,7 +207,7 @@ protected HostAndPort toHostAndPort(String endpoint) {
      * {@inheritDoc}
      */
     public void stop() {
-        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
+        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPED);
         sensors().set(Attributes.SERVICE_UP, false);
     }
 

test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java

@@ -107,7 +107,7 @@ public Integer get() {
      * {@inheritDoc}
      */
     public void stop() {
-        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
+        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPED);
         sensors().set(Attributes.SERVICE_UP, false);
     }
 

test-framework/src/main/java/org/apache/brooklyn/test/framework/TestSensorImpl.java

@@ -86,7 +86,7 @@ public Object get() {
      * {@inheritDoc}
      */
     public void stop() {
-        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
+        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPED);
         sensors().set(SERVICE_UP, false);
     }
 

test-framework/src/test/java/org/apache/brooklyn/test/framework/TestEndpointReachableTest.java

@@ -34,7 +34,6 @@
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation;
 import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.javalang.JavaClassNames;
 import org.apache.brooklyn.util.net.Networking;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
@@ -45,6 +44,7 @@
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.net.HostAndPort;
 
 public class TestEndpointReachableTest extends BrooklynAppUnitTestSupport {
@@ -138,6 +138,50 @@ public void testSensorUrlReachable() throws Exception {
         app.start(ImmutableList.of(loc));
     }
 
+    @Test
+    public void testHardcodedEndpointReachableWithExplicitAssertionForReachable() throws Exception {
+        app.createAndManageChild(EntitySpec.create(TestEndpointReachable.class)
+                .configure(TestEndpointReachable.TARGET_ENTITY, app)
+                .configure(TestEndpointReachable.ENDPOINT, serverSocketHostAndPort.toString())
+                .configure(TestEndpointReachable.ASSERTIONS, ImmutableMap.of(TestEndpointReachable.REACHABLE_KEY, "true")));
+
+        app.start(ImmutableList.of(loc));
+    }
+
+    @Test
+    public void testExplicitAssertionForNotReachableWhenReachable() throws Exception {
+        TestEndpointReachable test = app.createAndManageChild(EntitySpec.create(TestEndpointReachable.class)
+                .configure(TestEndpointReachable.TARGET_ENTITY, app)
+                .configure(TestEndpointReachable.ENDPOINT, serverSocketHostAndPort.toString())
+                .configure(TestEndpointReachable.TIMEOUT, Duration.millis(100))
+                .configure(TestEndpointReachable.ASSERTIONS, ImmutableMap.of(TestEndpointReachable.REACHABLE_KEY, "false")));
+
+        try {
+            app.start(ImmutableList.of(loc));
+        } catch (Exception e) {
+            // It should fail to start
+            Exceptions.propagateIfFatal(e);
+            LOG.debug("As desired, failed to start app with TestEndpointReachable: "+e.toString());
+        }
+        EntityAsserts.assertAttributeEqualsEventually(test, TestEndpointReachable.SERVICE_UP, false);
+        EntityAsserts.assertAttributeEqualsEventually(test, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.ON_FIRE);
+    }
+
+    // Not a normal unit-test, because other processes on shared infrastructure can grab the port between
+    // us identifying it is free and us checking the reachability.
+    @Test(groups="Integration")
+    public void testExplicitAssertionForNotReachableWhenNotReachable() throws Exception {
+        HostAndPort unusedPort = findUnusedPort();
+        TestEndpointReachable test = app.createAndManageChild(EntitySpec.create(TestEndpointReachable.class)
+                .configure(TestEndpointReachable.TARGET_ENTITY, app)
+                .configure(TestEndpointReachable.ENDPOINT, unusedPort.toString())
+                .configure(TestEndpointReachable.ASSERTIONS, ImmutableMap.of(TestEndpointReachable.REACHABLE_KEY, "false")));
+
+        app.start(ImmutableList.of(loc));
+        EntityAsserts.assertAttributeEqualsEventually(test, TestEndpointReachable.SERVICE_UP, true);
+        EntityAsserts.assertAttributeEqualsEventually(test, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.RUNNING);
+    }
+
     // Not a normal unit-test, because other processes on shared infrastructure can grab the port between
     // us identifying it is free and us checking the reachability.
     @Test(groups="Integration")
@@ -182,23 +226,9 @@ public void testSensorEndpointNotReachable() throws Exception {
         EntityAsserts.assertAttributeEqualsEventually(test, Attributes.SERVICE_STATE_ACTUAL, Lifecycle.ON_FIRE);
     }
 
-    protected ServerSocket openServerPort() {
-        int startPort = 58767;
-        int endPort = 60000;
-        int port = startPort;
+    protected ServerSocket openServerPort() throws IOException {
         InetAddress localAddress = Networking.getLocalHost();
-        do {
-            try {
-                ServerSocket ss = new ServerSocket(port, 1024, localAddress);
-                LOG.info("acquired server-socket on port "+port+" for test "+JavaClassNames.niceClassAndMethod());
-                return ss;
-            } catch (IOException e) {
-                // try next port
-                port++;
-            }
-        } while (port <= endPort);
-        Assert.fail("Could not open any local port in range "+startPort+"-"+endPort);
-        throw new IllegalStateException("Impossible code to reach");
+        return new ServerSocket(0, 1024, localAddress);
     }
 
     protected HostAndPort findUnusedPort() {