Ensure error messages are useful (#141)
* Add corner case test for CryptoTest * Add RandomService tests using default constructor * Improve FakeValueService error messaging and add tests
This commit is contained in:
@@ -107,6 +107,24 @@
|
||||
<version>1.5.1</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.powermock</groupId>
|
||||
<artifactId>powermock-core</artifactId>
|
||||
<version>1.6.4</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.powermock</groupId>
|
||||
<artifactId>powermock-api-mockito</artifactId>
|
||||
<version>1.6.4</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.powermock</groupId>
|
||||
<artifactId>powermock-module-junit4</artifactId>
|
||||
<version>1.6.4</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
</dependencies>
|
||||
<build>
|
||||
<pluginManagement>
|
||||
|
||||
@@ -435,7 +435,12 @@ public class FakeValuesService {
|
||||
return null;
|
||||
}
|
||||
// coerce the string arguments into the correct argument types
|
||||
List<Object> coerced = coerceArguments(accessor, args);
|
||||
List<Object> coerced = null;
|
||||
try {
|
||||
coerced = coerceArguments(accessor, args);
|
||||
} catch (RuntimeException re) {
|
||||
log.log(Level.FINE, "Unable to coerce arguments : " + re.getMessage());
|
||||
}
|
||||
return (accessor == null || coerced == null)
|
||||
? null
|
||||
: string(accessor.invoke(obj, coerced.toArray()));
|
||||
@@ -450,27 +455,25 @@ public class FakeValuesService {
|
||||
* {@link Faker#name()}'s {@link Name#firstName()} method.
|
||||
* @throws RuntimeException if there's a problem invoking the method or it doesn't exist.
|
||||
*/
|
||||
public String resolveFakerObjectAndMethod(Faker faker, String key, List<String> args) {
|
||||
private String resolveFakerObjectAndMethod(Faker faker, String key, List<String> args) {
|
||||
final String[] classAndMethod = key.split("\\.", 2);
|
||||
|
||||
try {
|
||||
String fakerMethodName = classAndMethod[0].replaceAll("_", "");
|
||||
Method fakerAccessor = accessor(faker, fakerMethodName, Collections.<String>emptyList());
|
||||
if (fakerAccessor == null) {
|
||||
throw new RuntimeException("Can't find top level faker object named " + fakerMethodName);
|
||||
throw new RuntimeException("Can't find top level faker object named " + fakerMethodName + ".");
|
||||
}
|
||||
Object objectWithMethodToInvoke = fakerAccessor.invoke(faker);
|
||||
String nestedMethodName = classAndMethod[1].replaceAll("_", "");
|
||||
final Method accessor = accessor(objectWithMethodToInvoke, classAndMethod[1].replaceAll("_", ""), args);
|
||||
if (accessor == null) {
|
||||
throw new RuntimeException("Can't find method on " + objectWithMethodToInvoke + " called " + nestedMethodName);
|
||||
throw new RuntimeException("Can't find method on "
|
||||
+ objectWithMethodToInvoke.getClass().getSimpleName()
|
||||
+ " called " + nestedMethodName + ".");
|
||||
}
|
||||
final List<Object> coerced = coerceArguments(accessor, args);
|
||||
|
||||
if (coerced == null) {
|
||||
throw new RuntimeException("Can't coerce arguments for downstream faker object.");
|
||||
}
|
||||
|
||||
Object ret = accessor.invoke(objectWithMethodToInvoke, coerced.toArray());
|
||||
|
||||
return ret == null ? null : ret.toString();
|
||||
@@ -487,7 +490,7 @@ public class FakeValuesService {
|
||||
* Find an accessor by name ignoring case.
|
||||
*/
|
||||
private Method accessor(Object onObject, String name, List<String> args) {
|
||||
log.fine("Find accessor named " + name + " on " + onObject + " with args " + args);
|
||||
log.log(Level.FINE, "Find accessor named " + name + " on " + onObject.getClass().getSimpleName() + " with args " + args);
|
||||
Method fakerAccessor = null;
|
||||
for (Method m : onObject.getClass().getMethods()) {
|
||||
if (m.getName().equalsIgnoreCase(name) && m.getParameterTypes().length == args.size()) {
|
||||
@@ -518,8 +521,7 @@ public class FakeValuesService {
|
||||
|
||||
coerced.add(coercedArgument);
|
||||
} catch (Exception e) {
|
||||
log.log(Level.FINE, "Unable to coerce " + args.get(i) + " to " + toType.getSimpleName() + " via string constructor" ,e);
|
||||
return null;
|
||||
throw new RuntimeException("Unable to coerce " + args.get(i) + " to " + toType.getSimpleName() + " via " + toType.getSimpleName() + "(String) constructor.");
|
||||
}
|
||||
}
|
||||
return coerced;
|
||||
|
||||
@@ -1,10 +1,22 @@
|
||||
package com.github.javafaker;
|
||||
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.Mockito;
|
||||
import org.powermock.api.mockito.PowerMockito;
|
||||
import org.powermock.core.classloader.annotations.PrepareForTest;
|
||||
import org.powermock.modules.junit4.PowerMockRunner;
|
||||
|
||||
import java.security.MessageDigest;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
|
||||
import static com.github.javafaker.matchers.MatchesRegularExpression.matchesRegularExpression;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.powermock.api.mockito.PowerMockito.mockStatic;
|
||||
|
||||
@RunWith(PowerMockRunner.class)
|
||||
public class CryptoTest extends AbstractFakerTest {
|
||||
@Test
|
||||
public void testMd5() {
|
||||
@@ -25,4 +37,25 @@ public class CryptoTest extends AbstractFakerTest {
|
||||
public void testSha512() {
|
||||
assertThat(faker.crypto().sha512(), matchesRegularExpression("[a-z\\d]+"));
|
||||
}
|
||||
|
||||
|
||||
// this is a total corner case, but if for some reason a system doesn't support
|
||||
// a specific algorithm then it will wrap the NoSuchAlgorithm in a RuntimeException
|
||||
// and throw it. This should never happen, but who knows.
|
||||
@Test(expected = RuntimeException.class)
|
||||
@PrepareForTest(Crypto.class)
|
||||
public void missingAlgorithVomitsHorribly() throws NoSuchAlgorithmException {
|
||||
mockStatic(MessageDigest.class);
|
||||
|
||||
PowerMockito.when(MessageDigest.getInstance(Mockito.anyString()))
|
||||
.thenThrow(new NoSuchAlgorithmException("I no haz that algorithm."));
|
||||
|
||||
try {
|
||||
faker.crypto().sha1();
|
||||
} catch (RuntimeException re) {
|
||||
assertThat(re.getCause(), is(instanceOf(NoSuchAlgorithmException.class)));
|
||||
assertThat(re.getCause().getMessage(), is("I no haz that algorithm."));
|
||||
throw re;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ import static com.github.javafaker.matchers.MatchesRegularExpression.matchesRegu
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.hamcrest.core.Is.is;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.mockito.Mockito.*;
|
||||
|
||||
public class FakeValuesServiceTest extends AbstractFakerTest {
|
||||
@@ -204,6 +205,55 @@ public class FakeValuesServiceTest extends AbstractFakerTest {
|
||||
assertThat(chain.get(1), is(Locale.ENGLISH));
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void expressionWithInvalidFakerObject() {
|
||||
expressionShouldFailWith("#{ObjectNotOnFaker.methodName}",
|
||||
"Can't find top level faker object named ObjectNotOnFaker.");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void expressionWithValidFakerObjectButInvalidMethod() {
|
||||
expressionShouldFailWith("#{Name.nonExistentMethod}",
|
||||
"Can't find method on Name called nonExistentMethod.");
|
||||
}
|
||||
|
||||
/**
|
||||
* Two things are important here:
|
||||
* 1) the message in the exception should be USEFUL
|
||||
* 2) a {@link RuntimeException} should be thrown.
|
||||
*
|
||||
* if the message changes, it's ok to update the test provided
|
||||
* the two conditions above are still true.
|
||||
*/
|
||||
@Test/*(expected = RuntimeException.class)*/
|
||||
public void expressionWithValidFakerObjectValidMethodInvalidArgs() {
|
||||
expressionShouldFailWith("#{Number.number_between 'x','y'}",
|
||||
"Unable to coerce x to Long via Long(String) constructor.");
|
||||
}
|
||||
|
||||
/**
|
||||
* Two things are important here:
|
||||
* 1) the message in the exception should be USEFUL
|
||||
* 2) a {@link RuntimeException} should be thrown.
|
||||
*
|
||||
* if the message changes, it's ok to update the test provided
|
||||
* the two conditions above are still true.
|
||||
*/
|
||||
@Test
|
||||
public void expressionCompletelyUnresolvable() {
|
||||
expressionShouldFailWith("#{x}", "Unable to resolve #{x} directive.");
|
||||
}
|
||||
|
||||
private void expressionShouldFailWith(String expression, String errorMessage) {
|
||||
try {
|
||||
fakeValuesService.expression(expression, faker);
|
||||
fail("Should have failed with RuntimeException and message of " + errorMessage);
|
||||
} catch (RuntimeException re) {
|
||||
assertThat(re.getMessage(), is(errorMessage));
|
||||
}
|
||||
}
|
||||
|
||||
public static class DummyService {
|
||||
public String firstName() {
|
||||
return "John";
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
package com.github.javafaker.service;
|
||||
|
||||
import com.github.javafaker.AbstractFakerTest;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.junit.runners.Parameterized;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.Random;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
@@ -15,16 +18,24 @@ import static org.junit.Assert.assertThat;
|
||||
* @author pmiklos
|
||||
*
|
||||
*/
|
||||
@RunWith(Parameterized.class)
|
||||
public class RandomServiceTest extends AbstractFakerTest {
|
||||
|
||||
private RandomService randomService;
|
||||
|
||||
@Before
|
||||
public void before() {
|
||||
super.before();
|
||||
randomService = new RandomService(new Random());
|
||||
public RandomServiceTest(String ignoredTitle, RandomService service) {
|
||||
this.randomService = service;
|
||||
}
|
||||
|
||||
@Parameterized.Parameters(name = "Created via {0}")
|
||||
public static Collection<Object[]> data() {
|
||||
Object[][] data = new Object[][]{
|
||||
{"RandomService(Random)", new RandomService(new Random())},
|
||||
{"RandomService()", new RandomService()}
|
||||
};
|
||||
return Arrays.asList(data);
|
||||
}
|
||||
|
||||
@Test(expected = IllegalArgumentException.class)
|
||||
public void testPositiveBoundariesOnly() {
|
||||
randomService.nextLong(0L);
|
||||
|
||||
Reference in New Issue
Block a user