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

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

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.


>> +-- Execute errinj_set_with_enviroment_vars_script.lua
>> +-- via tarantool with presetted environment variables.
>> +local TARANTOOL_PATH = arg[-1]
>> +local set_env_str = 'ERRINJ_TESTING=true ERRINJ_WAL_WRITE_PARTIAL=1 
>> ERRINJ_VY_READ_PAGE_TIMEOUT=2.5'
>> +local script_file = fio.pathjoin(
>
> 2. variable name should reflect a purpose of variable.
>
> name is about script file, when variable contains a path to a file.
2. Done. Renamed to path_to_test_file.