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

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

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.

>
>
>>> +-- 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.