[PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests

[PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests

[Anderson Lizardo]

This is made possible by adding the mtu parameter, given
/TP/SERVER/BRW/* tests use MTU of 672.
---
 unit/test-sdp.c |   24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 315a5cd..e9cbcdf 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -68,34 +68,22 @@ struct test_data {
 		.cont_len = cont,				\
 	}
 
-#define define_test(name, args...) \
+#define define_test(name, _mtu, args...) \
 	do {								\
 		const struct sdp_pdu pdus[] = {				\
 			args, { }, { }					\
 		};							\
 		static struct test_data data;				\
-		data.mtu = 48;						\
+		data.mtu = _mtu;					\
 		data.pdu_list = g_malloc(sizeof(pdus));			\
 		memcpy(data.pdu_list, pdus, sizeof(pdus));		\
 		g_test_add_data_func(name, &data, test_sdp);		\
 	} while (0)
 
-#define define_ss(name, args...) define_test("/TP/SERVER/SS/" name, args)
-#define define_sa(name, args...) define_test("/TP/SERVER/SA/" name, args)
-#define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, args)
-
-#define define_brw(name, args...) \
-	do {								\
-		const struct sdp_pdu pdus[] = {				\
-			args, { }, { }					\
-		};							\
-		static struct test_data data;				\
-		data.mtu = 672;						\
-		data.pdu_list = g_malloc(sizeof(pdus));			\
-		memcpy(data.pdu_list, pdus, sizeof(pdus));		\
-		g_test_add_data_func("/TP/SERVER/BRW/" name,		\
-						&data, test_sdp);	\
-	} while (0)
+#define define_ss(name, args...) define_test("/TP/SERVER/SS/" name, 48, args)
+#define define_sa(name, args...) define_test("/TP/SERVER/SA/" name, 48, args)
+#define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
+#define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
 
 struct context {
 	GMainLoop *main_loop;
-- 
1.7.9.5

[PATCH BlueZ 2/3] unit: Rename x_pdu() macro on SDP test program

[Anderson Lizardo]

Using the "raw_data" name makes sense, given the macro is just casting
input (raw) data. It will also be reused in other tests with raw input
data.

Also fix this minor checkpatch.pl error:

ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
parenthesis
+#define raw_data(args...) (const unsigned char[]) { args }
---
 unit/test-sdp.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index e9cbcdf..77a4c6c 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -51,20 +51,20 @@ struct test_data {
 	struct sdp_pdu *pdu_list;
 };
 
-#define x_pdu(args...) (const unsigned char[]) { args }
+#define raw_data(args...) ((const unsigned char[]) { args })
 
 #define raw_pdu(args...) \
 	{							\
 		.valid = true,					\
-		.raw_data = x_pdu(args),			\
-		.raw_size = sizeof(x_pdu(args)),		\
+		.raw_data = raw_data(args),			\
+		.raw_size = sizeof(raw_data(args)),		\
 	}
 
 #define raw_pdu_cont(cont, args...) \
 	{							\
 		.valid = true,					\
-		.raw_data = x_pdu(args),			\
-		.raw_size = sizeof(x_pdu(args)),		\
+		.raw_data = raw_data(args),			\
+		.raw_size = sizeof(raw_data(args)),		\
 		.cont_len = cont,				\
 	}
 
-- 
1.7.9.5

Re: [PATCH BlueZ 2/3] unit: Rename x_pdu() macro on SDP test program

[Marcel Holtmann]

Hi Anderson,

> Using the "raw_data" name makes sense, given the macro is just casting
> input (raw) data. It will also be reused in other tests with raw input
> data.
> 
> Also fix this minor checkpatch.pl error:
> 
> ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in
> parenthesis
> +#define raw_data(args...) (const unsigned char[]) { args }
> ---
>  unit/test-sdp.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

patch has been applied.

Regards

Marcel

[PATCH v2 BlueZ 3/3] unit: Implement tests for sdp_extract_attr()

[Anderson Lizardo]

These tests are for valid data. Other tests might be added later to
check for error paths, but will require separate macros.

As example of failure output, if commit
504a0cf46ad89cab8005ce9cffb22e41048f6a30 is reverted for testing, the
STR16 test will fail with:

ERROR:unit/test-sdp.c:776:test_sdp_de_attr: assertion failed
(test->input_size == size): (7 == 11)
---
 unit/test-sdp.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 77a4c6c..d511ef4 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -85,6 +85,29 @@ struct test_data {
 #define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
 #define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
 
+/* SDP Data Element (DE) tests */
+struct test_data_de {
+	const void *input_data;
+	size_t input_size;
+	sdp_data_t expected;
+};
+
+#define exp_data(_dtd, val_type, val_data) \
+	((const sdp_data_t) {			\
+		.dtd = _dtd,			\
+		.val.val_type = val_data,	\
+	})
+
+#define define_test_de_attr(name, input, exp) \
+	do {								\
+		static struct test_data_de data;			\
+		data.input_data = input;				\
+		data.input_size = sizeof(input);			\
+		data.expected = exp;					\
+		g_test_add_data_func("/sdp/DE/ATTR/" name, &data,	\
+						test_sdp_de_attr);	\
+	} while (0)
+
 struct context {
 	GMainLoop *main_loop;
 	guint server_source;
@@ -742,6 +765,35 @@ static void test_sdp(gconstpointer data)
 	g_free(test->pdu_list);
 }
 
+static void test_sdp_de_attr(gconstpointer data)
+{
+	const struct test_data_de *test = data;
+	sdp_data_t *d;
+	int size = 0;
+
+	d = sdp_extract_attr(test->input_data, test->input_size, &size, NULL);
+	g_assert(d != NULL);
+	g_assert_cmpuint(test->input_size, ==, size);
+	g_assert_cmpuint(test->expected.dtd, ==, d->dtd);
+
+	if (g_test_verbose() == TRUE)
+		g_print("DTD=0x%02x\n", d->dtd);
+
+	switch (d->dtd) {
+	case SDP_TEXT_STR8:
+	case SDP_TEXT_STR16:
+	case SDP_URL_STR8:
+	case SDP_URL_STR16:
+		g_assert_cmpstr(test->expected.val.str, ==, d->val.str);
+		break;
+	/* TODO: implement other DTDs here */
+	default:
+		g_assert_not_reached();
+	}
+
+	sdp_data_free(d);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -2697,5 +2749,18 @@ int main(int argc, char *argv[])
 			0x08, 0x09, 0x00, 0x01, 0x35, 0x03, 0x19, 0x11,
 			0x06, 0x00));
 
+	/*
+	 * SDP Data Element (DE) tests
+	 *
+	 * Test extraction of valid DEs supported by sdp_extract_attr().
+	 */
+	define_test_de_attr("STR8",
+			raw_data(0x25, 0x04, 0x41, 0x42, 0x43, 0x44),
+			exp_data(SDP_TEXT_STR8, str, "ABCD"));
+	define_test_de_attr("STR16",
+			raw_data(0x26, 0x00, 0x04, 0x41, 0x42, 0x43, 0x44),
+			exp_data(SDP_TEXT_STR16, str, "ABCD"));
+	/* TODO: implement other tests for valid DEs */
+
 	return g_test_run();
 }
