Skip to content

Add greaterThan and lessThan test framework assertions #619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asfgit merged 4 commits into from
Apr 6, 2017
Merged

Add greaterThan and lessThan test framework assertions #619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac89fbc
Delete System.out call in XmlSerializerTest.assertSerializeAndDeseria…
Mar 21, 2017
1962dda
Javadoc typo
Mar 21, 2017
1578088
Simplify test case assertion handling
Apr 5, 2017
c51c69a
Add greaterThan and lessThan test framework assertions
Apr 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion core/src/test/java/org/apache/brooklyn/util/core/xstream/XmlSerializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ public void testIllegalXmlCharacter() throws Exception {
protected void assertSerializeAndDeserialize(Object val) throws Exception {
String xml = serializer.toString(val);
Object result = serializer.fromString(xml);
System.out.println("val="+val+"'; xml="+xml+"; result="+result);
LOG.debug("val="+val+"'; xml="+xml+"; result="+result);
assertEquals(result, val);
}
Expand Down
197 changes: 74 additions & 123 deletions test-framework/src/main/java/org/apache/brooklyn/test/framework/TestFrameworkAssertions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.Callable;
import javax.annotation.Nullable;

import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.config.ConfigKey;
Expand All @@ -44,6 +46,7 @@
import com.google.common.base.Predicates;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.reflect.TypeToken;


Expand All @@ -63,6 +66,8 @@ public class TestFrameworkAssertions {
public static final String IS_EMPTY = "isEmpty";
public static final String NOT_EMPTY = "notEmpty";
public static final String HAS_TRUTH_VALUE = "hasTruthValue";
public static final String GREATER_THAN = "greaterThan";
public static final String LESS_THAN = "lessThan";
public static final String UNKNOWN_CONDITION = "unknown condition";

public static class AssertionOptions {
Expand Down Expand Up @@ -287,140 +292,86 @@ protected static <T> void checkActualAgainstAssertions(AssertionSupport support,
protected static <T> void checkActualAgainstAssertions(Map<String, ?> assertions,
String target, T actual) {
for (Map.Entry<String, ?> assertion : assertions.entrySet()) {
String condition = assertion.getKey().toString();
String condition = assertion.getKey();
Object expected = assertion.getValue();
switch (condition) {

case IS_EQUAL_TO :
case EQUAL_TO :
case EQUALS :
if (null == actual || !actual.equals(expected)) {
failAssertion(target, condition, expected, actual);
}
break;

case NOT_EQUAL :
if (Objects.equals(actual, expected)) {
failAssertion(target, condition, expected, actual);
}
break;

case IS_NULL :
if (isTrue(expected) != (null == actual)) {
failAssertion(target, condition, expected, actual);
}
break;

case NOT_NULL :
if (isTrue(expected) != (null != actual)) {
failAssertion(target, condition, expected, actual);
}
break;

case CONTAINS :
if (null == actual || !actual.toString().contains(expected.toString())) {
failAssertion(target, condition, expected, actual);
}
break;

case IS_EMPTY :
if (isTrue(expected) != (null == actual || Strings.isEmpty(actual.toString()))) {
failAssertion(target, condition, expected, actual);
}
break;

case NOT_EMPTY :
if (isTrue(expected) != ((null != actual && Strings.isNonEmpty(actual.toString())))) {
failAssertion(target, condition, expected, actual);
}
break;

case MATCHES :
if (null == actual || !actual.toString().matches(expected.toString())) {
failAssertion(target, condition, expected, actual);
}
break;

case HAS_TRUTH_VALUE :
if (isTrue(expected) != isTrue(actual)) {
failAssertion(target, condition, expected, actual);
}
break;

default:
failAssertion(target, UNKNOWN_CONDITION, condition, actual);
if (!knownCondition(condition)) {
failAssertion(target, UNKNOWN_CONDITION, expected, actual);
} else if (!conditionHolds(condition, actual, expected)) {
failAssertion(target, condition, expected, actual);
}
}
}

protected static <T> void checkActualAgainstAbortConditions(Map<String, ?> assertions, String target, T actual) {
for (Map.Entry<String, ?> assertion : assertions.entrySet()) {
String condition = assertion.getKey().toString();
String condition = assertion.getKey();
Object expected = assertion.getValue();
switch (condition) {

case IS_EQUAL_TO :
case EQUAL_TO :
case EQUALS :
if (null != actual && actual.equals(expected)) {
abort(target, condition, expected, actual);
}
break;

case NOT_EQUAL :
if (!Objects.equals(actual, expected)) {
abort(target, condition, expected, actual);
}
break;

case IS_NULL :
if (isTrue(expected) == (null == actual)) {
abort(target, condition, expected, actual);
}
break;

case NOT_NULL :
if (isTrue(expected) == (null != actual)) {
abort(target, condition, expected, actual);
}
break;

case CONTAINS :
if (null != actual && actual.toString().contains(expected.toString())) {
abort(target, condition, expected, actual);
}
break;

case IS_EMPTY :
if (isTrue(expected) == (null == actual || Strings.isEmpty(actual.toString()))) {
abort(target, condition, expected, actual);
}
break;

case NOT_EMPTY :
if (isTrue(expected) == ((null != actual && Strings.isNonEmpty(actual.toString())))) {
abort(target, condition, expected, actual);
}
break;

case MATCHES :
if (null != actual && actual.toString().matches(expected.toString())) {
abort(target, condition, expected, actual);
}
break;

case HAS_TRUTH_VALUE :
if (isTrue(expected) == isTrue(actual)) {
abort(target, condition, expected, actual);
}
break;

default:
abort(target, condition, expected, actual);
if (!knownCondition(condition)) {
abort(target, UNKNOWN_CONDITION, expected, actual);
} else if (conditionHolds(condition, actual, expected)) {
abort(target, condition, expected, actual);
}
}
}


private static boolean conditionHolds(String condition, Object actual, Object expected) {
switch (condition) {
case IS_EQUAL_TO:
case EQUAL_TO:
case EQUALS:
return null != actual && actual.equals(expected);
case NOT_EQUAL:
return !Objects.equals(actual, expected);
case IS_NULL:
return isTrue(expected) == (null == actual);
case NOT_NULL:
return isTrue(expected) == (null != actual);
case CONTAINS:
return null != actual && actual.toString().contains(expected.toString());
case IS_EMPTY:
return isTrue(expected) == (null == actual || Strings.isEmpty(actual.toString()));
case NOT_EMPTY:
return isTrue(expected) == ((null != actual && Strings.isNonEmpty(actual.toString())));
case MATCHES:
return null != actual && actual.toString().matches(expected.toString());
case HAS_TRUTH_VALUE:
return isTrue(expected) == isTrue(actual);
case GREATER_THAN:
return canCompare(actual, expected) && compare(actual, expected) > 0;
case LESS_THAN:
return canCompare(actual, expected) && compare(actual, expected) < 0;
default:
return false;
}
}

private static boolean knownCondition(String condition) {
// Everything but UNKNOWN_CONDITION. The conditions should really be an enum!
Set<String> allConditions = ImmutableSet.of(
IS_NULL, NOT_NULL, IS_EQUAL_TO, EQUAL_TO, EQUALS, NOT_EQUAL,
MATCHES, CONTAINS, IS_EMPTY, NOT_EMPTY, HAS_TRUTH_VALUE,
GREATER_THAN, LESS_THAN);
return allConditions.contains(condition);
}

/** @return True if actual and expected are both non-null instances of {@code Comparable<T>}. */
private static boolean canCompare(@Nullable Object actual, @Nullable Object expected) {
return actual != null
&& expected != null
&& actual instanceof Comparable
&& actual.getClass().equals(expected.getClass());
Copy link
Contributor

@aledsage aledsage Apr 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should do any coercion, or perhaps even just give it a go to compare! For example, comparing (Integer)1 with (Long)2 might well be possible. However, on balance I think your code is right as it is the simplest (and avoid ClassCastException).

Copy link
Contributor Author

@sjcorbett sjcorbett Apr 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting one. This implementation has already bitten me: "webapp.reqs.perSec.last expected greaterThan 80 but found 96.8063872255489". I'm inclined to leave this as-is until there are more complaints. I do think that this is to restrictive though - actual and expected could be different types if Comparable is implemented by a shared superclass. Don't think it'll be a common problem though.

}

@SuppressWarnings("unchecked")
private static int compare(@Nullable Object actual, @Nullable Object expected) {
if (!canCompare(actual, expected)) {
throw new IllegalArgumentException("Arguments are not comparable: " + actual + ", " + expected);
}
Comparable a = (Comparable) actual;
Comparable e = (Comparable) expected;
return a.compareTo(e);
}

static void failAssertion(String target, String assertion, Object expected, Object actual) {
throw new AssertionError(Joiner.on(' ').join(
Objects.toString(target),
Expand Down
24 changes: 22 additions & 2 deletions ...amework/src/test/java/org/apache/brooklyn/test/framework/TestFrameworkAssertionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@
import static org.testng.Assert.assertTrue;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.test.framework.TestFrameworkAssertions.AssertionOptions;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.text.Identifiers;
import org.apache.brooklyn.util.time.Duration;
import org.slf4j.Logger;
Expand Down Expand Up @@ -71,6 +73,12 @@ public Object[][] positiveTestsDP() {
{"some-non-null-value", Arrays.asList(ImmutableMap.of("notEmpty", Boolean.TRUE))},
{"true", Arrays.asList(ImmutableMap.of("hasTruthValue", Boolean.TRUE))},
{"false", Arrays.asList(ImmutableMap.of("hasTruthValue", Boolean.FALSE))},

{25, Collections.singletonList(ImmutableMap.of("greaterThan", 24))},
{"b", Collections.singletonList(ImmutableMap.of("greaterThan", "a"))},
{24, Collections.singletonList(ImmutableMap.of("lessThan", 25))},
{"a", Collections.singletonList(ImmutableMap.of("lessThan", "b"))},

{"some-non-null-value", Arrays.asList(ImmutableMap.of("hasTruthValue", Boolean.FALSE))},
};
}
Expand Down Expand Up @@ -137,6 +145,18 @@ public Object[][] negativeTestsDP() {
{"<html><body><h1>Im a H1 tag!</h1></body></html>", "contains", "quack", Arrays.asList(ImmutableMap.of("contains", "quack"))},
{"{\"a\":\"b\",\"c\":\"d\",\"e\":123,\"g\":false}", "contains", "moo", Arrays.asList(ImmutableMap.of("contains", "moo"))},

{25, "lessThan", 24, Collections.singletonList(ImmutableMap.of("lessThan", 24))},
{"b", "lessThan", "a", Collections.singletonList(ImmutableMap.of("lessThan", "a"))},
{null, "lessThan", "a", Collections.singletonList(ImmutableMap.of("lessThan", "a"))},
{"a", "lessThan", null, Collections.singletonList(MutableMap.of("lessThan", null))},
{5, "lessThan", 5, Collections.singletonList(ImmutableMap.of("lessThan", 5))},

{24, "greaterThan", 25, Collections.singletonList(ImmutableMap.of("greaterThan", 25))},
{"a", "greaterThan", "b", Collections.singletonList(ImmutableMap.of("greaterThan", "b"))},
{null, "greaterThan", "a", Collections.singletonList(ImmutableMap.of("greaterThan", "a"))},
{"a", "greaterThan", null, Collections.singletonList(MutableMap.of("greaterThan", null))},
{5, "greaterThan", 5, Collections.singletonList(ImmutableMap.of("greaterThan", 5))},

{"true", "hasTruthValue", Boolean.FALSE, Arrays.asList(ImmutableMap.of("hasTruthValue", Boolean.FALSE))},
{"false", "hasTruthValue", Boolean.TRUE, Arrays.asList(ImmutableMap.of("hasTruthValue", Boolean.TRUE))},
{"some-not-null-value", "hasTruthValue", Boolean.TRUE, Arrays.asList(ImmutableMap.of("hasTruthValue", Boolean.TRUE))}
Expand All @@ -161,7 +181,7 @@ public Object get() {
.timeout(timeout).assertions(assertions));
Asserts.shouldHaveFailedPreviously();
} catch (AssertionError e) {
Asserts.expectedFailureContains(e, Objects.toString(data), condition, expected.toString());
Asserts.expectedFailureContains(e, Objects.toString(data), condition, expected != null ? expected.toString() : "null");
}
}

Expand All @@ -187,7 +207,7 @@ public Object get() {
.assertions(assertions));
Asserts.shouldHaveFailedPreviously();
} catch (AssertionError e) {
Asserts.expectedFailureContains(e, Objects.toString(data), condition, expected.toString());
Asserts.expectedFailureContains(e, Objects.toString(data), condition, expected != null ? expected.toString() : "null");
}
}

Expand Down
2 changes: 1 addition & 1 deletion utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public static boolean isValidIp4(String input) {

/**
* Gets an InetAddress using the given hostname or IP. If it is an IPv4 address, then this is equivalent
* to {@link getInetAddressWithFixedName(byte[])}. If it is a hostname, then this hostname will be used
* to {@link #getInetAddressWithFixedName(byte[])}. If it is a hostname, then this hostname will be used
* in the returned InetAddress.
*/
public static InetAddress getInetAddressWithFixedName(String hostnameOrIp) {
Expand Down