Re: [Tarantool-patches] [PATCH v4] core: introduce evenly distributed int64 random in range

["Ilya Kosarev" <i.kosarev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]


Hi!

Sent v5 of the patch with fixes. 
>Четверг, 12 ноября 2020, 1:07 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
> 
>Hi! Thanks for the fixes!
>
>I hope we will end this eventually :)
>
>See 2 comments below.
>
>> diff --git a/test/unit/random.c b/test/unit/random.c
>> new file mode 100644
>> index 000000000..858ff7867
>> --- /dev/null
>> +++ b/test/unit/random.c
>> @@ -0,0 +1,56 @@
>> +#include <core/random.h>
>> +
>> +#include <stdio.h>
>> +#include <assert.h>
>> +
>> +#include "unit.h"
>> +
>> +static void
>> +test_random_in_range_one(int64_t min, int64_t max)
>> +{
>> + int64_t result = pseudo_random_in_range(min, max);
>> + assert(min <= result && result <= max);
>> + if (min == max)
>> + printf("pseudo_random_in_range(%lld, %lld) = %lld\n",
>> + (long long)min, (long long)max, (long long)result);
>> +
>> + result = real_random_in_range(min, max);
>> + assert(min <= result && result <= max);
>> + if (min == max)
>> + printf("real_random_in_range(%lld, %lld) = %lld\n",
>> + (long long)min, (long long)max, (long long)result);
>
>1. By using printf as a checker you complicate Sergey's life in scope
>of this  https://github.com/tarantool/tarantool/issues/5000 . Because your
>output is not TAP. Better not print anything except via helpers from
>unit.h, and use ok()/is()/isnt() for actual tests.
Thanks, didn’t really know this one.
>
>> +}
>> diff --git a/test/unit/swim_test_utils.c b/test/unit/swim_test_utils.c
>> index 9dbd28a9f..7ac43d82d 100644
>> --- a/test/unit/swim_test_utils.c
>> +++ b/test/unit/swim_test_utils.c
>> @@ -875,7 +875,8 @@ swim_run_test(const char *log_file, fiber_func test)
>> * Print the seed to be able to reproduce a bug with the
>> * same seed.
>> */
>> - say_info("Random seed = %llu", (unsigned long long) seed);
>> + say_info("Random seed = %llu", (unsigned long long)seed);
>
>2. Why did you change this line? 
Yes, no need to change it
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 2876 bytes --]