Re: [Tarantool-patches] [PATCHv2 1/2] core: add setting error injections via env

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hello,

thanks for patch!

small nit: with patch below alignment looks more accurately.

--- a/test/box-tap/errinj_set_with_enviroment_vars.test.lua
+++ b/test/box-tap/errinj_set_with_enviroment_vars.test.lua
@@ -4,8 +4,8 @@ local fio = require('fio')
  -- Execute errinj_set_with_enviroment_vars_script.lua
  -- via tarantool with presetted environment variables.
  local TARANTOOL_PATH = arg[-1]
-local bool_env = 'ERRINJ_TESTING=true ERRINJ_WAL_IO=True 
ERRINJ_WAL_SYNC=TRUE' ..
-                ' ERRINJ_WAL_WRITE=false ERRINJ_INDEX_ALLOC=False 
ERRINJ_WAL_WRITE_DISK=FALSE'
+local bool_env = 'ERRINJ_TESTING=true ERRINJ_WAL_IO=True 
ERRINJ_WAL_SYNC=TRUE ' ..
+                 'ERRINJ_WAL_WRITE=false ERRINJ_INDEX_ALLOC=False 
ERRINJ_WAL_WRITE_DISK=FALSE'
  local integer_env = 'ERRINJ_WAL_WRITE_PARTIAL=2 
ERRINJ_WAL_FALLOCATE=+2 ERRINJ_WAL_WRITE_COUNT=-2'
  local double_env = 'ERRINJ_VY_READ_PAGE_TIMEOUT=2.5 
ERRINJ_VY_SCHED_TIMEOUT=+2.5 ERRINJ_RELAY_TIMEOUT=-2.5'
  local set_env_str = ('%s %s %s'):format(bool_env, integer_env, double_env)

On 24.02.2021 12:38, Artem wrote:
> Hi,
>
> 20.02.2021 17:27, Sergey Bronnikov пишет:
>> Hi,
>>
>> On 20.02.2021 14:21, Artem wrote:
>>> Hello,
>>>
>>> thanks for the review!
>>>
>>>>> +
>>>>> +void errinj_set_with_environment_vars() {
>>>>> +    for (enum errinj_id i = 0; i < errinj_id_MAX; i++) {
>>>>> +        struct errinj *inj = &errinjs[i];
>>>>> +        const char *env_value = getenv(inj->name);
>>>>> +        if (!env_value || *env_value == '\0')
>>>>> +            continue;
>>>>> +
>>>>> +        if (inj->type == ERRINJ_INT) {
>>>>> +            char *end;
>>>>> +            int64_t int_value = strtoll(env_value, &end, 10);
>>>>> +            if (*end == '\0')
>>>>> +                inj->iparam = int_value;
>>>>> +        } else if (inj->type == ERRINJ_BOOL) {
>>>>> +            if (strcmp(env_value, "false") == 0 || 
>>>>> strcmp(env_value, "FALSE") == 0)
>>>>
>>>> 1. What happen if someone set boolean value to "False"?
>>>>
>>>> May be it's worth to convert env_value to lowercase and then 
>>>> compare? The same question for "true".
>>>
>>> 1. Done. Case-insensetive value can be set to env variable now.
>>
>> Thanks!
>>
>> It's worth to add different values for 'true'/'false' to test
>>
>> for documentation and regression purposes.
>
> Done, added tests for "true", "True", "TRUE" (same for false) and 
> tests for numeric injections with plus and minus signs.
>
> Updates you can see on the branch.
>