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

Implement Reactive-Streams TCK #436

Closed
rhart opened this issue Sep 4, 2014 · 6 comments
Closed

Implement Reactive-Streams TCK #436

rhart opened this issue Sep 4, 2014 · 6 comments
Assignees

Comments

@rhart
Copy link
Member

@rhart rhart commented Sep 4, 2014

Implement the TCK to verify our Reactive-Streams Publishers and Subscribers.

The TCK is still a work in progress, see here for details and any feedback reactive-streams/reactive-streams-jvm#91

The SNAPSHOT repo is here http://repo.akka.io/snapshots/org/reactivestreams/reactive-streams-tck/

@rhart rhart self-assigned this Sep 4, 2014
rhart added a commit that referenced this issue Sep 4, 2014
@ldaley ldaley added the in progress label Sep 4, 2014
@ldaley ldaley added this to the release-0.9.9 milestone Sep 4, 2014
rhart added a commit that referenced this issue Sep 4, 2014
rhart added a commit that referenced this issue Sep 4, 2014
@ktoso ktoso mentioned this issue Sep 5, 2014
4 of 6 tasks complete
rhart added a commit that referenced this issue Sep 5, 2014
…TCK verification (#436)

Fix DefaultResponseTransmitter#transmitter Reactive-Streams TCK violations
rhart added a commit that referenced this issue Sep 5, 2014
…ation (#436)

Fix WebsocketBroadcastPublisher Reactive-Streams TCK violations
@rhart

This comment has been minimized.

Copy link
Member Author

@rhart rhart commented Sep 30, 2014

Is there anything left to do on this?

@ldaley

This comment has been minimized.

Copy link
Member

@ldaley ldaley commented Sep 30, 2014

I havne't really explored the TCK. Are there other tests we should be implementing?

@rhart

This comment has been minimized.

Copy link
Member Author

@rhart rhart commented Sep 30, 2014

There are 4 suites of tests to be extended by implementors:
PublisherVerification
SubscriberBlackboxVerification
SubscriberWhiteboxVerification
IdentityProcessorVerification

We have done PublisherVerification and SubscriberBlackboxVerification. We don't have any Processors and SubscriberWhiteboxVerification is optional; ".....it is hard and non-trivial for some implementations....however we do not expect all implementations to make use of the plain SubscriberWhiteboxVerification, instead using the SubscriberBlackboxVerification instead."

I guess we could have a go at SubscriberWhiteboxVerification. There are some optional tests that are not running and could be investigated as to why maybe. Not sure whether we want to test more complex stream combinations.

@ldaley

This comment has been minimized.

Copy link
Member

@ldaley ldaley commented Sep 30, 2014

Let's close it and look at more testing as we discover issues.

@ldaley ldaley closed this Sep 30, 2014
@ldaley ldaley removed the in progress label Sep 30, 2014
@ktoso

This comment has been minimized.

Copy link

@ktoso ktoso commented Oct 1, 2014

Hello guys,
I'll fill in a little bit of the details on which ones to prefer to implement (since it was not clear enough, I think we update the readme with this info I think).

So in general the spec that covers most of the spec is the IdentityProcessorVerification. This is because it is-a SubscriberWhiteboxVerification and is able to provide the "puppet" (the element steering the tests in the whitebox style tests). It doesn't seem like you guys implement processors, just websocket publishers and subscribers though right?

If so sticking with these tests makes sense - giving the whitebox tests a try would be a good idea though, as the blackbox tests cover not much of the spec's rules.

Hope this helps, feel free to ping if need help with the whitebox :)

@ldaley

This comment has been minimized.

Copy link
Member

@ldaley ldaley commented Oct 2, 2014

We have quite a few processors, but nothing that implements Processor.

Our impls are in: https://github.com/ratpack/ratpack/tree/master/ratpack-core/src/main/java/ratpack/stream/internal

We don't use Processor because these are wrapped up behind static factory methods here: http://www.ratpack.io/manual/current/api/ratpack/stream/Streams.html

I'm going to reopen this ticket, because it seems like we need to add more test coverage.

@ldaley ldaley reopened this Oct 2, 2014
@ldaley ldaley modified the milestones: release-0.9.10, release-0.9.9 Oct 2, 2014
@danhyun danhyun modified the milestones: release-0.9.11, release-0.9.10 Nov 1, 2014
@ldaley ldaley modified the milestones: release-0.9.11, release-0.9.12 Dec 1, 2014
@ldaley ldaley modified the milestones: release-0.9.12, release-0.9.13 Jan 1, 2015
@ldaley ldaley modified the milestones: release-0.9.14, release-0.9.15 Mar 1, 2015
@ldaley ldaley modified the milestones: release-0.9.15, release-0.9.16 Apr 1, 2015
@rhart rhart modified the milestones: release-0.9.17, release-0.9.16 May 1, 2015
@ldaley ldaley closed this Jun 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.