Hi, I recently noticed that some very useful assertion methods are not 
available to use in the IterableAssert​ interface.  In particular, I wanted to 
add containsInAnyOrder(SerializableMatcher... matchers)​.  I did this in 
https://github.com/apache/beam/pull/15685.   However, this causes some tests to 
not compile because they use containsInAnyOrder()​ to mean empty()​, which 
causes the compiler to give up because the method is now ambiguous with 
containsInAnyOrder(T... elements)​.

It seems like there are a few potential ways forward

  *   Commit the breaking change and tell users to use empty()​ instead of no 
arguments to containsInAnyOrder​.
  *   Change the signature to containsInAnyOrder(SerializableMatcher<T> 
firstMatcher, SerializableMatcher<T>... rest)​
  *   Change the signature to 
containsInAnyOrder(Collection<SerializableMatcher<T>> matchers)​.
  *   Do nothing
  *   Something else

To me, the first two ways forward are the best.  empty()​ is more descriptive 
of what is being asserted than containsInAnyOrder()​.  But if that isn't 
tolerable, then the second should remove the ambiguity.

In any case, please let me know if you have feelings about what should be done.
[https://opengraph.githubassets.com/f93657a3199653b0d47e7f5cc35f08f7fa1fe20691c4e17e13dcdd02e2276bb9/apache/beam/pull/15685]<https://github.com/apache/beam/pull/15685>
[BEAM-13019] Add `containsInAnyOrder` with matchers to the `IterableAssert` 
interface by chrismgrayftsinc · Pull Request #15685 · 
apache/beam<https://github.com/apache/beam/pull/15685>
This lets an existing assertion be usable in the PAssert class. Thank you for 
your contribution! Follow this checklist to help us incorporate your 
contribution quickly and easily: Choose reviewe...
github.com


Reply via email to