Delay

[beastgrim]

Hello! I've done some work for "Delay" feature.
It's not complete yet because I need advice for some troubles.
First: It can't be completed until "FlatMap" is done.
Second: I can't finish tests for "Delay" because we don't have RunLoop in OpenCombine.
But I've tested all functionality on iOS and all works as expected with this implementation (all existed tests pass).

Best regards!

[broadwaylamb]

Hello @beastgrim!

I've quickly looked at your implementation.

I don't really think we need FlatMap here. You can look at how other publishers like Map are implemented: in the receive(subcriber:) method they create an object of some private type Inner which acts as both Subscriber and Subscription.

In case of Delay, I imagine that in the receive(_ input: Input) method of that Inner object sould just call scheduler.schedule(after:tolerance:options:action:), without any FlatMap or PassthroughSubject.

We must keep those Inner objects as lightweight as possible in order for the framework to be efficient.

Also can I ask you to fix the conflicts?

[broadwaylamb]

I also don't think you need RunLoop in your tests. Using RunLoop in tests makes those test asynchronous, which automatically means less stable and slower.

My idea was to use a custom-crafted synchronous Scheduler, something like RxSwift's VirtualTimeScheduler, which uses virtual clock with priority queue for scheduling actions. It works as follows:

1. We schedule an action. The scheduler inserts it into its priority queue.
2. We explicitly start the scheduler and it executes the actions synchronously, according to their priority (i. e. the time they were scheduled to execute at).

This will allow us to not depend on RunLoop and make our tests rubost.

[beastgrim]

@broadwaylamb I'm going to do all that you recommended. First I will implement VirtualTimeScheduler and complete the tests and then complete implementation of Delay. But I think VirtualTimeScheduler (our tests) can't cover all async behaviour of Delay that I found and there is may some bugs because of that. Maybe I'm wrong. Anyway, we will return to this when everything is ready. Thanks for help!

[broadwaylamb]

But I think VirtualTimeScheduler (our tests) can't cover all async behaviour of Delay that I found and there is may some bugs because of that.

All the asynchrony in Delay is abstracted away by the Scheduler protocol, so we can test all the observable behavior of Delay just by plugging in some mock scheduler instead of an actual scheduler. Basically we just want to make sure that our Delay implementation makes exactly the same calls to the scheduler as Combine's implementation.

[OpenCombineBot]

LGTM

Generated by 🚫 Danger Swift against c6d5df4

[beastgrim]

@broadwaylamb Take a look on current progress please.
I implemented VirtualTimeScheduler, all tests is synchronous now and all tests pass like before.
But one thing I can't understand, how can I use our "Delay" with "RunLoop"?
I've tried to implement extension for RunLoop
extension RunLoop: OpenCombine.Scheduler { ... }
but it's seems unreal, because of conflicting type names. Your expertise is required.
Thanks!

[broadwaylamb]

Thanks @beastgrim, I’ll take a look as soon as I have some time.

I’ve just merged #46, you can take a look at how I’ve implemented DispatchQueue’s conformance to the Scheduler protocol. Tl;dr: not without a certain compromise.

[broadwaylamb]

I've created #114 based on your work with more tests, and also refined some things, so I'm gonna close this. Thank you for the PR!