Skip to content

Fail test in case of double onError/onComplete signals #122

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

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Oct 7, 2014

While updating some Akka streams tests @patriknw discovered we accidentally called onError twice on the overflow protection. We believe onError should be a "final" call, according to https://github.com/reactive-streams/reactive-streams#1.7 (the situational rules mentioned in this rule don't apply here IMO).

This addition should help to notice such implementation mistakes sooner. Please review @reactive-streams/contributors (I somehow can't get this to be a "ping" hm...).

This PR changes:

  • spec317_mustSignalOnErrorWhenPendingAboveLongMaxValue to check that no other errors than the one expected error were signalled
  • in general protects from multiple onError and onComplete calls on the same subscriber. The situational rule linked from 1.7 still holds as it states "onError if onComplete failed", which is still allowed with this update of course (we only disallow multiple onError / onComplete calls)

@ktoso ktoso force-pushed the impr-fail-tck-when-onError-twice-ktoso branch from deb5a31 to 6aac3f9 Compare October 7, 2014 10:21
@viktorklang
Copy link
Contributor

It's not a belief, it's mandatory :)

"7 Once a terminal state has been signaled (onError, onComplete) it is REQUIRED that no further signals occur. Situational scenario MAY apply [see 2.13]"

As for 2.13, see: https://github.com/reactive-streams/reactive-streams/pull/117/files

@patriknw
Copy link
Contributor

patriknw commented Oct 8, 2014

LGTM, thanks

@jbrisbin
Copy link

👍

@viktorklang
Copy link
Contributor

@reactive-streams/contributors Ample time has been given to comment, please give a 👍 or object to this change before the 24th of October GMT+0. If no one objects before that then this will be merged.

@viktorklang
Copy link
Contributor

@reactive-streams/contributors Merging without objections.

viktorklang added a commit that referenced this pull request Oct 24, 2014
…toso

Fail test in case of double onError/onComplete signals
@viktorklang viktorklang merged commit 095467f into reactive-streams:master Oct 24, 2014
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

Successfully merging this pull request may close these issues.

4 participants