mdedetrich commented on code in PR #1207:
URL: https://github.com/apache/incubator-pekko/pull/1207#discussion_r1529789529


##########
stream-tests/src/test/scala/org/apache/pekko/stream/scaladsl/FlowSplitAfterSpec.scala:
##########
@@ -250,10 +248,10 @@ class FlowSplitAfterSpec extends StreamSpec("""
       upstreamSubscription.sendNext(6)
       substreamPuppet1.expectNext(6)
       substreamPuppet1.expectComplete()
+      upstreamSubscription.sendNext(7)

Review Comment:
   > @mdedetrich in the test, it requests 10, so the SubSource is expected to 
be there after 6.
   
   But its also sending a completion because of this condition `if (elem == 3) 
throw exc else elem % 3 == 0` (i.e. `6 % 3` == 0 so the split happens right 
after that) which is why the test is failing with
   
   ```
   Expected OnNext(_), yet no element signaled during 3 seconds
   java.lang.AssertionError: Expected OnNext(_), yet no element signaled during 
3 seconds
        at 
org.apache.pekko.stream.testkit.TestSubscriber$ManualProbe.expectNext(StreamTestKit.scala:398)
        at 
org.apache.pekko.stream.testkit.TestSubscriber$ManualProbe.expectNext(StreamTestKit.scala:389)
        at 
org.apache.pekko.stream.scaladsl.FlowSplitAfterSpec.$anonfun$new$11(FlowSplitAfterSpec.scala:251)
   ```
   
   at `val substream2 = subscriber.expectNext()` because there isn't going to 
be a `expectNext`, it's expecting a completion instead.
   
   Note that at this point, we are dealing with the normal logic of 
`SplitWhen`. The new functionality of skipping exceptions with 
`SupervisionDecider.resume` occurs earlier when `elem == 3` so we are past that 
and just testing happy path logic of `SplitAfter` which is not changed at all 
(and it shouldn't be either!).
   
   This is why I think that part of the test is written incorrectly. In fact 
you can argue it can even be removed since it has nothing to do with recovering 
from thrown exceptions with `SupervisionDecider.resume`, its testing something 
entirely different (but I would personally leave it there now so its consistent 
with the other tests).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to