| Message ID | 1422914560-8870-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 02/03/2015 12:02 AM, Mike Holmes wrote: > init_global can only be called once per executable, split the > init_global testing into multiple executables. > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > v2 > fix copyright year > remove uneeded defines > > test/validation/Makefile.am | 4 +-- > test/validation/odp_init.c | 39 --------------------- > test/validation/odp_init_log.c | 78 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 80 insertions(+), 41 deletions(-) > create mode 100644 test/validation/odp_init_log.c > > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am > index 2942b85..3ee59c8 100644 > --- a/test/validation/Makefile.am > +++ b/test/validation/Makefile.am > @@ -6,10 +6,10 @@ AM_LDFLAGS += -static > TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} > > if test_vald > -TESTS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > +TESTS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > endif > > -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > +bin_PROGRAMS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto > odp_buffer_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/buffer > odp_classification_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/classification > diff --git a/test/validation/odp_init.c b/test/validation/odp_init.c > index c8e5a11..82f8849 100644 > --- a/test/validation/odp_init.c > +++ b/test/validation/odp_init.c > @@ -11,10 +11,6 @@ > #define DEFAULT_MSG_POOL_SIZE (4*1024*1024) > #define DEFAULT_MSG_SIZE (8) > > -int replacement_logging_used; > - > -static int odp_init_log(odp_log_level_e level , const char *fmt, ...); > - > static void test_odp_init_global(void) > { > int status; > @@ -25,27 +21,8 @@ static void test_odp_init_global(void) > CU_ASSERT(status == 0); > } > > -static void test_odp_init_global_replace_log(void) > -{ > - int status; > - struct odp_init_t init_data; > - > - init_data.log_fn = &odp_init_log; > - > - replacement_logging_used = 0; > - > - status = odp_init_global(&init_data, NULL); > - CU_ASSERT_FATAL(status == 0); > - > - CU_ASSERT_TRUE(replacement_logging_used); For replacement_logging_used to be true implementation have to call log function at least once in odp_init_global(). Maybe add a note just above to make this assumption is obvious? It would be even better to explicitly call some odp_*_print() function before, to be sure that log function is called.
On 3 February 2015 at 04:54, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 02/03/2015 12:02 AM, Mike Holmes wrote: > >> init_global can only be called once per executable, split the >> init_global testing into multiple executables. >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> --- >> v2 >> fix copyright year >> remove uneeded defines >> >> test/validation/Makefile.am | 4 +-- >> test/validation/odp_init.c | 39 --------------------- >> test/validation/odp_init_log.c | 78 ++++++++++++++++++++++++++++++ >> ++++++++++++ >> 3 files changed, 80 insertions(+), 41 deletions(-) >> create mode 100644 test/validation/odp_init_log.c >> >> diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am >> index 2942b85..3ee59c8 100644 >> --- a/test/validation/Makefile.am >> +++ b/test/validation/Makefile.am >> @@ -6,10 +6,10 @@ AM_LDFLAGS += -static >> TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} >> >> if test_vald >> -TESTS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run >> odp_buffer odp_system odp_timer odp_time odp_synchronizers >> odp_classification >> +TESTS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule >> odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers >> odp_classification >> endif >> >> -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_schedule >> odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers >> odp_classification >> +bin_PROGRAMS = odp_init odp_init_log odp_queue odp_crypto odp_shm >> odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time >> odp_synchronizers odp_classification >> odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto >> odp_buffer_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/buffer >> odp_classification_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/classification >> diff --git a/test/validation/odp_init.c b/test/validation/odp_init.c >> index c8e5a11..82f8849 100644 >> --- a/test/validation/odp_init.c >> +++ b/test/validation/odp_init.c >> @@ -11,10 +11,6 @@ >> #define DEFAULT_MSG_POOL_SIZE (4*1024*1024) >> #define DEFAULT_MSG_SIZE (8) >> >> -int replacement_logging_used; >> - >> -static int odp_init_log(odp_log_level_e level , const char *fmt, ...); >> - >> static void test_odp_init_global(void) >> { >> int status; >> @@ -25,27 +21,8 @@ static void test_odp_init_global(void) >> CU_ASSERT(status == 0); >> } >> >> -static void test_odp_init_global_replace_log(void) >> -{ >> - int status; >> - struct odp_init_t init_data; >> - >> - init_data.log_fn = &odp_init_log; >> - >> - replacement_logging_used = 0; >> - >> - status = odp_init_global(&init_data, NULL); >> - CU_ASSERT_FATAL(status == 0); >> - >> - CU_ASSERT_TRUE(replacement_logging_used); >> > > For replacement_logging_used to be true implementation have to call log > function at least once in odp_init_global(). Maybe add a note just > above to make this assumption is obvious? > It would be even better to explicitly call some odp_*_print() function > before, to be sure that log function is called. > That will be a change to what is checked in, this was to just split things as they were. The init_global print a lot of stuff as it is, can altering it be a follow on? It currently prints things like odp_buffer_pool.c:91:odp_buffer_pool_init_global(): pool_entry_s size 320 odp_buffer_pool.c:92:odp_buffer_pool_init_global(): pool_entry_t size 320 odp_buffer_pool.c:93:odp_buffer_pool_init_global(): odp_buffer_hdr_t size 112 odp_buffer_pool.c:94:odp_buffer_pool_init_global():
On 02/03/2015 12:39 PM, Mike Holmes wrote: > > > On 3 February 2015 at 04:54, Taras Kondratiuk > <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>> wrote: > > On 02/03/2015 12:02 AM, Mike Holmes wrote: > > init_global can only be called once per executable, split the > init_global testing into multiple executables. > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > --- > v2 > fix copyright year > remove uneeded defines > > test/validation/Makefile.am | 4 +-- > test/validation/odp_init.c | 39 --------------------- > test/validation/odp_init_log.c | 78 > ++++++++++++++++++++++++++++++__++++++++++++ > 3 files changed, 80 insertions(+), 41 deletions(-) > create mode 100644 test/validation/odp_init_log.c > > diff --git a/test/validation/Makefile.am > b/test/validation/Makefile.am > index 2942b85..3ee59c8 100644 > --- a/test/validation/Makefile.am > +++ b/test/validation/Makefile.am > @@ -6,10 +6,10 @@ AM_LDFLAGS += -static > TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} > > if test_vald > -TESTS = odp_init odp_queue odp_crypto odp_shm odp_schedule > odp_pktio_run odp_buffer odp_system odp_timer odp_time > odp_synchronizers odp_classification > +TESTS = odp_init odp_init_log odp_queue odp_crypto odp_shm > odp_schedule odp_pktio_run odp_buffer odp_system odp_timer > odp_time odp_synchronizers odp_classification > endif > > -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm > odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time > odp_synchronizers odp_classification > +bin_PROGRAMS = odp_init odp_init_log odp_queue odp_crypto > odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer > odp_time odp_synchronizers odp_classification > odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto > odp_buffer_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/buffer > odp_classification_CFLAGS = $(AM_CFLAGS) > -I$(srcdir)/classification > diff --git a/test/validation/odp_init.c b/test/validation/odp_init.c > index c8e5a11..82f8849 100644 > --- a/test/validation/odp_init.c > +++ b/test/validation/odp_init.c > @@ -11,10 +11,6 @@ > #define DEFAULT_MSG_POOL_SIZE (4*1024*1024) > #define DEFAULT_MSG_SIZE (8) > > -int replacement_logging_used; > - > -static int odp_init_log(odp_log_level_e level , const char > *fmt, ...); > - > static void test_odp_init_global(void) > { > int status; > @@ -25,27 +21,8 @@ static void test_odp_init_global(void) > CU_ASSERT(status == 0); > } > > -static void test_odp_init_global_replace___log(void) > -{ > - int status; > - struct odp_init_t init_data; > - > - init_data.log_fn = &odp_init_log; > - > - replacement_logging_used = 0; > - > - status = odp_init_global(&init_data, NULL); > - CU_ASSERT_FATAL(status == 0); > - > - CU_ASSERT_TRUE(replacement___logging_used); > > > For replacement_logging_used to be true implementation have to call > log function at least once in odp_init_global(). Maybe add a note just > above to make this assumption is obvious? > It would be even better to explicitly call some odp_*_print() function > before, to be sure that log function is called. > > > That will be a change to what is checked in, this was to just split > things as they were. > The init_global print a lot of stuff as it is, can altering it be a > follow on? Yes that can be a follow up. Reviewed-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > > It currently prints things like > odp_buffer_pool.c:91:odp_buffer_pool_init_global(): pool_entry_s size > 320 > odp_buffer_pool.c:92:odp_buffer_pool_init_global(): pool_entry_t size > 320 > odp_buffer_pool.c:93:odp_buffer_pool_init_global(): odp_buffer_hdr_t > size 112 > odp_buffer_pool.c:94:odp_buffer_pool_init_global(): It is an assumption made by this test, but not all implementations print something on global init.
Patch applied with small correction. You deleted odp_classification from the list which is obvious error, no need to do that. Thanks, Maxim. On 02/03/2015 01:02 AM, Mike Holmes wrote: > init_global can only be called once per executable, split the > init_global testing into multiple executables. > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > v2 > fix copyright year > remove uneeded defines > > test/validation/Makefile.am | 4 +-- > test/validation/odp_init.c | 39 --------------------- > test/validation/odp_init_log.c | 78 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 80 insertions(+), 41 deletions(-) > create mode 100644 test/validation/odp_init_log.c > > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am > index 2942b85..3ee59c8 100644 > --- a/test/validation/Makefile.am > +++ b/test/validation/Makefile.am > @@ -6,10 +6,10 @@ AM_LDFLAGS += -static > TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} > > if test_vald > -TESTS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > +TESTS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > endif > > -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > +bin_PROGRAMS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification > odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto > odp_buffer_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/buffer > odp_classification_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/classification > diff --git a/test/validation/odp_init.c b/test/validation/odp_init.c > index c8e5a11..82f8849 100644 > --- a/test/validation/odp_init.c > +++ b/test/validation/odp_init.c > @@ -11,10 +11,6 @@ > #define DEFAULT_MSG_POOL_SIZE (4*1024*1024) > #define DEFAULT_MSG_SIZE (8) > > -int replacement_logging_used; > - > -static int odp_init_log(odp_log_level_e level , const char *fmt, ...); > - > static void test_odp_init_global(void) > { > int status; > @@ -25,27 +21,8 @@ static void test_odp_init_global(void) > CU_ASSERT(status == 0); > } > > -static void test_odp_init_global_replace_log(void) > -{ > - int status; > - struct odp_init_t init_data; > - > - init_data.log_fn = &odp_init_log; > - > - replacement_logging_used = 0; > - > - status = odp_init_global(&init_data, NULL); > - CU_ASSERT_FATAL(status == 0); > - > - CU_ASSERT_TRUE(replacement_logging_used); > - > - status = odp_term_global(); > - CU_ASSERT(status == 0); > -} > - > CU_TestInfo test_odp_init[] = { > {"test_odp_init_global", test_odp_init_global}, > - {"replace log", test_odp_init_global_replace_log}, > CU_TEST_INFO_NULL, > }; > > @@ -74,19 +51,3 @@ int main(void) > > return ret; > } > - > -int odp_init_log(odp_log_level_e level __attribute__((unused)), > - const char *fmt, ...) > -{ > - va_list args; > - int r; > - > - /* just set a flag to be sure the replacement fn was used */ > - replacement_logging_used = 1; > - > - va_start(args, fmt); > - r = vfprintf(stderr, fmt, args); > - va_end(args); > - > - return r; > -} > diff --git a/test/validation/odp_init_log.c b/test/validation/odp_init_log.c > new file mode 100644 > index 0000000..372d4f5 > --- /dev/null > +++ b/test/validation/odp_init_log.c > @@ -0,0 +1,78 @@ > +/* Copyright (c) 2015, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +#include <stdarg.h> > +#include <odp.h> > +#include <CUnit/Basic.h> > + > +int replacement_logging_used; > + > +static int odp_init_log(odp_log_level_e level , const char *fmt, ...); > + > +static void test_odp_init_global_replace_log(void) > +{ > + int status; > + struct odp_init_t init_data; > + > + init_data.log_fn = &odp_init_log; > + > + replacement_logging_used = 0; > + > + status = odp_init_global(&init_data, NULL); > + CU_ASSERT_FATAL(status == 0); > + > + CU_ASSERT_TRUE(replacement_logging_used); > + > + status = odp_term_global(); > + CU_ASSERT(status == 0); > +} > + > +CU_TestInfo test_odp_init[] = { > + {"replace log", test_odp_init_global_replace_log}, > + CU_TEST_INFO_NULL, > +}; > + > +CU_SuiteInfo odp_testsuites[] = { > + {"Init", NULL, NULL, NULL, NULL, test_odp_init}, > + CU_SUITE_INFO_NULL, > +}; > + > +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()); > + > + 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(); > + > + return ret; > +} > + > +int odp_init_log(odp_log_level_e level __attribute__((unused)), > + const char *fmt, ...) > +{ > + va_list args; > + int r; > + > + /* just set a flag to be sure the replacement fn was used */ > + replacement_logging_used = 1; > + > + va_start(args, fmt); > + r = vfprintf(stderr, fmt, args); > + va_end(args); > + > + return r; > +}
On 02/03/2015 03:12 PM, Maxim Uvarov wrote: > Patch applied with small correction. > > You deleted odp_classification from the list which is obvious error, no > need to do that. Hmm. Where is it removed?
On 02/03/2015 04:15 PM, Taras Kondratiuk wrote: > On 02/03/2015 03:12 PM, Maxim Uvarov wrote: >> Patch applied with small correction. >> >> You deleted odp_classification from the list which is obvious error, no >> need to do that. > > Hmm. Where is it removed? > Skip that message please. I think I wrongly interpret merge message. Maxim.
diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am index 2942b85..3ee59c8 100644 --- a/test/validation/Makefile.am +++ b/test/validation/Makefile.am @@ -6,10 +6,10 @@ AM_LDFLAGS += -static TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} if test_vald -TESTS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification +TESTS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule odp_pktio_run odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification endif -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification +bin_PROGRAMS = odp_init odp_init_log odp_queue odp_crypto odp_shm odp_schedule odp_pktio odp_buffer odp_system odp_timer odp_time odp_synchronizers odp_classification odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto odp_buffer_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/buffer odp_classification_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/classification diff --git a/test/validation/odp_init.c b/test/validation/odp_init.c index c8e5a11..82f8849 100644 --- a/test/validation/odp_init.c +++ b/test/validation/odp_init.c @@ -11,10 +11,6 @@ #define DEFAULT_MSG_POOL_SIZE (4*1024*1024) #define DEFAULT_MSG_SIZE (8) -int replacement_logging_used; - -static int odp_init_log(odp_log_level_e level , const char *fmt, ...); - static void test_odp_init_global(void) { int status; @@ -25,27 +21,8 @@ static void test_odp_init_global(void) CU_ASSERT(status == 0); } -static void test_odp_init_global_replace_log(void) -{ - int status; - struct odp_init_t init_data; - - init_data.log_fn = &odp_init_log; - - replacement_logging_used = 0; - - status = odp_init_global(&init_data, NULL); - CU_ASSERT_FATAL(status == 0); - - CU_ASSERT_TRUE(replacement_logging_used); - - status = odp_term_global(); - CU_ASSERT(status == 0); -} - CU_TestInfo test_odp_init[] = { {"test_odp_init_global", test_odp_init_global}, - {"replace log", test_odp_init_global_replace_log}, CU_TEST_INFO_NULL, }; @@ -74,19 +51,3 @@ int main(void) return ret; } - -int odp_init_log(odp_log_level_e level __attribute__((unused)), - const char *fmt, ...) -{ - va_list args; - int r; - - /* just set a flag to be sure the replacement fn was used */ - replacement_logging_used = 1; - - va_start(args, fmt); - r = vfprintf(stderr, fmt, args); - va_end(args); - - return r; -} diff --git a/test/validation/odp_init_log.c b/test/validation/odp_init_log.c new file mode 100644 index 0000000..372d4f5 --- /dev/null +++ b/test/validation/odp_init_log.c @@ -0,0 +1,78 @@ +/* Copyright (c) 2015, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include <stdarg.h> +#include <odp.h> +#include <CUnit/Basic.h> + +int replacement_logging_used; + +static int odp_init_log(odp_log_level_e level , const char *fmt, ...); + +static void test_odp_init_global_replace_log(void) +{ + int status; + struct odp_init_t init_data; + + init_data.log_fn = &odp_init_log; + + replacement_logging_used = 0; + + status = odp_init_global(&init_data, NULL); + CU_ASSERT_FATAL(status == 0); + + CU_ASSERT_TRUE(replacement_logging_used); + + status = odp_term_global(); + CU_ASSERT(status == 0); +} + +CU_TestInfo test_odp_init[] = { + {"replace log", test_odp_init_global_replace_log}, + CU_TEST_INFO_NULL, +}; + +CU_SuiteInfo odp_testsuites[] = { + {"Init", NULL, NULL, NULL, NULL, test_odp_init}, + CU_SUITE_INFO_NULL, +}; + +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()); + + 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(); + + return ret; +} + +int odp_init_log(odp_log_level_e level __attribute__((unused)), + const char *fmt, ...) +{ + va_list args; + int r; + + /* just set a flag to be sure the replacement fn was used */ + replacement_logging_used = 1; + + va_start(args, fmt); + r = vfprintf(stderr, fmt, args); + va_end(args); + + return r; +}
init_global can only be called once per executable, split the init_global testing into multiple executables. Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- v2 fix copyright year remove uneeded defines test/validation/Makefile.am | 4 +-- test/validation/odp_init.c | 39 --------------------- test/validation/odp_init_log.c | 78 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 41 deletions(-) create mode 100644 test/validation/odp_init_log.c