Re: [Tarantool-patches] [PATCH 4/8] test: introduce raft unit tests

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the review!

>>>> diff --git a/test/unit/raft.c b/test/unit/raft.c
>>>> new file mode 100644
>>>> index 000000000..dfb5f8e43
>>>> --- /dev/null
>>>> +++ b/test/unit/raft.c
>>>> +
>>>> +static void
>>>> +raft_test_vote_skip(void)
>>>> +{
>>>> +    raft_start_test(33);
>>>> +    struct raft_node node;
>>>> +    raft_node_create(&node);
>>>> +
>>>> +    /* Everything is skipped if the term is outdated. */
>>>
>>> 1. Let's also test a case when vote response has greater term than the candidate itself.
>> Good idea. Below is the diff (without .result file diff, to be
>> short).
>>
>> ====================
>> @@ -346,7 +346,7 @@ raft_test_vote(void)
>>   static void
>>   raft_test_vote_skip(void)
>>   {
>> -    raft_start_test(33);
>> +    raft_start_test(37);
>>       struct raft_node node;
>>       raft_node_create(&node);
>>   @@ -507,6 +507,35 @@ raft_test_vote_skip(void)
>>         raft_node_cfg_is_candidate(&node, true);
>>   +    /*
>> +     * Vote response with a bigger term must be skipped, but it will bump
>> +     * the term.
>> +     */
>> +
>> +    /* Re-create the node so as not to write the vclock each time. */
>> +    raft_node_destroy(&node);
>> +    raft_node_create(&node);
>> +
>> +    raft_run_next_event();
>> +    is(node.raft.state, RAFT_STATE_CANDIDATE, "became candidate");
>> +    is(node.raft.term, 2, "term is bumped");
>> +
>> +    is(raft_node_send_vote_response(&node,
>> +        3 /* Term. */,
>> +        1 /* Vote. */,
>> +        2 /* Source. */
>> +    ), 0, "message is accepted");
>> +
>> +    ok(raft_node_check_full_state(&node,
>> +        RAFT_STATE_CANDIDATE /* State. */,
>> +        0 /* Leader. */,
>> +        3 /* Term. */,
>> +        1 /* Vote. */,
>> +        3 /* Volatile term. */,
>> +        1 /* Volatile vote. */,
>> +        "{0: 3}" /* Vclock. */
>> +    ), "term is bumped and became candidate");
>> +
>>       raft_node_destroy(&node);
>>       raft_finish_test();
>>   }
>> ====================
> 
> 
> I'd also set the quorum to 2 to make sure the node doesn't become leader in the old term
> 
> and that it doesn't count votes from a bigger term, but maybe that's an overkill for this test.
> 
> So, up to you.

Sounds good, added:

====================
@@ -525,6 +525,11 @@ raft_test_vote_skip(void)
 	/* Re-create the node so as not to write the vclock each time. */
 	raft_node_destroy(&node);
 	raft_node_create(&node);
+	/*
+	 * Set quorum to 2 to ensure the node does not count the bigger-term
+	 * vote and doesn't become a leader.
+	 */
+	raft_node_cfg_election_quorum(&node, 2);
 
 	raft_run_next_event();