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

required_spec208_mustBePreparedToReceiveOnNextSignalsAfterHavingCalledSubscriptionCancel does not wait for request before invoking onNext #277

Closed
jroper opened this issue Jun 29, 2015 · 3 comments

Comments

@jroper
Copy link
Contributor

commented Jun 29, 2015

  @Override @Test
  public void required_spec208_mustBePreparedToReceiveOnNextSignalsAfterHavingCalledSubscriptionCancel() throws Throwable {
    subscriberTest(new TestStageTestRun() {
      @Override
      public void run(WhiteboxTestStage stage) throws InterruptedException {
        stage.puppet().triggerRequest(1);
        stage.puppet().signalCancel();
        stage.signalNext();

        stage.puppet().triggerRequest(1);
        stage.puppet().triggerRequest(1);

        stage.verifyNoAsyncErrors();
      }
    });
  }

Note, it triggers the request, but then does not wait for the request to arrive at the subscription before signalling next. This violates the spec, and is causing my tests to fail, since my subscriber setup and request triggering is asynchronous, so my subscriber under test is not necessarily ready to receive elements before requesting them.

@jroper

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2015

required_spec308_requestMustRegisterGivenNumberElementsToBeProduced also has the same problem:

  public void required_spec308_requestMustRegisterGivenNumberElementsToBeProduced() throws Throwable {
    subscriberTest(new TestStageTestRun() {
      @Override
      public void run(WhiteboxTestStage stage) throws InterruptedException {
        stage.puppet().triggerRequest(2);
        stage.probe.expectNext(stage.signalNext());
        stage.probe.expectNext(stage.signalNext());

        stage.probe.expectNone();
        stage.puppet().triggerRequest(3);

        stage.verifyNoAsyncErrors();
      }
    });
  }
@ktoso

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2015

Thanks for reporting James! What you observe indeed looks wrong, I'll give it a look today evening.

@jroper

This comment has been minimized.

Copy link
Contributor Author

commented Nov 18, 2018

Here's an instance of the TCK failing against Akka Streams due to this bug:

akka/akka#25927

jroper added a commit to jroper/reactive-streams-jvm that referenced this issue Nov 18, 2018
Wait for demaind before signalling onNext
Fixes reactive-streams#277.

Fixes two whitebox subscriber tests where they weren't waiting for
demand before signalling onNext.
jroper added a commit to jroper/reactive-streams-jvm that referenced this issue Nov 18, 2018
Wait for demand before signaling onNext
Fixes reactive-streams#277.

Fixes two whitebox subscriber tests where they weren't waiting for
demand before signaling onNext.
viktorklang added a commit that referenced this issue Dec 6, 2018
Wait for demand before signaling onNext (#441)
* Wait for demand before signaling onNext

Fixes #277.

Fixes two whitebox subscriber tests where they weren't waiting for
demand before signaling onNext.

* Added unit tests ensuring that requests have been expected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.