Skip to content
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.

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

Hamcrest Matcher for Iterable comparison #23

Closed
lemonboston opened this issue Oct 4, 2017 · 5 comments
Closed

Hamcrest Matcher for Iterable comparison #23

lemonboston opened this issue Oct 4, 2017 · 5 comments
Assignees

Comments

@lemonboston
Copy link

In order to assert Iterables easily, as a developer, I want a Iterable hamcrest Matcher.


Hamcrest's IsIterableContainingInOrder doesn't have a factory method with Iterable parameter unfortunately.

@lemonboston lemonboston self-assigned this Oct 4, 2017
lemonboston pushed a commit that referenced this issue Oct 4, 2017
…Move TestDoubles and FailAnswer to packages. #23
lemonboston pushed a commit that referenced this issue Oct 4, 2017
…Move TestDoubles and FailAnswer to packages. #23
lemonboston pushed a commit that referenced this issue Oct 4, 2017
…Move TestDoubles and FailAnswer to packages. #23
@dmfs
Copy link
Owner

dmfs commented Oct 4, 2017

What's an actual use case for this that doesn't work with contains?

@dmfs
Copy link
Owner

dmfs commented Oct 4, 2017

Nevermind, I just noticed that you "use" it in #25.

I think that doesn't really justify this class, because the premise is wrong (see my comment in #25).

Regarding ValueObject, I thinks it's a good thing to have. It gets us closer to the "only assertThat" idea. I'm wondering if we can get this to work for dummies and mocks instead to allow using it with any type. I'm thinking of a "named" or "indexed" "dummy" or "mock". Something like this:

assertTrue(dummy(Foo.class, "x").equals(dummy(Foo.class, "x")));
assertTrue(dummy(Foo.class, "x").equals(dummy(Foo.class, "y")));

Unfortunately that doesn't work this way, because you can not mock equals. However, I guess we could use the mock name and a MockNameMatcher which verifies that the given object is a mock and has the same name as the given one.

this this:

    public static <T> T namedDummy(Class<T> clazz, String o)
    {
        T result = mock(clazz, withSettings().defaultAnswer(new FailAnswer()).name("org.dmfs.MOCK:" + clazz.getCanonicalName() + ":" + o));
        doReturn(clazz.toString()).when(result).toString();
        return result;
    }


    public final static class MockNameMatcher extends BaseMatcher
    {
        private final MockingDetails mMockingDetails;


        public static MockNameMatcher isMock(Object object)
        {
            return new MockNameMatcher(mockingDetails(object));
        }


        private MockNameMatcher(MockingDetails mockingDetails)
        {
            mMockingDetails = mockingDetails;
        }


        @Override
        public boolean matches(Object item)
        {
            MockingDetails mockingDetails = mockingDetails(item);
            return mockingDetails.isMock() && mockingDetails.getMockCreationSettings()
                    .getMockName().toString()
                    .equals(mMockingDetails.getMockCreationSettings().getMockName().toString());
        }


        @Override
        public void describeTo(Description description)
        {
        }
    }

which would be used like this

assertThat(namedDummy(Object.class, "x"), isMock(namedDummy(Object.class, "x")));
assertThat(namedDummy(Object.class, "x"), not(isMock(namedDummy(Object.class, "y"))));

There may be better ways to achieve this.

the example in #25 would look like this:

