-
Notifications
You must be signed in to change notification settings - Fork 534
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
Removes a partial sentence in the javadoc #370
The head ref may contain hidden characters: "wip-278-remove-partial-doc-\u221A"
Conversation
@@ -128,7 +128,6 @@ public long maxElementsFromPublisher() { | |||
|
|||
/** | |||
* Override and return {@code true} in order to skip executing tests marked as {@code Stochastic}. | |||
* Such tests MAY sometimes fail even though the impl |
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.
Or simply paraphrase the docs from tck/README:
Stochastic tests can yield false positives ("be green") even if for some case, the given implementation may violate the tested behaviour. If for some reason, a stochastic test fails for your implementation, yet you are confident the implementation is correct you can disable them using this flag. Please contact the working group on /~https://github.com/reactive-streams/reactive-streams-jvm in case you encounter such issue with the tests.
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.
Good point. I'll add:
"Stochastic in this case means that the Rule is impossible or infeasible to deterministically verify— usually this means that this test case can yield false positives ("be green") even if for some case, the given implementation may violate the tested behaviour."
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.
Proposed a wording that would be more useful for us in the long run, WDYT?
LGTM either way, but would prefer to include the note I proposed.
535f59a
to
b1e5b1d
Compare
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.
LGTM, sounds good :)
The original issue #278 has been open for almost 2 years with no one else commenting.
Improving the status quo by removing the partial sentence for now.