[tarantool-patches] Re: [PATCH 1/5] test: create isolated ev_loop for swim unit tests

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review.

On 05/06/2019 09:51, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/06/03 14:33]:
>> --- a/src/lib/swim/swim_ev.c
>> +++ b/src/lib/swim/swim_ev.c
>> @@ -55,3 +55,9 @@ swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *watcher)
>>  {
>>  	ev_timer_stop(loop, watcher);
>>  }
>> +
>> +struct ev_loop *
>> +swim_loop(void)
> 
> The comment explaining why you need a separate loop should be
> here, not in the tests, since this is the place most people will
> be looking at and wondering why you need this wrapper at all.
> 
> You could hack this around with a define, but I think your
> approach is more clean, so please just add a comment.
> 

It can't be solved with define, because I need swim.o. I can't
postpone preprocessor work till linking time. Otherwise we could
just implement every function in swim_ev.h and swim_transport.h
as a macros.

The comment is moved and slightly modified.

=======================================================================

diff --git a/src/lib/swim/swim_ev.h b/src/lib/swim/swim_ev.h
index 900be150f..37e743d45 100644
--- a/src/lib/swim/swim_ev.h
+++ b/src/lib/swim/swim_ev.h
@@ -52,6 +52,21 @@ swim_ev_timer_again(struct ev_loop *loop, struct ev_timer *watcher);
 void
 swim_ev_timer_stop(struct ev_loop *loop, struct ev_timer *watcher);
 
+/**
+ * The unit tests code with the fake events and time does lots of
+ * forbidden things: it manually invokes pending watcher
+ * callbacks; manages global time without a kernel; puts not
+ * existing descriptors into the loop. All these actions does not
+ * affect the loop until yield. On yield a scheduler fiber wakes
+ * up and 1) infinitely generates EV_READ on not existing
+ * descriptors because considers them closed; 2) manual pending
+ * callbacks invocation asserts, because it is not allowed for
+ * non-scheduler fibers. To avoid these problems a new isolated
+ * loop is created, not visible for the scheduler. Here the fake
+ * events library can rack and ruin whatever it wants. This
+ * function is supposed to be an alias for 'loop()' in the
+ * Tarantool core, but be an isolated object in tests.
+ */
 struct ev_loop *
 swim_loop(void);
 
diff --git a/test/unit/swim_test_ev.c b/test/unit/swim_test_ev.c
index fb25ac9e4..23d909b05 100644
--- a/test/unit/swim_test_ev.c
+++ b/test/unit/swim_test_ev.c
@@ -63,17 +63,9 @@ typedef void (*swim_event_process_f)(struct swim_event *, struct ev_loop *);
 typedef void (*swim_event_delete_f)(struct swim_event *);
 
 /**
- * The unit tests code with the fake events and time does lots of
- * forbidden things: it manually invokes pending watcher
- * callbacks; manages global time without a kernel; puts not
- * existing descriptors into the loop. All these actions does not
- * affect the loop until yield. On yield a scheduler fiber wakes
- * up and 1) infinitely generates EV_READ on not existing
- * descriptors because considers them closed; 2) manual pending
- * callbacks invocation asserts, because it is not allowed for
- * non-scheduler fibers. To avoid these problems a new isolated
- * loop is created, not visible for the scheduler. Here the fake
- * events library can rack and ruin whatever it wants.
+ * An isolated event loop not visible to the fiber scheduler,
+ * where it is safe to use fake file descriptors, manually invoke
+ * callbacks etc.
  */
 static struct ev_loop *test_loop;