diff mbox

[PATCHv2,1/5] validation: shm: move main() to a common place

Message ID 1417598541-31897-2-git-send-email-taras.kondratiuk@linaro.org
State Accepted
Commit 8146540a14096d95240d91fae59b72f1598871b9
Headers show

Commit Message

Taras Kondratiuk Dec. 3, 2014, 9:22 a.m. UTC 
Most of test application will have the same main function.
Move main() from odp_shm to a common place where it can be reused by
other test applications.
Unifying main() will also simplify transition to a combined single test
application in future.

Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
Reviewed-by: Mike Holmes <mike.holmes@linaro.org>
---
 test/validation/Makefile.am               |    3 ++-
 test/validation/common/odp_cunit_common.c |   33 +++++++++++++++++++++++
 test/validation/common/odp_cunit_common.h |    9 +++++++
 test/validation/odp_shm.c                 |   42 ++---------------------------
 4 files changed, 46 insertions(+), 41 deletions(-)

Comments

Ciprian Barbu Dec. 3, 2014, 5:13 p.m. UTC | #1 
On Wed, Dec 3, 2014 at 3:45 PM, Stuart Haslam <stuart.haslam@arm.com> wrote:
> On Wed, Dec 03, 2014 at 09:22:17AM +0000, Taras Kondratiuk wrote:
>> Most of test application will have the same main function.
>> Move main() from odp_shm to a common place where it can be reused by
>> other test applications.
>> Unifying main() will also simplify transition to a combined single test
>> application in future.
>>
>
> A couple of very minor nits below, but otherwise the series looks good
> to me.
>
>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
>> Reviewed-by: Mike Holmes <mike.holmes@linaro.org>
>> ---
>>  test/validation/Makefile.am               |    3 ++-
>>  test/validation/common/odp_cunit_common.c |   33 +++++++++++++++++++++++
>>  test/validation/common/odp_cunit_common.h |    9 +++++++
>>  test/validation/odp_shm.c                 |   42 ++---------------------------
>>  4 files changed, 46 insertions(+), 41 deletions(-)
>>
>> diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
>> index 2632fb8..837ae95 100644
>> --- a/test/validation/Makefile.am
>> +++ b/test/validation/Makefile.am
>> @@ -1,5 +1,6 @@
>>  include $(top_srcdir)/test/Makefile.inc
>>
>> +AM_CFLAGS += -I$(srcdir)/common
>>  AM_LDFLAGS += -static
>>
>>  if ODP_CUNIT_ENABLED
>> @@ -10,7 +11,7 @@ odp_init_LDFLAGS = $(AM_LDFLAGS)
>>  odp_queue_LDFLAGS = $(AM_LDFLAGS)
>>  odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto
>>  odp_crypto_LDFLAGS = $(AM_LDFLAGS)
>> -odp_shm_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/common
>> +odp_shm_CFLAGS = $(AM_CFLAGS)
>>  odp_shm_LDFLAGS = $(AM_LDFLAGS)
>>  endif
>>
>> diff --git a/test/validation/common/odp_cunit_common.c b/test/validation/common/odp_cunit_common.c
>> index 885b981..c87e103 100644
>> --- a/test/validation/common/odp_cunit_common.c
>> +++ b/test/validation/common/odp_cunit_common.c
>> @@ -35,3 +35,36 @@ int odp_cunit_thread_exit(pthrd_arg *arg)
>>
>>       return 0;
>>  }
>> +
>> +int main(void)
>> +{
>> +     int ret;
>> +
>> +     printf("\tODP API version: %s\n", odp_version_api_str());
>> +     printf("\tODP implementation version: %s\n", odp_version_impl_str());
>> +
>> +     if (0 != odp_init_global(NULL, NULL)) {
>> +             printf("odp_init_global fail.\n");
>
> Shouldn't this go to stderr?

We started out by printing everything to stdout, but since this is a
common piece of code, kind of the only one that is supposed to print
stuff, then I second Stuart's proposal.

>
>> +             return -1;
>> +     }
>> +     if (0 != odp_init_local()) {
>> +             printf("odp_init_local fail.\n");
>
> Same here.
>
>> +             return -1;
>> +     }
>> +
>> +     CU_set_error_action(CUEA_ABORT);
>> +
>> +     CU_initialize_registry();
>> +     CU_register_suites(odp_testsuites);
>> +     CU_basic_set_mode(CU_BRM_VERBOSE);
>> +     CU_basic_run_tests();
>> +
>> +     ret = CU_get_number_of_failure_records();
>
> automake uses the exit code of 77 to indicate a skipped test, so if
> exactly 77 tests failed we'd get the wrong result.

That is at least interesting :-)

>
> --
> Stuart.
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Mike Holmes Dec. 3, 2014, 5:38 p.m. UTC | #2 
On 3 December 2014 at 12:13, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:

