Start of Multimap Assertions

[mattbertolini]

This is some of the work started from the disucssion in this Eclipse Collections thread. It begins work on assertions for the Multimap type

[scordio]

@mattbertolini thanks for raising this PR!

It's of course up to you whether to take care of the Javadoc in this PR or as a second step, so feel free to ignore the failing job.

Just let us know how we can help. Personally, I'm happy to provide a general review, although my Eclipse Collections expertise is rather limited.

[mattbertolini]

@scordio Thank you for all of your work setting this up!

I think I'm going to ignore the javadoc in this PR. It's already way too big for my usual comfort. I'd like to land what I have right now to establish the package structure and initial skeleton.

[mattbertolini]

@scordio I've fixed all of the remaining Javadoc warnings. The build should pass. I could use a review and an approval. I realize its a lot of code. Let me try and describe what I'm trying to do and hopefully that will help guide the review.

My general goal is to have these assertions "feel" like AssertJ assertions. I can imagine a world where a user of AssertJ is starting to migrate from the JCF to Eclipse Collections and wants that migration to be as painless as possible. This means keeping the APIs familiar to the core APIs. Having the assertions be more or less drop in replacements seems like a good idea to me. Also, having them map to pre-existing names makes sense if this module is living in the AssertJ organization.

The bulk of the implementation of this PR lives in AbstractMultimapAssert. It is modeled after AssertJ-core's AbstractMapAssert. I haven't finished implementing all of the methods but the goal is to implement as many as is feasible or makes sense in the context of a Multimap. I'm attempting to leverage as much existing AssertJ-core code as possible. This means reusing ErrorMessageFactory when possible. I've also created some new ErrorMessageFactory implementations where it isn't possible to reuse (e.g. the Multimap's distinct size assertions).

The assertion methods themselves try to leverage as much Eclipse Collections APIs as possible. I am trying to stay away from the existing JCF classes and lean into the power and conciseness of the Eclipse Collections.

I've created 3 concrete implementations of this assertion: BagMultimap, ListMultimap, and SetMultimap. Right now they don't do much except leverage their abstract parent class. I can see a future where some additional assertions go onto these classes but it's too early to tell. There may be more implementations but I haven't full explored the type hierarchy of Multimap yet.

I've also created the basic Assertions, SoftAssertions, and BDDAssertions types (still need to make BDDSoftAssertions. This is to keep the API the same as the core assertions.

For testing, I went with the JUnit 5 test interface pattern with an interface for each assertion method. The interface represents the contract test for that assertion. The interface is then implemented by the three implementations for execution purposes. For example, the hasSize method will have an AbstractMultimapAssert_HasSize_Contract test interface and the three implementations will have *_HasSize_Test classes (e.g. ListMultimapAssert_HasSize_Test):

// Current pattern of testing
public interface AbstractMultimapAssert_HasSize_Contract {
  // ...
}

class ListMultimapAssert_HasSize_Test implements AbstractMultimapAssert_HasSize_Contract {
  // ..
}

I don't love the pattern. There's quite a bit of duplication involved. But I think having all of the assertions in one test class seems more unwieldy than navigating the test interfaces. I've also thought about using nested test classes that implement the test interfaces. That way There's one "test class" that has all of the contract tests inside of them:

// Not done in this PR but I've thought about this
class ListMultimapAssertTest {
  @Nested
  class HasSize implements AbstractMultimapAssert_HasSize_Contract {
    // ...
  }
}

I'm open to suggestions on testing so I'd be curious to hear everyone's thoughts on it.

I want to stress that the AbstractMultimapAssert is far from complete but at over 6000 lines this PR is plenty too big. I'd like to merge this and iterate in much smaller chunks going forward. Landing this PR gives me the basic class structure needed to do that.

Let me know if you have any questions or concerns. 🙂

[scordio]

Hi @mattbertolini, many thanks for your work!

We're now finalizing the first milestone of AssertJ version 4 – I'll jump on your PR as soon as the milestone is released, which should ideally happen in the upcoming days.

[scordio]

Hi @mattbertolini, I just wanted to let you know that we haven't forgotten about this PR 🙃

Our M1 release is out, but we have to focus on our documentation for some technical changes, which will also impact this module.

However, we would like to proceed with your onboarding to the AssertJ team.

Can you please email me and we will take it from there? You should be able to see my address in the git commit history.

[scordio]

Thanks for the good work, @mattbertolini!

I am trying to stay away from the existing JCF classes and lean into the power and conciseness of the Eclipse Collections.

I fully support that 👍 go for the best solution you can leverage in each assertion implementation.

I've created 3 concrete implementations of this assertion: BagMultimap, ListMultimap, and SetMultimap. Right now they don't do much except leverage their abstract parent class.

I propose postponing the introduction of these concrete types and starting with a single MultimapAssert (more in my review comment).

For testing, I went with the JUnit 5 test interface pattern with an interface for each assertion method.

I'm an intensive user of that pattern, so I understand the flexibility you're looking for 🙃 However, if my proposal of having a single concrete class is feasible (at least for the time being), you might be able to achieve the same result, perhaps even more concise, with parameterized tests. See for example the testing strategy used in Maps_assertContainsKeys_Test.

[mattbertolini]

I've updated the PR with the suggestions and ready for another round of review 😄