Re: [Tarantool-patches] [PATCH v20 3/7] box/func: fix modules functions restore

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

>>> diff --git a/test/box/CMakeLists.txt b/test/box/CMakeLists.txt
>>> index 06bfbbe9d..944831af2 100644
>>> --- a/test/box/CMakeLists.txt
>>> +++ b/test/box/CMakeLists.txt
>>> @@ -2,4 +2,6 @@ include_directories(${MSGPUCK_INCLUDE_DIRS})
>>>  build_module(function1 function1.c)
>>>  build_module(reload1 reload1.c)
>>>  build_module(reload2 reload2.c)
>>> +build_module(func_restore1 func_restore1.c)
>>> +build_module(func_restore2 func_restore2.c)
>>>  build_module(tuple_bench tuple_bench.c)
>>> diff --git a/test/box/func_restore.result b/test/box/func_restore.result
>>
>> 5. The test also passes if I just replace rlist_foreach_entry_safe with
>> rlist_foreach_entry_safe_reverse in the original code. Which means it
>> won't test anything in case we ever change how do we put the functions
>> to the list, or how we walk the list on reload.
> 
> The key moment here is not about the list traverse direction: that's
> why I use for_each macro instead of the broken cycle we had before.
> The order does matter to trigger the issue *without* this patch,
> ie to put functions in reverse order and force the recovery
> (as is done in my test, I even put a comment there).
> 
> With the new approach using list_for() it is doesn't matter now
> how exactly we traverse. If that is what you mean.

Nope, that is not what I mean. But I don't know how to say it
easier, even in Russian.

>> I propose you to make the test harder to bypass.
> 
> The original code restores only one last failed function and
> my test is done the way to trigger the issue. I would like to
> make it more harder to bypass but I don't see an other way.

Well, did you try? The test would fail regardless of the order, if
you would try to drop the middle function. Or both the first and
the last functions. Or try all the combinations.