> On Wed, Dec 3, 2014 at 3:45 PM, Stuart Haslam <stuart.haslam@arm.com>
> wrote:
> > On Wed, Dec 03, 2014 at 09:22:17AM +0000, Taras Kondratiuk wrote:
> >> Most of test application will have the same main function.
> >> Move main() from odp_shm to a common place where it can be reused by
> >> other test applications.
> >> Unifying main() will also simplify transition to a combined single test
> >> application in future.
> >>
> >
> > A couple of very minor nits below, but otherwise the series looks good
> > to me.
> >
> >> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
> >> Reviewed-by: Mike Holmes <mike.holmes@linaro.org>
> >> ---
> >>  test/validation/Makefile.am               |    3 ++-
> >>  test/validation/common/odp_cunit_common.c |   33
> +++++++++++++++++++++++
> >>  test/validation/common/odp_cunit_common.h |    9 +++++++
> >>  test/validation/odp_shm.c                 |   42
> ++---------------------------
> >>  4 files changed, 46 insertions(+), 41 deletions(-)
> >>
> >> diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
> >> index 2632fb8..837ae95 100644
> >> --- a/test/validation/Makefile.am
> >> +++ b/test/validation/Makefile.am
> >> @@ -1,5 +1,6 @@
> >>  include $(top_srcdir)/test/Makefile.inc
> >>
> >> +AM_CFLAGS += -I$(srcdir)/common
> >>  AM_LDFLAGS += -static
> >>
> >>  if ODP_CUNIT_ENABLED
> >> @@ -10,7 +11,7 @@ odp_init_LDFLAGS = $(AM_LDFLAGS)
> >>  odp_queue_LDFLAGS = $(AM_LDFLAGS)
> >>  odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto
> >>  odp_crypto_LDFLAGS = $(AM_LDFLAGS)
> >> -odp_shm_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/common
> >> +odp_shm_CFLAGS = $(AM_CFLAGS)
> >>  odp_shm_LDFLAGS = $(AM_LDFLAGS)
> >>  endif
> >>
> >> diff --git a/test/validation/common/odp_cunit_common.c
> b/test/validation/common/odp_cunit_common.c
> >> index 885b981..c87e103 100644
> >> --- a/test/validation/common/odp_cunit_common.c
> >> +++ b/test/validation/common/odp_cunit_common.c
> >> @@ -35,3 +35,36 @@ int odp_cunit_thread_exit(pthrd_arg *arg)
> >>
> >>       return 0;
> >>  }
> >> +
> >> +int main(void)
> >> +{
> >> +     int ret;
> >> +
> >> +     printf("\tODP API version: %s\n", odp_version_api_str());
> >> +     printf("\tODP implementation version: %s\n",
> odp_version_impl_str());
> >> +
> >> +     if (0 != odp_init_global(NULL, NULL)) {
> >> +             printf("odp_init_global fail.\n");
> >
> > Shouldn't this go to stderr?
>
> We started out by printing everything to stdout, but since this is a
> common piece of code, kind of the only one that is supposed to print
> stuff, then I second Stuart's proposal.
>
> >
> >> +             return -1;
> >> +     }
> >> +     if (0 != odp_init_local()) {
> >> +             printf("odp_init_local fail.\n");
> >
> > Same here.
> >
> >> +             return -1;
> >> +     }
> >> +
> >> +     CU_set_error_action(CUEA_ABORT);
> >> +
> >> +     CU_initialize_registry();
> >> +     CU_register_suites(odp_testsuites);
> >> +     CU_basic_set_mode(CU_BRM_VERBOSE);
> >> +     CU_basic_run_tests();
> >> +
> >> +     ret = CU_get_number_of_failure_records();
> >
> > automake uses the exit code of 77 to indicate a skipped test, so if
> > exactly 77 tests failed we'd get the wrong result.
>
> That is at least interesting :-)
>

You are absolutely right I added return 77; and got

 ============================================================================
Testsuite summary for OpenDataPlane 0.3.0
============================================================================
# TOTAL: 4
# PASS:  3
# SKIP:  1
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================


> >
> > --
> > Stuart.
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk Dec. 4, 2014, 8:11 a.m. UTC | #3 
On 12/03/2014 03:45 PM, Stuart Haslam wrote:
> On Wed, Dec 03, 2014 at 09:22:17AM +0000, Taras Kondratiuk wrote:
>> Most of test application will have the same main function.
>> Move main() from odp_shm to a common place where it can be reused by
>> other test applications.
>> Unifying main() will also simplify transition to a combined single test
>> application in future.
>>
>
> A couple of very minor nits below, but otherwise the series looks good
> to me.

I agree with those comment, but those parts are preserved form the
original main() function. I'd better add these fixes as a follow
up patches. Will post them separately.
diff mbox

Patch

diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
index 2632fb8..837ae95 100644
--- a/test/validation/Makefile.am
+++ b/test/validation/Makefile.am
@@ -1,5 +1,6 @@ 
 include $(top_srcdir)/test/Makefile.inc
 
