Re: [PATCH v4 4/6] unit: dbus: Add test for filter rule

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4901 bytes --]

Hi Jukka,

On 02/20/2015 06:16 AM, Jukka Rissanen wrote:
> Testing the _dbus1_format_rule() function.
> ---
>   Makefile.am            |   3 ++
>   unit/test-dbus-watch.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 112 insertions(+)
>   create mode 100644 unit/test-dbus-watch.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 36a680b..cea1b7a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -107,6 +107,7 @@ unit_tests = unit/test-unit \
>   			unit/test-dbus-util \
>   			unit/test-dbus-message \
>   			unit/test-dbus-service \
> +			unit/test-dbus-watch \
>   			unit/test-gvariant-util \
>   			unit/test-gvariant-message \
>   			unit/test-siphash \
> @@ -154,6 +155,8 @@ unit_test_dbus_util_LDADD = ell/libell-private.la
>
>   unit_test_dbus_service_LDADD = ell/libell-private.la
>
> +unit_test_dbus_watch_LDADD = ell/libell-private.la
> +
>   unit_test_gvariant_util_LDADD = ell/libell-private.la
>
>   unit_test_gvariant_message_LDADD = ell/libell-private.la
> diff --git a/unit/test-dbus-watch.c b/unit/test-dbus-watch.c
> new file mode 100644
> index 0000000..e1d3f9d
> --- /dev/null
> +++ b/unit/test-dbus-watch.c
> @@ -0,0 +1,109 @@
> +/*
> + *
> + *  Embedded Linux library
> + *
> + *  Copyright (C) 2011-2015  Intel Corporation. All rights reserved.
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <sys/wait.h>
> +#include <assert.h>
> +#include <time.h>
> +#include <stdio.h>
> +
> +#include <ell/ell.h>
> +#include "ell/dbus-private.h"
> +
> +#define DBUS_SERVICE_DBUS	"org.freedesktop.DBus"
> +#define DBUS_PATH_DBUS		"/org/freedesktop/DBus"
> +#define DBUS_INTERFACE_DBUS	"org.freedesktop.DBus"
> +#define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
> +
> +struct dbus_filter_data {
> +	struct l_dbus *dbus;
> +	l_dbus_message_func_t handle_func;
> +	l_dbus_watch_func_t disconnect_func;
> +	char *sender;
> +	char *path;
> +	char *interface;
> +	char *member;
> +	char *argument;
> +	void *user_data;
> +	l_dbus_destroy_func_t destroy_func;
> +};

Don't ever do this.  If you need to use an opaque data structure, ask 
yourself why.  If it is genuinely required, then expose that in the 
header file.  Do not re-define opaque data structures in a unit test.

> +
> +struct watch_test {
> +	const char *name;
> +	const char *expected;
> +};
> +
> +static const struct watch_test match_test = {
> +	.name = ":1.101",
> +	.expected = "type='signal',"
> +		"sender='org.freedesktop.DBus',"
> +		"path='/org/freedesktop/DBus',"
> +		"interface='org.freedesktop.DBus',"
> +		"member='NameOwnerChanged',"
> +		"arg0=':1.101'",
> +};
> +
> +static void setup_data(struct dbus_filter_data *data, const char *name)
> +{
> +	data->sender = l_strdup(DBUS_SERVICE_DBUS);
> +	data->path = l_strdup(DBUS_PATH_DBUS);
> +	data->interface = l_strdup(DBUS_INTERFACE_DBUS);
> +	data->member = l_strdup("NameOwnerChanged");
> +	data->argument = l_strdup(name);
> +}
> +
> +static void clean_data(struct dbus_filter_data *data)
> +{
> +	l_free(data->sender);
> +	l_free(data->path);
> +	l_free(data->interface);
> +	l_free(data->member);
> +	l_free(data->argument);
> +}
> +static void test_match(const void *test_data)
> +{
> +	const struct watch_test *test = test_data;
> +	struct dbus_filter_data data;
> +	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
> +
> +	setup_data(&data, test->name);

Use _dbus1_filter_data_get

> +
> +	_dbus1_format_rule(&data, rule, sizeof(rule));
> +
> +	assert(strcmp(rule, test->expected) == 0);
> +
> +	clean_data(&data);

Use _dbus1_filter_data_destroy

> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	l_test_init(&argc, &argv);
> +
> +	l_test_add("DBus filter", test_match, &match_test);

Add more cases please, especially focusing on each type of filter 
combination.  E.g. interface only, member only, interface + member, etc.

> +
> +	return l_test_run();
> +}
>

Regards,
-Denis