On Tue, 25 Jul 2017 15:59:06 +0200
Michael Teyfel <[email protected]> wrote:

> Two cases are tested: success and fail case of the screen-remove-layer API.
> 
> Signed-off-by: Michael Teyfel <[email protected]>
> ---
>  tests/ivi_layout-internal-test.c | 62 
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)

Hi Michael,

thanks for these, they look mostly good, just some polishing required.
Comments inline.

> 
> diff --git a/tests/ivi_layout-internal-test.c 
> b/tests/ivi_layout-internal-test.c
> index 37a2356..64380ec 100644
> --- a/tests/ivi_layout-internal-test.c
> +++ b/tests/ivi_layout-internal-test.c
> @@ -341,6 +341,66 @@ test_layer_source_rectangle(struct test_context *ctx)
>  }
>  
>  static void
> +test_screen_remove_layer(struct test_context *ctx)
> +{
> +     const struct ivi_layout_interface *lyt = ctx->layout_interface;
> +     struct ivi_layout_layer *ivilayer;
> +     struct weston_output *output;
> +     struct ivi_layout_layer **array;
> +     int32_t length = 0;
> +
> +     ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200, 
> 300);
> +     iassert(ivilayer != NULL);
> +
> +     if (wl_list_empty(&ctx->compositor->output_list))
> +             return;

This leaks the ivilayer, possibly affecting also further tests. Move
the check before the layer creation.

> +
> +     output = wl_container_of(ctx->compositor->output_list.next, output, 
> link);
> +
> +     iassert(lyt->screen_add_layer(output, ivilayer) == IVI_SUCCEEDED);
> +     lyt->commit_changes();
> +
> +     iassert(lyt->get_layers_on_screen(output, &length, &array) == 
> IVI_SUCCEEDED);
> +     iassert(length == 1);
> +     iassert(array[0] == ivilayer);
> +
> +     iassert(lyt->screen_remove_layer(output ,ivilayer) == IVI_SUCCEEDED);

Space on the wrong side of comma.

> +     lyt->commit_changes();
> +

'array' gets leaked.

> +     iassert(lyt->get_layers_on_screen(output, &length, &array) == 
> IVI_SUCCEEDED);
> +     iassert(length == 0);

'array' would get leaked if there were any layers, but since we fail
the test in that case, it doesn't matter.

> +
> +     lyt->layer_destroy(ivilayer);
> +}
> +
> +static void
> +test_screen_bad_remove_layer(struct test_context *ctx)
> +{
> +     const struct ivi_layout_interface *lyt = ctx->layout_interface;
> +     struct ivi_layout_layer *ivilayer;
> +     struct weston_output *output;
> +
> +     ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200, 
> 300);
> +     iassert(ivilayer != NULL);
> +
> +     if (wl_list_empty(&ctx->compositor->output_list))
> +             return;

The same leak of ivilayer as before.

> +
> +     output = wl_container_of(ctx->compositor->output_list.next, output, 
> link);
> +
> +     iassert(lyt->screen_remove_layer(NULL, ivilayer) == IVI_FAILED);
> +     lyt->commit_changes();
> +
> +     iassert(lyt->screen_remove_layer(output, NULL) == IVI_FAILED);
> +     lyt->commit_changes();
> +
> +     iassert(lyt->screen_remove_layer(NULL, NULL) == IVI_FAILED);
> +     lyt->commit_changes();
> +
> +     lyt->layer_destroy(ivilayer);
> +}
> +
> +static void
>  test_layer_bad_remove(struct test_context *ctx)
>  {
>       const struct ivi_layout_interface *lyt = ctx->layout_interface;
> @@ -951,6 +1011,8 @@ run_internal_tests(void *data)
>       test_layer_position(ctx);
>       test_layer_destination_rectangle(ctx);
>       test_layer_source_rectangle(ctx);
> +     test_screen_remove_layer(ctx);
> +     test_screen_bad_remove_layer(ctx);

These two would be better grouped with the other screen tests, I think.

>       test_layer_bad_remove(ctx);
>       test_layer_bad_visibility(ctx);
>       test_layer_bad_opacity(ctx);


Thanks,
pq

Attachment: pgpjuuuLU6znk.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to