+AM_CFLAGS += -I$(srcdir)/common
 AM_LDFLAGS += -static
 
 if ODP_CUNIT_ENABLED
@@ -10,7 +11,7 @@  odp_init_LDFLAGS = $(AM_LDFLAGS)
 odp_queue_LDFLAGS = $(AM_LDFLAGS)
 odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto
 odp_crypto_LDFLAGS = $(AM_LDFLAGS)
-odp_shm_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/common
+odp_shm_CFLAGS = $(AM_CFLAGS)
 odp_shm_LDFLAGS = $(AM_LDFLAGS)
 endif
 
diff --git a/test/validation/common/odp_cunit_common.c b/test/validation/common/odp_cunit_common.c
index 885b981..c87e103 100644
--- a/test/validation/common/odp_cunit_common.c
+++ b/test/validation/common/odp_cunit_common.c
@@ -35,3 +35,36 @@  int odp_cunit_thread_exit(pthrd_arg *arg)
 
 	return 0;
 }
+
+int main(void)
+{
+	int ret;
+
+	printf("\tODP API version: %s\n", odp_version_api_str());
+	printf("\tODP implementation version: %s\n", odp_version_impl_str());
+
+	if (0 != odp_init_global(NULL, NULL)) {
+		printf("odp_init_global fail.\n");
+		return -1;
+	}
+	if (0 != odp_init_local()) {
+		printf("odp_init_local fail.\n");
+		return -1;
+	}
+
+	CU_set_error_action(CUEA_ABORT);
+
+	CU_initialize_registry();
+	CU_register_suites(odp_testsuites);
+	CU_basic_set_mode(CU_BRM_VERBOSE);
+	CU_basic_run_tests();
+
+	ret = CU_get_number_of_failure_records();
+
+	CU_cleanup_registry();
+
+	odp_term_local();
+	odp_term_global();
+
+	return ret;
+}
diff --git a/test/validation/common/odp_cunit_common.h b/test/validation/common/odp_cunit_common.h
index 71a3350..1f30788 100644
--- a/test/validation/common/odp_cunit_common.h
+++ b/test/validation/common/odp_cunit_common.h
@@ -13,8 +13,17 @@ 
 #ifndef ODP_CUNICT_COMMON_H
 #define ODP_CUNICT_COMMON_H
 
+#include "CUnit/Basic.h"
+
 #define MAX_WORKERS 32 /**< Maximum number of work threads */
 
+/**
+ * Array of testsuites provided by a test application. Array must be terminated
+ * by CU_SUITE_INFO_NULL and must be suitable to be used by
+ * CU_register_suites().
+ */
+extern CU_SuiteInfo odp_testsuites[];
+
 typedef struct {
 	uint32_t foo;
 	uint32_t bar;
diff --git a/test/validation/odp_shm.c b/test/validation/odp_shm.c
index bcd46c7..8a991b1 100644
--- a/test/validation/odp_shm.c
+++ b/test/validation/odp_shm.c
@@ -5,7 +5,6 @@ 
  */
 
 #include "odp.h"
-#include "CUnit/Basic.h"
 #include "odp_cunit_common.h"
 
 #define ALIGE_SIZE  (128)
@@ -71,50 +70,13 @@  static void test_odp_shm_sunnyday(void)
 	odp_cunit_thread_exit(&thrdarg);
 }
 
-static int init(void)
-{
-	printf("\tODP API version: %s\n", odp_version_api_str());
-	printf("\tODP implementation version: %s\n", odp_version_impl_str());
-	return 0;
-}
-
 CU_TestInfo test_odp_shm[] = {
 	{"test_odp_shm_creat",  test_odp_shm_sunnyday},
 	CU_TEST_INFO_NULL,
 };
 
-CU_SuiteInfo suites[] = {
-	{"odp_system", init, NULL, NULL, NULL, test_odp_shm},
+CU_SuiteInfo odp_testsuites[] = {
+	{"Shared Memory", NULL, NULL, NULL, NULL, test_odp_shm},
 	CU_SUITE_INFO_NULL,
 };
 
-
-int main(void)
-{
-	int ret;
-
-	if (0 != odp_init_global(NULL, NULL)) {
-		printf("odp_init_global fail.\n");
-		return -1;
-	}
-	if (0 != odp_init_local()) {
-		printf("odp_init_local fail.\n");
-		return -1;
-	}
-
-	CU_set_error_action(CUEA_ABORT);
-
-	CU_initialize_registry();
-	CU_register_suites(suites);
-	CU_basic_set_mode(CU_BRM_VERBOSE);
-	CU_basic_run_tests();
-
-	ret = CU_get_number_of_failure_records();
-
-	CU_cleanup_registry();
-
-	odp_term_local();
-	odp_term_global();
-
-	return ret;
-}