> On 1 Jun 2017, at 13:45, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > Hi Pavel, > > I agree it would be good to have a test :-( > These kind of complex state logic have a tendency to > bite you if you only test them by code reading.
True. The thing is I would refrain from writing a black-box test in this case. The reason is that it would be very non-obvious. I think we can get away with descending to a appropriate level of abstraction and test Receiver directly as if it was the target system. I will have to provide a suitable implementation of RawChannel which would allow us to mimic different corner cases though. >> I tried to rewrite the state machine the way that both fixes this bug and >> makes >> the code easier to follow (I'm biased here obviously). > > Can we throw an InternalError or assert if > 124 reader.readFrame(data, frameConsumer); > does not advance the buffer position? Makes sense. Done in place. > 131 break; > hmmm... what if demand.get() is no longer 0 when we reach this line? That would be fine, Daniel. > what if the cooperative handler already thinks that the current thread > will read the data? Is that possible? CooperativeHandler does not care about what it executes. It simply makes sure that for each call to `handle`, there will be a subsequent execution. If we go one level up, then we will see there's a enclosing (hidden) `while` that actually calls `pushContinuously`. So no harm here. The `while` loop in `pushContinuously` exists _mainly_ to make sure we eventually exhaust either data or demand. Whichever comes first. Does it make any sense?
