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

TCK: optional_spec105_emptyStreamMustTerminateBySignallingOnComplete requests 1 #350

Closed
akarnokd opened this issue Apr 4, 2017 · 9 comments

Comments

@akarnokd
Copy link
Contributor

akarnokd commented Apr 4, 2017

The test implementation requests 1 from a supposedly empty Publisher, but based on other tests and the likely interpretation of the spec (plus what @viktorklang expressed here), a Publisher should emit an onComplete or onError without a call to request after any previous onNext items were requested/consumed. For example, a length 3 finite Publisher, when requesting 3 should emit 3 items and complete without issuing any further request() calls.

  // Verifies rule: https://github.com/reactive-streams/reactive-streams-jvm#1.5
  @Override @Test
  public void optional_spec105_emptyStreamMustTerminateBySignallingOnComplete() throws Throwable {
    optionalActivePublisherTest(0, true, new PublisherTestRun<T>() {
      @Override
      public void run(Publisher<T> pub) throws Throwable {
        ManualSubscriber<T> sub = env.newManualSubscriber(pub);
        sub.request(1);
        sub.expectCompletion();
        sub.expectNone();
      }
    });
  }
@ktoso
Copy link
Contributor

ktoso commented Apr 28, 2017

a Publisher should emit an onComplete or onError without a call to request

Nowhere in the spec is such eagerness required.

The test is designed to pass with both "eagerly completing" Publishers as well as "lazily completing once they notice they don't have anything to signal" Publishers.

I don't see your point why this is wrong, it is valid and inclusive to many implementations.

I'm sure you've seen plenty use cases for both behaviours.

@akarnokd
Copy link
Contributor Author

Okay. I'll check my assumption about the TCK tests if all of them accept such lazy completing source and get back to you.

@viktorklang
Copy link
Contributor

@akarnokd Shall we close this Issue?

@akarnokd
Copy link
Contributor Author

akarnokd commented Dec 1, 2017

It's fine. There are a couple of operators, such as takeWhile(Predicate) which does require a request, otherwise it can't complete as the predicate requires an item to check.

@akarnokd akarnokd closed this as completed Dec 1, 2017
@viktorklang
Copy link
Contributor

@akarnokd True. But assuming a test which only needs 2 elements, providing a source of only 2 elements and a takeWhile(_ => true), shouldn't that onComplete even after 2 elements?

@akarnokd
Copy link
Contributor Author

akarnokd commented Dec 1, 2017

Depends on what the upstream of takeWhile() is. If it is something like the example RangePublisher, it will push an onComplete without further requests (after it emitted onNexts in response to requests).

@viktorklang
Copy link
Contributor

@akarnokd Yep, and since this is a Publisher test it seems tractable to provide a RangePublisher + takeWhile(_ => true) a sa Publisher? :)

@akarnokd
Copy link
Contributor Author

akarnokd commented Dec 1, 2017

Do you mean adding an example takeWhile or adding a TCK-only test with a takeWhile behavior?

@viktorklang
Copy link
Contributor

@akarnokd No, I meant that it is possible to TCK-validate takeWhile using the solution of passing a RangePublisher with the takeWhile connected to it if the RangePublisher only provides as many elements as will be passed on by takeWhile, since onComplete will be sent after the last element.

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

3 participants