-- 
1.7.9.5

Re: [PATCH v2 BlueZ 3/3] unit: Implement tests for sdp_extract_attr()

[Marcel Holtmann]

Hi Anderson,

> These tests are for valid data. Other tests might be added later to
> check for error paths, but will require separate macros.
> 
> As example of failure output, if commit
> 504a0cf46ad89cab8005ce9cffb22e41048f6a30 is reverted for testing, the
> STR16 test will fail with:
> 
> ERROR:unit/test-sdp.c:776:test_sdp_de_attr: assertion failed
> (test->input_size == size): (7 == 11)
> ---
>  unit/test-sdp.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index 77a4c6c..d511ef4 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -85,6 +85,29 @@ struct test_data {
>  #define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
>  #define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
>  
> +/* SDP Data Element (DE) tests */
> +struct test_data_de {
> +	const void *input_data;
> +	size_t input_size;
> +	sdp_data_t expected;
> +};
> +
> +#define exp_data(_dtd, val_type, val_data) \
> +	((const sdp_data_t) {			\
> +		.dtd = _dtd,			\
> +		.val.val_type = val_data,	\
> +	})
> +
> +#define define_test_de_attr(name, input, exp) \
> +	do {								\
> +		static struct test_data_de data;			\
> +		data.input_data = input;				\
> +		data.input_size = sizeof(input);			\
> +		data.expected = exp;					\
> +		g_test_add_data_func("/sdp/DE/ATTR/" name, &data,	\
> +						test_sdp_de_attr);	\
> +	} while (0)
> +
>  struct context {
>  	GMainLoop *main_loop;
>  	guint server_source;
> @@ -742,6 +765,35 @@ static void test_sdp(gconstpointer data)
>  	g_free(test->pdu_list);
>  }
>  
> +static void test_sdp_de_attr(gconstpointer data)
> +{
> +	const struct test_data_de *test = data;
> +	sdp_data_t *d;
> +	int size = 0;
> +
> +	d = sdp_extract_attr(test->input_data, test->input_size, &size, NULL);
> +	g_assert(d != NULL);
> +	g_assert_cmpuint(test->input_size, ==, size);
> +	g_assert_cmpuint(test->expected.dtd, ==, d->dtd);
> +
> +	if (g_test_verbose() == TRUE)
> +		g_print("DTD=0x%02x\n", d->dtd);
> +
> +	switch (d->dtd) {
> +	case SDP_TEXT_STR8:
> +	case SDP_TEXT_STR16:
> +	case SDP_URL_STR8:
> +	case SDP_URL_STR16:
> +		g_assert_cmpstr(test->expected.val.str, ==, d->val.str);
> +		break;
> +	/* TODO: implement other DTDs here */

just remove both TODOs from the patch. I assume you will send updates on
this.

Regards

Marcel

Re: [PATCH v2 BlueZ 3/3] unit: Implement tests for sdp_extract_attr()

[Anderson Lizardo]

Hi Marcel,

On Wed, Jan 9, 2013 at 3:50 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> just remove both TODOs from the patch. I assume you will send updates on
> this.

Yes, I will prepare a series with tests for other valid input.

Using this macro for testing sequences is not possible given it is
implemented as a linked list. SEQ*/ALT* stuff need to be tested
separately.

Testing invalid input (and checking for error) is also important,
specially if it triggers some memory leak/corruption. But they will
use a different test function because it should always return a NULL
pointer instead.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests

[Marcel Holtmann]

Hi Anderson,

> This is made possible by adding the mtu parameter, given
> /TP/SERVER/BRW/* tests use MTU of 672.
> ---
>  unit/test-sdp.c |   24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index 315a5cd..e9cbcdf 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -68,34 +68,22 @@ struct test_data {
>  		.cont_len = cont,				\
>  	}
>  
> -#define define_test(name, args...) \
> +#define define_test(name, _mtu, args...) \
>  	do {								\
>  		const struct sdp_pdu pdus[] = {				\
>  			args, { }, { }					\
>  		};							\
>  		static struct test_data data;				\
> -		data.mtu = 48;						\
> +		data.mtu = _mtu;					\

so using _mtu instead of mtu was the trick here. For the heck of it I
could not figure that one out. So in the end I gave up on it and
hardcoded it :(

Patch has been applied.

Regards

Marcel

Re: [PATCH BlueZ 1/3] unit: Reuse define_test() macro for /TP/SERVER/BRW/* tests

[Anderson Lizardo]

Hi Marcel,

On Wed, Jan 9, 2013 at 3:08 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> -#define define_test(name, args...) \
>> +#define define_test(name, _mtu, args...) \
>>       do {                                                            \
>>               const struct sdp_pdu pdus[] = {                         \
>>                       args, { }, { }                                  \
>>               };                                                      \
>>               static struct test_data data;                           \
>> -             data.mtu = 48;                                          \
>> +             data.mtu = _mtu;                                        \
>
> so using _mtu instead of mtu was the trick here. For the heck of it I
> could not figure that one out. So in the end I gave up on it and
> hardcoded it :(

Yes, I was caught by it as well, until I realized that the
preprocessor might be replacing the "mtu" part of "data.mtu" (I
confirmed when I saw the cpp output).

>
> Patch has been applied.

Thanks,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil