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

["Daniel P. Berrangé" <berrange@redhat.com>]

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.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|