google
Decide and document how inOrder() works with Multimap
Currently, it checks that the keySet is in the expected order and that the values for each key are in the expected order.
This is different than checking whether all entries are in the expected order, at least in the strange case of LinkedListMultimap (often used for URL query parameters).
This distinction is probably not important to most users. Still, we should decide which way we want to do it, and we should document accordingly.
Current status: https://truth.dev/api/1.0/com/google/common/truth/MultimapSubject.html
The ordering test (keys, then values in each key) is documented on:
containsAtLeastEntriesIn(Multimap)
containsExactlyEntriesIn(Multimap)
but is not documented on:
containsExactly(Object, Object, Object...)
containsAtLeast(Object, Object, Object...)
See also internal bug 111197955. It seems likely that we'll at least try to change the behavior to actually check the order of entries.
I definitely think we need to clear this up one way or another.
Right now, the javadoc for containsAtLeastEntriesIn says:
A subsequent call to Ordered.inOrder() may be made if the caller wishes to verify that the entries are present in the same order as given. That is, the keys are present in the given order in the key set, and the values for each key are present in the given order order in the value collections.
I find this confusing since (in the general case, and specifically for LinkedListMultimap) the two sentences seem to contradict each other (and the first sentence seems inaccurate).
For containsExactlyEntiresIn it says:
A subsequent call to Ordered.inOrder() may be made if the caller wishes to verify that the two multimaps iterate fully in the same order. That is, their key sets iterate in the same order, and the value collections for each key iterate in the same order.
I guess that's a little better since the first sentence is kind of ambiguous rather than inaccurate...
The Fuzzy Truth versions (assertThat(multimap).comparingValuesUsing(correspondence).containsExactlyEntriesIn(expected).inOrder()) have the other behaviour, i.e. it already enforces order of entries. This wasn't deliberate, and should be rationalized.
A couple of things to remember if we do make the change to the stricter assertion.
One is that isEqualTo for ListMultimaps delegates to containsExactly().inOrder() for its failure messages, so we need to think about the consequences on that. (The failure messages aren't great as it is, see internal bug 111198059.)
Another is that Multimap.toString() groups entries by key, so you can have instances that are not equal according to the strict entries-iterate-in-order assertion but which have the same toString(), so we'd probably have to stop using toString().
I actually think there's an argument to be made for opting for the weaker semantic. I suspect (though I have no proof, of course) that most folks probably think that if two instances are equal according to equals and toString (which both use the weaker semantic) then they're effectively the same thing, and would expect all equality-like assertions about them to pass. So I suspect that most users wouldn't actually want the stronger semantic. They can always call entries() and assert about that if they want — arguably, doing so would be more readable as it's explicit about the semantic.
I'm not saying that I'm definitely arguing that, just that I think the debate is worth having. It would be interesting to hear from any users who have an expectation one way or the other.
(It may come down to whether your mental model of ListMultimap is "it's like a Map with List values, but with a more helpful API" or not. LinkedListMultimap is proof that this mental model isn't quite accurate, but I still suspect that it is how a lot of folks think.)
Yes, I may have put things too strongly. This is one of those situations in which I plan, before thinking about this too much, to exploit our giant codebase by testing the behavior change against the whole depot and seeing what happens.
But while we're at it.... :)
We've definitely tried to steer people away from the Map<K, Collection<V>> interpretation of Multimap, but it hasn't always worked. And at times I feel a little guilty about even trying, since it's very clear that, say, a SortedSetMultimap is not "a sorted set of entries" -- and it's closer to "a SortedSet collection of values for each key." But in principle "collection of entries" is still more accurate, even if that collection sometimes happens to be grouped by key and then sorted within each group. (And even ListMultimap doesn't override entries() to return a List, presumably for performance reasons.)
One thing about LinkedListMultimap is that it's what's used by our internal UriParameterMap. UriParameterMap has its own toString() that is just the query string, so it does show the difference between a=1&b=2&a=3 and a=1&a=3&b=2. Plus, we have UriSubject.queryParameters(), which returns a MultimapSubject for the UriParameterMap. So maybe the ordering will change test behavior somewhere. Even if it does change behavior, though, it's somewhat hard to believe that the difference between a=1&b=2&a=3 and a=1&a=3&b=2 is significant to a real application....