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 minor test name fixup, it is a required test #219

Merged
merged 1 commit into from
Mar 12, 2015

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Feb 15, 2015

Trivial name fixup, introduced in https://github.com/reactive-streams/reactive-streams/pull/214/files#diff-4ebca955800e47695d157fcd89b4fec6R263
Notice that this rule is a required rule, thus the method name of the test should reflect this.

( see why such name here: https://github.com/reactive-streams/reactive-streams/pull/219/files#diff-4ebca955800e47695d157fcd89b4fec6R273 )

@viktorklang
Copy link
Contributor

Nice catch, LGTM

@@ -264,7 +264,7 @@ public void required_spec109_subscribeThrowNPEOnNullSubscriber_shouldFailIfDoesn
}

@Test
public void optional_spec109_mayRejectCallsToSubscribeIfPublisherIsUnableOrUnwillingToServeThemRejectionMustTriggerOnErrorAfterOnSubscribe_actuallyPass() throws Throwable {
public void required_spec109_mayRejectCallsToSubscribeIfPublisherIsUnableOrUnwillingToServeThemRejectionMustTriggerOnErrorAfterOnSubscribe_actuallyPass() throws Throwable {
customPublisherVerification(SKIP, new Publisher<Integer>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ktoso Why is it SKIP here tho?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's OK, because this test is for the error state publisher. customPublisherVerification first the normal publisher, then the error state publisher - we're testing the error state publisher here, thus only the 2nd param.

// In the test we have whenHasErrorPublisherTest(...)

@viktorklang
Copy link
Contributor

@smaldini We should make sure we get this into the RC3 as well

@rkuhn
Copy link
Member

rkuhn commented Mar 12, 2015

This was slated for RC3, merging for RC4 instead.

rkuhn added a commit that referenced this pull request Mar 12, 2015
=tck minor test name fixup, it is a required test
@rkuhn rkuhn merged commit be35ad3 into reactive-streams:master Mar 12, 2015
@rkuhn rkuhn deleted the fixup-minor branch March 12, 2015 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants