Re: [PATCH 06/11] test-util-sockets: Test the complete abstract socket matrix

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Oct 29, 2020 at 02:38:28PM +0100, Markus Armbruster wrote:
>> The test covers only two out of nine combinations.  Test all nine.
>> Four turn out to be broken.  Marked /* BUG */.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/test-util-sockets.c | 86 ++++++++++++++++++++++++++++-----------
>>  1 file changed, 63 insertions(+), 23 deletions(-)
>> 
>> diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
>> index c2802f69ee..f8b6586e70 100644
>> --- a/tests/test-util-sockets.c
>> +++ b/tests/test-util-sockets.c
>> @@ -230,59 +230,99 @@ static void test_socket_fd_pass_num_nocli(void)
>>  #endif
>>  
>>  #ifdef __linux__
>> +
>> +#define ABSTRACT_SOCKET_VARIANTS 3
>> +
>> +typedef struct {
>> +    SocketAddress *server, *client[ABSTRACT_SOCKET_VARIANTS];
>> +    bool expect_connect[ABSTRACT_SOCKET_VARIANTS];
>> +} abstract_socket_matrix_row;
>
> snip
>
>>  
>> -static void test_socket_unix_abstract_good(void)
>> +static void test_socket_unix_abstract(void)
>>  {
>> -    SocketAddress addr;
>> +    SocketAddress addr, addr_tight, addr_padded;
>> +    abstract_socket_matrix_row matrix[ABSTRACT_SOCKET_VARIANTS] = {
>> +        { &addr,
>> +          { &addr_tight, &addr_padded, &addr },
>> +          { false /* BUG */, true /* BUG */, true } },
>> +        { &addr_tight,
>> +          { &addr_padded, &addr, &addr_tight },
>> +          { false, false /* BUG */, true } },
>> +        { &addr_padded,
>> +          { &addr, &addr_tight, &addr_padded },
>> +          { true /* BUG */, false, true } }
>> +    };
>
> This is overloading multiple separate tests in one, which is bad for
> test result reporting.
>
>
>> @@ -329,8 +369,8 @@ int main(int argc, char **argv)
>>      }
>>  
>>  #ifdef __linux__
>> -    g_test_add_func("/util/socket/unix-abstract/good",
>> -                    test_socket_unix_abstract_good);
>> +    g_test_add_func("/util/socket/unix-abstract",
>> +                    test_socket_unix_abstract);
>
> We should instead be registering multiple test funcs, passing in
> param to say which scenario to test.  This ensures we still see
> the test name reflecting which scenario is being run.

There are hundreds of such test functions in tests/ just waiting for you
to create them ;)

Back to serious.  Before the patch, one test function tested two
scenarios.  The patch adds the missing seven.  Feel free to improve on
top.