-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Description
BaseTestConsumer#assertValueSet is documented and implemented as:
/**
* Assert that the TestObserver/TestSubscriber received only the specified values in any order.
* <p>This helps asserting when the order of the values is not guaranteed, i.e., when merging
* asynchronous streams.
*
* @param expected the collection of values expected in any order
* @return this
*/
@SuppressWarnings("unchecked")
public final U assertValueSet(Collection<? extends T> expected) {
if (expected.isEmpty()) {
assertNoValues();
return (U)this;
}
for (T v : this.values) {
if (!expected.contains(v)) {
throw fail("Value not in the expected collection: " + valueAndClass(v));
}
}
return (U)this;
}Within our team we have several tests using assertValueSet, each attempting to assert that all of the listed values are emitted. Today we found out that assertValueSet verifies a weaker condition, namely that no "unexpected" values are emitted. It does not verify that all listed values are emitted. This took us by surprise. If this is the intended behaviour, perhaps the documentation could be clarified on this point.
NB: the RxJava code base itself contains several tests using assertValueSet. Some of these also seem to assume the stronger interpretation (e.g. FlowableFlatMapSingleTest#normalAsync), while others would surely fail under the stronger semantics (e.g.FlowableFlatMapSingleTest#takeAsync).
NB2: I do observe that (nearly?) all tests in the RxJava code base invoking assertValueSet involve some form of nondeterminism, and a benefit of the current weaker semantics is that these tests are now less likely to fail on a system under heavy load. Still, as-is assertValueSet also passes if no values are emitted; surely these tests wish to assert something more definitive?