assertThat(
    new MappedRowSetBatch<>(
        new TestRowSet<Contract>(
            namedDummy(RowSnapshot.class, "a"),
            namedDummy(RowSnapshot.class, "b"),
            namedDummy(RowSnapshot.class, "c")),
        new MockFunction<>(
            new Pair(namedDummy(RowSnapshot.class, "a"), namedDummy(Operation.class, "x")),
            new Pair(namedDummy(RowSnapshot.class, "b"), namedDummy(Operation.class, "y")),
            new Pair(namedDummy(RowSnapshot.class, "c"), namedDummy(Operation.class, "z"))),
    contains(
        isMock(namedDummy(Operation.class, "x"),
        isMock(namedDummy(Operation.class, "y"),
        isMock(namedDummy(Operation.class, "z")));

@lemonboston
Copy link
Author

Interesting, nice idea to have value object for any type easily with those named dummies.

Although, I am not sure that this MappedRowSetBatch would be a good place to use them since we need to assert on reference equality here, not object equality, right?. The same instances of RowSnapshots that are going in have to be mapped to instances of Operation that are asserted afterwards. RowSnapshot and Operation type definitions are also not really referring to value objects, so it can be a bit misleading to make them that.
As I see it, this way we would want to circumvent the need to keep references of dummies by making them value objects and asserting on object equality instead of reference equality at the end.
For me this would make understanding the intentions in a tests and what we are really testing a bit more difficult.
For the same reasons, now I think it could be useful to be explicit about Iterable matcher method names, whether they use object or reference equality. Just as with a single item, there is equalTo() and sameInstance().
And MappedRowSetBatchTest would use the reference comparison version, not the object equality as in my sample code.

Back to these namedDummys, I am not sure about the name 'dummy', for me that means just an instance of a type that we don't call any methods on. Value object is I think a standard terminology, so for me it would be clear to use that, so maybe valueDummy() but again, I would probably not use 'dummy'. So just 'value(clazz, str/int). And I think we should only use that when it makes sense for the type to be a value object. I am not sure we have too many of those types by the way. I can think of dmfs.DateTimenow, not really anything else. Because it comes back to the difficulty in Java to defineequals()along with polymorphism. So even if implementations ofdmfs.Urifor example could be a good candidate for value objects, since we don't actually have thoseequals()implemented, it is probably also wrong to assume that in tests. So because of this, as I see now, value objects mostly can only be 'represented' with classes safely and not interfaces. And since we use interfaces we don't really have value objects. So thisValueObjectwill most probably be useful only inMatchertests, if theMatcher` uses object equality.

What do you think?

How about having something like hasEqualToElements() and hasSameInstanceElements()/hasSameElements() IterableMatchers methods, with overloaded versions?

@lemonboston
Copy link
Author

Thinking about it a bit more, I can see now that you don't really want to create value objects with these named mocks but, in fact, find a solution for not needing to keep the references to dummies.
I still think it's not the best to use equals() under the hood for that.
How about an object pool or factory kind of thing?
And calling with dummyRef(class, str/int) and there is just some Map underneath. The problem is clearing that up after each test method. Or maybe that's not important, since they can be re-used without side effects anyway.

@dmfs
Copy link
Owner

dmfs commented Oct 5, 2017

I've considered a map or list to return the same instance for the same names/numbers. But that didn't appear like a clean solution to me. I felt this solution is a slightly cleaner way (although still not perfect).

I don't see equals as a problem here. The Matcher checks for a very specific condition that production code can not create. In particular it only matches if the checked object is a mock and has the same name (it doesn't call equals on the mock object itself). If production code would create such an object, something would be really wrong. So it should be safe to assume that the checked object is actually the test dummy we passed to the tested object.

For brevity we could use numbered dummies (like you use it in ValueObject) and a shorter factory method name. For frequently used dummies (or mocks) we could also use dedicated factory methods (rowSnapshotDummy(int)). Also I just noticed that the MockFunction Pair would have to take a Matcher to match the arguments (which is more flexible anyway)

The test above would look like this:

assertThat(
    new MappedRowSetBatch<>(
        new MockRowSet<Contract>(
            rowSnapthotDummy(1),
            rowSnapthotDummy(2),
            rowSnapthotDummy(3)),
        new MockFunction<>(
            new Pair(isDummy(rowSnapthotDummy(1)), operationDummy(1)),
            new Pair(isDummy(rowSnapthotDummy(2)), operationDummy(2)),
            new Pair(isDummy(rowSnapthotDummy(3)), operationDummy(3)))),
    contains(
        isDummy(operationDummy(1)),
        isDummy(operationDummy(2)),
        isDummy(operationDummy(3))));

lemonboston pushed a commit that referenced this issue Oct 12, 2017
lemonboston pushed a commit that referenced this issue Oct 12, 2017
lemonboston pushed a commit that referenced this issue Oct 12, 2017
dmfs pushed a commit that referenced this issue Oct 12, 2017
@dmfs dmfs closed this as completed Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants