Mailing-List: contact dev-help@brooklyn.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@brooklyn.incubator.apache.org
From: ahgittin <git@git.apache.org>
To: dev@brooklyn.incubator.apache.org
Reply-To: dev@brooklyn.incubator.apache.org
References: <git-pr-999-incubator-brooklyn@git.apache.org>
In-Reply-To: <git-pr-999-incubator-brooklyn@git.apache.org>
Subject: [GitHub] incubator-brooklyn pull request: Implementation of proposed
 Test E...
Content-Type: text/plain
Message-Id: <20151111093846.39C25E03A9@git1-us-west.apache.org>
Date: Wed, 11 Nov 2015 09:38:46 +0000 (UTC)

Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/999#discussion_r44515581
  
    --- Diff: usage/test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java ---
    @@ -0,0 +1,107 @@
    +package org.apache.brooklyn.test.framework;
    +
    +import com.google.api.client.util.Objects;
    +import com.google.common.collect.ImmutableMap;
    +import com.google.common.collect.Lists;
    +import org.apache.brooklyn.api.location.Location;
    +import org.apache.brooklyn.core.entity.AbstractEntity;
    +import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
    +import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
    +import org.apache.brooklyn.util.core.flags.TypeCoercions;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Collection;
    +import java.util.Map;
    +
    +import static org.apache.brooklyn.util.http.HttpAsserts.assertContentEventuallyContainsText;
    +import static org.apache.brooklyn.util.http.HttpAsserts.assertContentEventuallyMatches;
    +import static org.apache.brooklyn.util.http.HttpAsserts.assertHttpStatusCodeEventuallyEquals;
    +
    +/**
    + * {@inheritDoc}
    + */
    +public class TestHttpCallImpl extends AbstractEntity implements TestHttpCall {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(TestHttpCallImpl.class);
    +
    +    /**
    +     * {@inheritDoc}
    +     */
    +    public void start(Collection<? extends Location> locations) {
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);
    +        final String url = getConfig(TARGET_URL);
    +        final Map assertions = getConfig(ASSERTIONS);
    +        final Duration timeout = getConfig(TIMEOUT);
    +        try {
    +            checkAssertions(url.toString(), ImmutableMap.of("timeout", timeout), assertions);
    +            sensors().set(SERVICE_UP, true);
    +            ServiceStateLogic.setExpectedState(this, Lifecycle.RUNNING);
    +        } catch (Throwable t) {
    +            LOG.info("Url [{}] test failed", url);
    +            sensors().set(SERVICE_UP, false);
    +            ServiceStateLogic.setExpectedState(this, Lifecycle.ON_FIRE);
    +            throw Exceptions.propagate(t);
    +        }
    +    }
    +
    +    /**
    +     * Tests HTTP Request reponse matches assertions
    +     * <p>
    +     * Supported keys in the <code>assertions</code> {@link Map} include:
    +     * <ul>
    +     * <li>string - simple string match
    +     * <li>regex - uses {@link java.lang.String#matches(String)}, if the url returns a multi-line response you should
    +     * use the embedded dotall flag expression <code>(?s)</code> in your regex.
    +     * <li>status - HTTP status code
    +     * </ul>
    +     * Wraps the {@link org.apache.brooklyn.util.http.HttpAsserts} immediate assertion methods.
    +     * <p>
    +     * See the test/resources directory for examples.
    +     *
    +     * @param url        The target URL to be tested
    +     * @param flags      Passed to {@link org.apache.brooklyn.util.http.HttpAsserts#assertContentEventuallyContainsText(Map, String, String, String...)},
    +     *                   {@link org.apache.brooklyn.util.http.HttpAsserts#assertContentEventuallyMatches(Map, String, String)},
    +     *                   {@link org.apache.brooklyn.util.http.HttpAsserts#assertHttpStatusCodeEventuallyEquals(Map, String, int)}
    +     *
    +     * @param assertions The map of assertions
    +     */
    +    private void checkAssertions(final String url, final Map<String,?> flags, final Map<?, ?> assertions) {
    +
    +        for (final Map.Entry<?, ?> entry : assertions.entrySet()) {
    --- End diff --
    
    this kind of map feels like the wrong syntax as it prevents multiple tests of the same type (e.g. multiple `regex` checks)
    
    i think we should take a list of maps, where the maps should be single-entry.  (if a map is provided we could support the current implementation but with a warning.)  this would also allow us to define more complex assertions which might require multiple settings (e.g. `ignoreCase` for `string` (aka `contains`)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---