improved mismatch reporting in HasPropertyWithValue

smgfreeman · smgfreeman · commit 359c3ca25774 · 2009-02-27T17:45:07.000Z
diff --git a/hamcrest-library/src/main/java/org/hamcrest/beans/HasPropertyWithValue.java b/hamcrest-library/src/main/java/org/hamcrest/beans/HasPropertyWithValue.java
@@ -68,26 +68,25 @@
  */
 public class HasPropertyWithValue<T> extends TypeSafeDiagnosingMatcher<T> {
     private final String propertyName;
-    private final Matcher<?> value;
+    private final Matcher<?> valueMatcher;
 
-    public HasPropertyWithValue(String propertyName, Matcher<?> value) {
+    public HasPropertyWithValue(String propertyName, Matcher<?> valueMatcher) {
         this.propertyName = propertyName;
-        this.value = value;
+        this.valueMatcher = valueMatcher;
     }
 
     @Override
-	public boolean matchesSafely(T argument, Description mismatchDescription) {
+	public boolean matchesSafely(T bean, Description mismatchDescription) {
         try {
-            Method readMethod = getReadMethod(argument);
+            Method readMethod = findReadMethod(bean, mismatchDescription);
             if (readMethod == null) {
-              mismatchDescription.appendText("missing.");
             	return false;
             }
-            Object propertyValue = readMethod.invoke(argument, NO_ARGUMENTS);
-            boolean valueMatches = value.matches(propertyValue);
+            Object propertyValue = readMethod.invoke(bean, NO_ARGUMENTS);
+            boolean valueMatches = valueMatcher.matches(propertyValue);
             if (!valueMatches) {
-            	value.describeMismatch(propertyValue, mismatchDescription);
-            	mismatchDescription.appendText(".");
+              mismatchDescription.appendText("property \"" + propertyName + "\" ");
+            	valueMatcher.describeMismatch(propertyValue, mismatchDescription);
             }
             return valueMatches;
         } catch (IllegalArgumentException e) {
@@ -99,16 +98,24 @@ public boolean matchesSafely(T argument, Description mismatchDescription) {
         }
     }
 
-    private Method getReadMethod(Object argument) throws IllegalArgumentException {
+    private Method findReadMethod(Object argument, Description mismatchDescription) throws IllegalArgumentException {
         PropertyDescriptor property = PropertyUtil.getPropertyDescriptor(propertyName, argument);
-        return property == null ? null : property.getReadMethod();
+        if (null == property) {
+          mismatchDescription.appendText("No property \"" + propertyName + "\"");
+          return null;
+        }
+        Method readMethod = property.getReadMethod();
+        if (null == readMethod) {
+          mismatchDescription.appendText("property \"" + propertyName + "\" is not readable");
+        }
+        return readMethod;
     }
 
     public void describeTo(Description description) {
         description.appendText("hasProperty(");
         description.appendValue(propertyName);
         description.appendText(", ");
-        description.appendDescriptionOf(value);
+        description.appendDescriptionOf(valueMatcher);
         description.appendText(")");
     }
 
diff --git a/hamcrest-unit-test/src/main/java/org/hamcrest/beans/HasPropertyWithValueTest.java b/hamcrest-unit-test/src/main/java/org/hamcrest/beans/HasPropertyWithValueTest.java
@@ -6,7 +6,6 @@
 import static org.hamcrest.beans.HasPropertyWithValue.hasProperty;
 import static org.hamcrest.core.IsAnything.anything;
 import static org.hamcrest.core.IsEqual.equalTo;
-import static org.hamcrest.core.IsNot.not;
 
 import java.beans.IntrospectionException;
 import java.beans.PropertyDescriptor;
@@ -38,45 +37,43 @@ protected Matcher<?> createMatcher() {
   }
 
   public void testMatchesInfolessBeanWithMatchedNamedProperty() {
-    assertThat(shouldMatch, hasProperty("property", equalTo("is expected")));
-    assertThat(shouldNotMatch, not(hasProperty("property", equalTo("is expected"))));
+    assertMatches("with property", hasProperty("property", equalTo("is expected")), shouldMatch);
+    assertMismatchDescription("property \"property\" was \"not expected\"", 
+                              hasProperty("property", equalTo("is expected")), shouldNotMatch);
   }
 
   public void testMatchesBeanWithInfoWithMatchedNamedProperty() {
-    assertThat(beanWithInfo, hasProperty("property", equalTo("with info")));
+    assertMatches("with bean info", hasProperty("property", equalTo("with info")), beanWithInfo);
+    assertMismatchDescription("property \"property\" was \"with info\"", 
+        hasProperty("property", equalTo("without info")), beanWithInfo);
   }
 
   public void testDoesNotMatchInfolessBeanWithoutMatchedNamedProperty() {
-    assertThat(shouldNotMatch, not(hasProperty("nonExistentProperty", anything())));
-  }
+    assertMismatchDescription("No property \"nonExistentProperty\"", 
+                              hasProperty("nonExistentProperty", anything()), shouldNotMatch);
+   }
 
   public void testDoesNotMatchWriteOnlyProperty() {
-    assertThat(shouldNotMatch,
-        not(hasProperty("writeOnlyProperty", anything())));
+    assertMismatchDescription("property \"writeOnlyProperty\" is not readable",
+                              hasProperty("writeOnlyProperty", anything()), shouldNotMatch); 
   }
 
   public void testDescribeTo() {
-    Matcher<?> matcher = equalTo(true);
-
-    assertDescription("hasProperty(\"property\", "
-        + StringDescription.asString(matcher) + ")", hasProperty("property", matcher));
+    assertDescription("hasProperty(\"property\", <true>)", hasProperty("property", equalTo(true)));
   }
 
+  public void testMatchesPropertyAndValue() {
+    assertMatches("property with value", hasProperty( "property", anything()), beanWithInfo);
+  }
+  
   public void testDoesNotWriteMismatchIfPropertyMatches() {
-    Matcher<Object> matcher = hasProperty( "property", anything());
-    assertTrue("Precondtion: Matcher should match item.", matcher.matches(beanWithInfo));
     Description description = new StringDescription();
-    matcher.describeMismatch(beanWithInfo, description);
+    hasProperty( "property", anything()).describeMismatch(beanWithInfo, description);
     assertEquals("Expected mismatch description", "", description.toString());
   }
 
   public void testDescribesMissingPropertyMismatch() {
-    assertMismatchDescription("missing.", hasProperty( "honk", anything()), shouldNotMatch);
-  }
-
-  public void testDescribesMismatchingPropertyValueMismatch() {
-    assertMismatchDescription("was \"not expected\".",
-        hasProperty("property", equalTo("foo")), shouldNotMatch);
+    assertMismatchDescription("No property \"honk\"", hasProperty( "honk", anything()), shouldNotMatch);
   }
 
   public void testCanAccessAnAnonymousInnerClass() {
@@ -133,8 +130,9 @@ public static class BeanWithInfoBeanInfo extends SimpleBeanInfo {
     @Override
     public PropertyDescriptor[] getPropertyDescriptors() {
       try {
-        return new PropertyDescriptor[] { new PropertyDescriptor("property",
-            BeanWithInfo.class, "property", null) };
+        return new PropertyDescriptor[] { 
+            new PropertyDescriptor("property", BeanWithInfo.class, "property", null) 
+          };
       } catch (IntrospectionException e) {
         throw new RuntimeException("Introspection exception: " + e.getMessage());
       }
