-
Notifications
You must be signed in to change notification settings - Fork 534
Bring back TCK for 0.4.0 #91
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
Bring back TCK for 0.4.0 #91
Conversation
import java.util.HashSet; | ||
import java.util.Set; | ||
|
||
public abstract class IdentityProcessorVerification<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not yet fully cleaned up (contains tests from old tck)
Pushed some updates, the |
* @param t throwable to be matched for fatal-ness | ||
* @return true if is a non-fatal throwable, false otherwise | ||
*/ | ||
public static boolean apply(Throwable t) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given the Java audience I’d rename this one to isNonFatal
to be able to statically import it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(also made constructor private – no need for instances of NonFatal
)
Hi, I've been looking at Subscriber blackbox tests today. Has been pretty pain free so far, thanks for all this hard work! I wonder whether the TCK could provide the helper Publisher? Why does the implementator need to provide this? |
Making an agnostic publisher is not that easy as it should support a minimum scope of the rules itself. If that is an issue because the publisher in ratpack is tied to a web context, feel free to pull a dependency onto reactor-core 👍 (hehe) and use:
|
Good plug for Reactor :) Good idea though too, might be worth a mention in the readme? |
The initial idea of the If there's enough demand I think we could provide a "simpleIterablePublisher" but I'd opt for keeping the method abstract, so you actively think about and select what publisher you're running with. Opinions? |
I agree that the tester should supply the impl. On Sun, Sep 7, 2014 at 1:50 AM, Konrad Malawski [email protected]
|
if your impl is not able to support multiple subscribers
fcd5387
to
cc7f204
Compare
@reactive-streams/contributors Ping. Any opinions if we can merge this and perhaps release another milestone? We need a few more votes I believe. It would make things easier for devs who are now trying to implement their libraries, and test drive the TCK. |
I think it makes total sense to merge this and cut another milestone release of 0.4 as it'd make it much easier for implementors to try it out and give feedback. |
I would expect at least another implementor to use it. What about Ratpack and Vert.x @alkemist @purplefox ? |
We're already using it, if that's what you mean. |
Ha does it fully pass ? |
The tests that are there are passing yes. We have a few more tests to add though. |
If you don't have any specific feedbacks I think we have our 3 implementors to go for 0.4.0. We might wait for Vert.x and RxJava before 1.0 then :) |
@smaldini We were only talking about releasing an M3 of 0.4 so far :), and then get some extra feedback before we release 0.4, then we need to achieve the goals for 1.0 before we can ship 1.0-RC1. I'm really proud of what we have achieved so far! |
I think nothing prevents us to go for 0.4 but if the scope is only M3 we should really do it like today :) |
@smaldini I'd be happy with a 0.4.0-M3 today :) |
@benjchristensen @purplefox @tmontgomery @mariusaeriksen we need two more LGTM in order to proceed with this (i.e. merge it and release the artifacts as 0.4.0.M3 so that further TCK validation can proceed more easily), is there anything holding this back? |
Nothing I'm aware of but I haven't had time yet to try it. I imagine I'll want the new artifacts when I try so I'm good releasing since this isn't 1.0. |
I'll defer to you guys on this. |
All right I guess we have our two chaps. Let's accept the Konrad PR and proceed then. |
So great to see this merged, thanks a lot for all the feedback and cooperation guys (and for the upcoming feedback once others try it :-))! |
Thanks to you to make it real :) that was great to just polish a couple detail and beta test the beast. Really impressive work. Waiting for Maven Central to sync on 0.4.0 |
Thanks everyone, I can't believe how close we are now! :) |
Initial draft of 0.4.0 TCK.
Submitting this PR "early", as in it's not completely done, but I want to get your feedback as soon as possible.
Please have a look at
PublisherVerification
andSubscriberVerification
first - the specs are documented and link to the respective points in the spec.IdentityProcessorVerification
delegates to subscriber and publisher tests, as well as contains some additional specs which I have not yet reviewed if they make sense under the 0.4.0.M2 spec.Some initial ideas around #87 are also included in this PR, and I would welcome opinions about it. The idea is around
maxElementsFromPublisher
, which implementors can override, and then if it's 1 for example, but my test requires at least 3 elements, we can mark this test as skipped.In order to run the TCK against your implementations (please do report if I got anything wrong) you can publish it locally:
sbt publishLocal
(to local ivy repo) orsbt publishM2
(to local maven repo).This PR aims to eventually resolve #85
Also addresses "limited publishers"- #87
Thanks in advance for your feedback and comments!
Depends on:
Implementations passing the TCK: