Issue #2552 Skip Executing Actions if the Future is cancelled. #2608
Conversation
There was a problem hiding this comment.
Hi! Thank you for the PR. The implementation looks good to me!
It is a good sign that the unit tests still run ;)
But we need at least one additional unit test in FutureTest that covers the issue #2552. Ideally, you take the example the reporter gave and make a test out of it. Sometimes it is hard to test concurrency. Please take a look at the other tests. If you need help please give me a sign and I will assist you!
Thanks!
Daniel
| @@ -376,6 +376,9 @@ boolean tryComplete(Try<? extends T> value) { | |||
| if (waiters != null) { | |||
| waiters.forEach(this::unlock); | |||
| } | |||
| if(isCancelled()) { | |||
There was a problem hiding this comment.
(I don't want to be a nitpicker but you missed one space if(is... --> if (is...)
|
@charvakcpatel007 add the missed space to this branch so it can be merged. |
|
Better late than ever - this change is looking good, but we still need a test that proves that the new functionality works |
|
@charvakcpatel007 are you willing to finish it? we just need tests |
Fixes #2552