{And|Or|Not|Field}PredicateTest: Convert remaining tests to ExpectedException

Some of the tests are still using the pattern:

  try {
    do something that will raise an exception
  } catch (ExpectedExceptionType e) {
    assertEquals("Expected error message", e.getMessage());
  }

The problem with this pattern is that if the expected exception is not
raised, the test still passes.  This could be mitigated by adding a
call to fail() in the try-block:

  try {
    do something that will raise an exception
    fail(); // only reached when the exception is not raised
  } catch (ExpectedExceptionType e) {
    assertEquals("Expected error message", e.getMessage());
  }

but since we have the ExpectedException, and it's already being used in
other parts of these tests, let's use it here too:

  expect(ExpectedException.class);
  expectMessage("Expected error message");
  do something that will raise an exception

This causes failures in the `testCopy` method of AndPredicateTest and
OrPredicateTest because the expected exceptions are not being raised.

The implementations of AndPredicate and OrPredicate were  modified in
changes Id55ec1b84 and I2b8f85891 to not raise the exception, and due
to the flaw in the test pattern mentioned above, the tests continued to
pass.

Remove the now out-of-date test statements.

Change-Id: I078f4c6433a26899f7c1c7efd7b8d8f89e721da8
This commit is contained in:
David Pursehouse
2015-11-05 17:16:08 +09:00
parent 7e1f643a8f
commit a7cc474cdf
4 changed files with 9 additions and 29 deletions

View File

@@ -24,7 +24,6 @@ import static org.junit.Assert.assertTrue;
import org.junit.Test;
import java.util.Collections;
import java.util.List;
public class AndPredicateTest extends PredicateTest {
@@ -109,11 +108,5 @@ public class AndPredicateTest extends PredicateTest {
assertNotSame(n2, n2.copy(s2));
assertEquals(s2, n2.copy(s2).getChildren());
assertEquals(s3, n2.copy(s3).getChildren());
try {
n2.copy(Collections.<Predicate<String>> emptyList());
} catch (IllegalArgumentException e) {
assertEquals("Need at least two predicates", e.getMessage());
}
}
}

View File

@@ -61,10 +61,8 @@ public class FieldPredicateTest extends PredicateTest {
assertSame(f, f.copy(Collections.<Predicate<String>> emptyList()));
assertSame(f, f.copy(f.getChildren()));
try {
exception.expect(IllegalArgumentException.class);
exception.expectMessage("Expected 0 children");
f.copy(Collections.singleton(f("owner", "bob")));
} catch (IllegalArgumentException e) {
assertEquals("Expected 0 children", e.getMessage());
}
}
}

View File

@@ -103,16 +103,12 @@ public class NotPredicateTest extends PredicateTest {
assertNotSame(n, n.copy(sb));
assertEquals(sb, n.copy(sb).getChildren());
try {
exception.expect(IllegalArgumentException.class);
exception.expectMessage("Expected exactly one child");
n.copy(Collections.<Predicate> emptyList());
} catch (IllegalArgumentException e) {
assertEquals("Expected exactly one child", e.getMessage());
}
try {
exception.expect(IllegalArgumentException.class);
exception.expectMessage("Expected exactly one child");
n.copy(and(a, b).getChildren());
} catch (IllegalArgumentException e) {
assertEquals("Expected exactly one child", e.getMessage());
}
}
}

View File

@@ -24,7 +24,6 @@ import static org.junit.Assert.assertTrue;
import org.junit.Test;
import java.util.Collections;
import java.util.List;
public class OrPredicateTest extends PredicateTest {
@@ -109,11 +108,5 @@ public class OrPredicateTest extends PredicateTest {
assertNotSame(n2, n2.copy(s2));
assertEquals(s2, n2.copy(s2).getChildren());
assertEquals(s3, n2.copy(s3).getChildren());
try {
n2.copy(Collections.<Predicate<String>> emptyList());
} catch (IllegalArgumentException e) {
assertEquals("Need at least two predicates", e.getMessage());
}
}
}