Skip to content

+tck #362 wait for request signal in 209, and new additional tests #374

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 16, 2017
Merged

+tck #362 wait for request signal in 209, and new additional tests #374

merged 3 commits into from
Jun 16, 2017

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented May 29, 2017

Another cleanup to things found in #362

Now we test both cases, not waiting for request and waiting for it explicitly.
Simplified setup a bit as well for existing tests in 209, while problem was pointed out about 210

@ktoso
Copy link
Contributor Author

ktoso commented May 29, 2017

Note from other PR:

We (attempt to) cover both cases now, without explicitly waiting for the request which could mean we issue before the request happens, and with explicitly waiting, which means it always is after the request signal. We use the TCK infra to force a request be made.

@ktoso ktoso mentioned this pull request May 29, 2017
Merged
@viktorklang
Copy link
Contributor

@ktoso this fails the build:

 java.lang.ClassCastException: org.reactivestreams.tck.SubscriberWhiteboxVerification$BlackboxSubscriberProxy cannot be cast to org.reactivestreams.tck.support.SyncTriggeredDemandSubscriber

@viktorklang
Copy link
Contributor

Ping @ktoso

@ktoso
Copy link
Contributor Author

ktoso commented Jun 14, 2017

Will get to this during my day

@ktoso
fix tests by using proxied subscriber,
fcd1533 
thanks Viktor for helping push this fix
@ktoso
Copy link
Contributor Author

ktoso commented Jun 16, 2017

Pushed the fix, thanks for the reminders @viktorklang !

ktoso
ktoso commented Jun 16, 2017
View reviewed changes
tck/src/main/java/org/reactivestreams/tck/support/SubscriberBlackboxVerificationRules.java Outdated
* does.
* <p>
* <b>Verifies rule:</b> <a href='https://github.com/reactive-streams/reactive-streams-jvm#2.11'>2.11</a>
*/
Copy link
Contributor Author

@ktoso ktoso Jun 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right, forgot to add the new-style-comment; thanks a ton for doing so.
Comments sound good, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Np! Thanks for all your hard work, @ktoso!

@viktorklang viktorklang merged commit 940a51f into reactive-streams:master Jun 16, 2017
@viktorklang viktorklang added this to the 1.0.1 milestone Jun 16, 2017
@ktoso ktoso deleted the wip-209-210-improve branch June 16, 2017 14:09
@ktoso ktoso mentioned this pull request Jun 26, 2017
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viktorklang viktorklang viktorklang left review comments

@akarnokd akarnokd akarnokd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.1
Development

Successfully merging this pull request may close these issues.
3 participants
@ktoso @viktorklang @akarnokd