From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id E9447309E4 for ; Wed, 5 Jun 2019 17:53:10 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yJBf6QksoJHR for ; Wed, 5 Jun 2019 17:53:10 -0400 (EDT) Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B2161309DD for ; Wed, 5 Jun 2019 17:53:09 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 1/5] test: create isolated ev_loop for swim unit tests References: <50a84d1ffdd25646894e576ef4ff19195aaf769e.1559433539.git.v.shpilevoy@tarantool.org> <20190605065155.GB28736@atlas> From: Vladislav Shpilevoy Message-ID: <1b959ddd-ea04-c018-a8a9-cb8dba2b042f@tarantool.org> Date: Wed, 5 Jun 2019 23:53:05 +0200 MIME-Version: 1.0 In-Reply-To: <20190605065155.GB28736@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Konstantin Osipov Cc: tarantool-patches@freelists.org Hi! Thanks for the review. On 05/06/2019 09:51, Konstantin Osipov wrote: > * Vladislav Shpilevoy [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;