Daniel P. Berrangé <berra...@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 <arm...@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.
