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

BaseTestConsumer#assertValueSet doesn't verify that all listed values were received #6151

Closed
Stephan202 opened this issue Aug 9, 2018 · 1 comment

Comments

@Stephan202
Copy link
Contributor

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?

@akarnokd
Copy link
Member

It is meant to assert received values are all in the collection. You can make a stronger assumption by adding assertValueCount.

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