sheat2500 / hamcrest

Automatically exported from code.google.com/p/hamcrest
0 stars 0 forks source link

Matchers.hasProperty doesn't appear to work correctly all the time #185

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I found this when trying to write a test which asserted that the user's 
Principal in a ServletRequest was correctly set. For some reason the matcher 
doesn't return true for this object even though it has the "name" property 
which matches the string value I'm testing for.

I tried to reproduce the test with some other Principal implementation and that 
worked, so I pulled a cut-down version of our own implementation into this test 
case and indeed, the problem still occurs but I can't figure out why anymore.

public class TestMatchers
{
    Matcher<Principal> m = Matchers.hasProperty("name", equalTo("bob"));

    @Test
    public void testSunPrincipal() throws Exception
    {
        Principal p = new PrincipalImpl("bob");
        assertThat(p.getName(), is(equalTo("bob")));
        assertThat(p, is(m));
    }

    @Test
    public void testUserPrincipal() throws Exception
    {
        Mockery mockery = new Mockery();
        final User user = mockery.mock(User.class);
        mockery.checking(new Expectations() {{
            allowing(user).getName(); will(returnValue("bob"));
        }});

        Principal p = new UserPrincipal(user);
        assertThat(p.getName(), is(equalTo("bob")));
        assertThat(p, is(m)); // <- fails here
    }

    private static interface User
    {
        String getName();
    }

    private static class UserPrincipal implements Principal
    {
        private final User user;

        public UserPrincipal(User user)
        {
            this.user = user;
        }

        @Override
        public String getName()
        {
            return user.getName();
        }

        @Override
        public String toString()
        {
            return String.format("UserPrincipal(%s)", user);
        }
    }

}

Original issue reported on code.google.com by trejkaz on 14 Jun 2012 at 12:00

GoogleCodeExporter commented 9 years ago
I can write a naive replacement for hasProperty which works correctly for both 
cases:

    private void assertContainsPropertyTheHardWay(Object object, String propertyName, Matcher valueMatcher) throws Exception
    {
        for (PropertyDescriptor property : Introspector.getBeanInfo(object.getClass()).getPropertyDescriptors())
        {
            if (propertyName.equals(property.getName()))
            {
                Object actualValue = property.getReadMethod().invoke(object);
                assertThat(actualValue, valueMatcher);
                return;
            }
        }

        fail("Couldn't find property " + propertyName);
    }

Original comment by trejkaz on 14 Jun 2012 at 12:09

GoogleCodeExporter commented 9 years ago
And here's a drop-in replacement for hasProperty with a snarky method name.

    private static <T> Matcher<T> hasPropertyWhichActuallyWorks(final String name, final Matcher<?> valueMatcher)
    {
        return new BaseMatcher<T>()
        {
            @Override
            public boolean matches(Object o)
            {
                try
                {
                    for (PropertyDescriptor property : Introspector.getBeanInfo(o.getClass()).getPropertyDescriptors())
                    {
                        if (name.equals(property.getName()))
                        {
                            Object actualValue = property.getReadMethod().invoke(o);
                            return valueMatcher.matches(actualValue);
                        }
                    }

                    return false;
                }
                catch (Exception e)
                {
                    throw new RuntimeException("Unexpected error introspecting the bean", e);
                }
            }

            @Override
            public void describeTo(Description description)
            {
                description.appendText("hasProperty(");
                description.appendValue(name);
                description.appendText(")");
            }
        };
    }

Original comment by trejkaz on 14 Jun 2012 at 12:14

GoogleCodeExporter commented 9 years ago

Original comment by t.denley on 18 Jun 2012 at 9:56

GoogleCodeExporter commented 9 years ago
I believe that this issue has been fixed on trunk, which we hope to release 
from soon.  The changes made in this check-in appear to have been responsible 
for the resolution: 
https://github.com/hamcrest/JavaHamcrest/commit/8e00b60beb6e13b9c476e5e45d159a9d
a646d7d1

Original comment by t.denley on 18 Jun 2012 at 10:11

GoogleCodeExporter commented 9 years ago
now that Hamcrest 1.3 has been released, please can you confirm whether this 
issue still exists.

Original comment by t.denley on 18 Jul 2012 at 4:20

GoogleCodeExporter commented 9 years ago
Closing this as fixed, given the raising user has gone dark.

Original comment by t.denley on 29 Jul 2012 at 5:20

GoogleCodeExporter commented 9 years ago
Sorry for the delay... I had some other stuff to do. :(

The test code above still fails on Hamcrest 1.3.

Original comment by trejkaz on 29 Jul 2012 at 11:06

GoogleCodeExporter commented 9 years ago
OK, thanks for the feedback. I'll have another look at this.

Original comment by t.denley on 30 Jul 2012 at 7:44