On 1 September 2014 03:44, Savolainen, Petri (NSN - FI/Espoo) < [email protected]> wrote:
> > > > -----Original Message----- > > From: [email protected] [mailto:lng-odp- > > [email protected]] On Behalf Of ext Mike Holmes > > Sent: Friday, August 29, 2014 5:41 PM > > To: [email protected] > > Subject: [lng-odp] [PATCH v2] test/unit: add odp_buffer_test > > > > Signed-off-by: Mike Holmes <[email protected]> > > Signed-off-by: Anders Roxell <[email protected]> > > --- > > > > v2: > > Add dependencies > > Add cross compile > > Add comments on test cases > > > > .gitignore | 1 + > > DEPENDENCIES | 19 +++- > > configure.ac | 20 ++++ > > test/Makefile.am | 2 +- > > test/api_test/Makefile.am | 1 + > > test/unit/Makefile.am | 11 +++ > > test/unit/odp_buffer_test.c | 222 > > ++++++++++++++++++++++++++++++++++++++++++++ > > 7 files changed, 274 insertions(+), 2 deletions(-) > > create mode 100644 test/unit/Makefile.am > > create mode 100644 test/unit/odp_buffer_test.c > > > > diff --git a/.gitignore b/.gitignore > > index 6a97f17..1000401 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -42,4 +42,5 @@ odp_pktio > > odp_timer_test > > odp_generator > > odp_l2fwd > > +odp_buffer > > doxygen-doc > > diff --git a/DEPENDENCIES b/DEPENDENCIES > > index 9597511..f54724d 100644 > > --- a/DEPENDENCIES > > +++ b/DEPENDENCIES > > @@ -75,4 +75,21 @@ Prerequisites for building the OpenDataPlane (ODP) API > > # Or build 64 bit version of ODP > > $ ./configure --host=aarch64-linux-gnu \ > > --with-openssl-path=/home/user/src/install-openssl-aarch64 > > - $ make > > \ No newline at end of file > > + $ make > > + > > +4. packages needed to build API tests > > + > > +4.1 Native compile > > + > > + # Debian/Ubuntu > > + $ apt-get install libcunit1-dev > > + > > + On CentOS/RedHat/Fedora systems: > > + $ su -c 'yum update CUnit' > > + > > +4.2 Cross compile > > + > > + $ git svn clone http://svn.code.sf.net/p/cunit/code/trunk cunit-code > > + $ cd cunit-code > > + $ ./bootstrap > > + $ ./configure --host=arm-linux-gnueabihf -- > > prefix=/home/user/src/install-cunit > > diff --git a/configure.ac b/configure.ac > > index 5574f82..d63f9ef 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -73,6 +73,25 @@ AC_ARG_ENABLE([netmap], > > AM_CONDITIONAL([ODP_NETMAP_ENABLED], [test x$netmap_support = xyes ]) > > > > > > > ########################################################################## > > +# Enable/disable Unit tests > > > +######################################################################### > > # > > +AC_ARG_ENABLE([cunit], > > + [ --enable-cunit Enable/Disable cunit. See cunit-path], > > + [if test x$enableval = xyes; then > > + unit_support=yes > > + fi]) > > + > > +AC_ARG_WITH([cunit-path], > > +AC_HELP_STRING([--with-cunit-path=DIR Path to Cunit libs and headers], > > + [(automaticly implies --enable-cunit).]), > > +[CUNIT_PATH=$withval unit_support=yes > > +],[ AC_MSG_WARN([Cunit not found - continuing without Cunit support]) > > +]) > > + > > +AC_SUBST(CUNIT_PATH) > > +AM_CONDITIONAL([ODP_UNIT_ENABLED], [test x$unit_support = xyes ]) > > + > > > +######################################################################### > > # > > # Enable/disable ODP_DEBUG_PRINT > > > > > ########################################################################## > > ODP_DEBUG=1 > > @@ -152,6 +171,7 @@ AC_CONFIG_FILES([Makefile > > example/timer/Makefile > > test/Makefile > > test/api_test/Makefile > > + test/unit/Makefile > > pkgconfig/libodp.pc]) > > > > AC_SEARCH_LIBS([timer_create],[rt posix4]) > > diff --git a/test/Makefile.am b/test/Makefile.am > > index 9bd7db1..dda7d5e 100644 > > --- a/test/Makefile.am > > +++ b/test/Makefile.am > > @@ -1 +1 @@ > > -SUBDIRS = api_test > > +SUBDIRS = api_test unit > > diff --git a/test/api_test/Makefile.am b/test/api_test/Makefile.am > > index 5104454..d76bc26 100644 > > --- a/test/api_test/Makefile.am > > +++ b/test/api_test/Makefile.am > > @@ -1,6 +1,7 @@ > > include $(top_srcdir)/test/Makefile.inc > > > > bin_PROGRAMS = odp_atomic odp_shm odp_ring odp_timer_ping > > + > > odp_atomic_LDFLAGS = $(AM_LDFLAGS) -static > > odp_shm_LDFLAGS = $(AM_LDFLAGS) -static > > odp_ring_LDFLAGS = $(AM_LDFLAGS) -static > > diff --git a/test/unit/Makefile.am b/test/unit/Makefile.am > > new file mode 100644 > > index 0000000..9554ed9 > > --- /dev/null > > +++ b/test/unit/Makefile.am > > @@ -0,0 +1,11 @@ > > +include $(top_srcdir)/test/Makefile.inc > > + > > +AM_CFLAGS += -I$(CUNIT_PATH)/include > > +AM_LDFLAGS += -L$(CUNIT_PATH)/lib > > + > > +if ODP_UNIT_ENABLED > > +bin_PROGRAMS = odp_buffer > > +odp_buffer_LDFLAGS = $(AM_LDFLAGS) -static -lcunit > > +endif > > + > > +dist_odp_buffer_SOURCES = odp_buffer_test.c > > diff --git a/test/unit/odp_buffer_test.c b/test/unit/odp_buffer_test.c > > new file mode 100644 > > index 0000000..5bc990e > > --- /dev/null > > +++ b/test/unit/odp_buffer_test.c > > @@ -0,0 +1,222 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +#include "odp.h" > > +#include "CUnit/Basic.h" > > + > > +#define DEFAULT_MSG_POOL_SIZE (4*1024*1024) > > +#define DEFAULT_MSG_SIZE (8) > > + > > +static void test_odp_buffer_addr(void) > > +{ > > + void *startaddress; > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + /* correct address returned */ > > + pool_base = odp_shm_reserve("msg_pool1", > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool1", > > + pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, > > + ODP_BUFFER_TYPE_RAW); > > + mybuffer = odp_buffer_alloc(msg_pool); > > + startaddress = odp_buffer_addr(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_TRUE(startaddress != NULL); > > No error checks for invalid handles ... Test passes also if invalid > handles are accepted and odp_buffer_addr() returns garbage. > Thank you for the comments. This test case is only for buffer_addr, my thinking was that if we add all the checks for all the supporting APIs every time each test case will get very large and obscure the tests purpose. Previous tests will have already proven those APIs work won't they ? We can add them for every set up call but I expect the global_init unit test to have already been run - what do people think ? > > > +} > > + > > +static void test_odp_buffer_size(void) > > +{ > > + size_t size; > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + /* correct size returned */ > > + pool_base = odp_shm_reserve("msg_pool2", > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool2", pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, > > + ODP_BUFFER_TYPE_RAW); > > Are you sure you want to do a shm and pool allocation per test. An > implementation may run out of shm segments or pools (especially when more > test cases are added). > I wanted to start as close to clean as possible for each API, so that if there is a failure it is with the specific API under test and not due to any artifacts from a previous case. But to support this ODP really needs a clean up API so that the same buffer can be re-made each time, then we don't consume them. > > + mybuffer = odp_buffer_alloc(msg_pool); > > + size = odp_buffer_size(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_TRUE(size == DEFAULT_MSG_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool2_1", pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + -1, > > + ODP_CACHE_LINE_SIZE, > > + ODP_BUFFER_TYPE_RAW); > > Same pool_base should not be used twice. So the failure may be also caused > by that, > The docs don't say this so we should add that information ? Basically you can only create one buffer pool per odp shared memory reservation. > > + CU_ASSERT_TRUE(msg_pool == ODP_BUFFER_POOL_INVALID); > > + mybuffer = odp_buffer_alloc(msg_pool); > > + size = odp_buffer_size(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_TRUE(size == ODP_BUFFER_INVALID); > > odp_buffer_size() does not return ODP_BUFFER_INVALID. > Typo thanks > > > +} > > + > > +static void test_odp_buffer_type(int type_in) > > +{ > > + int type; > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + char pool_name[32]; > > + /* valid types */ > > + sprintf(pool_name, "mag_pool_%d", type_in); > > + pool_base = odp_shm_reserve(pool_name, > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create(pool_name, pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, type_in); > > + mybuffer = odp_buffer_alloc(msg_pool); > > + type = odp_buffer_type(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_TRUE(type == type_in); > > +} > > + > > +static void test_odp_buffer_type_valid(void) > > +{ > > + test_odp_buffer_type(ODP_BUFFER_TYPE_ANY); > > + test_odp_buffer_type(ODP_BUFFER_TYPE_RAW); > > + test_odp_buffer_type(ODP_BUFFER_TYPE_PACKET); > > + test_odp_buffer_type(ODP_BUFFER_TYPE_TIMEOUT); > > +} > > + > > +static void test_odp_buffer_type_invalid(void) > > +{ > > + int type; > > + int type_in = ODP_BUFFER_TYPE_INVALID; > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + /* invalid type */ > > + pool_base = odp_shm_reserve("msg_pool4", > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool4", pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, type_in); > > + /* msg_pool should fail */ > > + CU_ASSERT_TRUE(msg_pool == ODP_BUFFER_POOL_INVALID); > > + /* mybuffer should fail */ > > + mybuffer = odp_buffer_alloc(msg_pool); > > + CU_ASSERT_TRUE(mybuffer == ODP_BUFFER_INVALID); > > + /* type should fail */ > > + type = odp_buffer_type(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_TRUE(type == type_in); > > +} > > + > > +static void test_odp_is_scatter(void) > > +{ > > + int is_scatter; > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + /* scatter is boolean */ > > + pool_base = odp_shm_reserve("msg_pool5", > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool5", pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, > > + ODP_BUFFER_TYPE_RAW); > > + mybuffer = odp_buffer_alloc(msg_pool); > > + is_scatter = odp_buffer_size(mybuffer); > > This is now odp_packet_is_segmented(), not odp_buffer_size()... > I was only targeting APIs in odp_buffer.h so I will delete this. > > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_TRUE((is_scatter < 0) || (is_scatter > 1)); > > +} > > + > > +static void test_odp_is_valid(void) > > +{ > > + int is_valid; > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + /* valid is boolean */ > > + pool_base = odp_shm_reserve("msg_pool6", > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool6", pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, > > + ODP_BUFFER_TYPE_RAW); > > + mybuffer = odp_buffer_alloc(msg_pool); > > + is_valid = odp_buffer_is_valid(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_ASSERT_FALSE((is_valid < 0) || (is_valid > 1)); > > It should be valid. So check only is_valid == 1 ? > The return type is an integer, do we have to check for other values being returned because that is an error in implementation that could work i.e. 2 could work in some tests of is valid but it is incorrect according to the API doc. I will also add a test to narrow the expected return in this case to a 1 and in addition add an invalid case. > > +} > > + > > +static void test_odp_buffer_print(void) > > +{ > > + odp_buffer_pool_t msg_pool; > > + odp_buffer_t mybuffer; > > + void *pool_base; > > + pool_base = odp_shm_reserve("msg_pool8", > > + DEFAULT_MSG_POOL_SIZE, > > + ODP_CACHE_LINE_SIZE); > > + msg_pool = odp_buffer_pool_create("msg_pool8", pool_base, > > + DEFAULT_MSG_POOL_SIZE, > > + DEFAULT_MSG_SIZE, > > + ODP_CACHE_LINE_SIZE, > > + ODP_BUFFER_TYPE_RAW); > > + mybuffer = odp_buffer_alloc(msg_pool); > > + odp_buffer_print(mybuffer); > > + odp_buffer_free(mybuffer); > > + CU_PASS("test_odp_buffer_print"); > > +} > > + > > +static int init_suite1(void) > > +{ > > + odp_init_global(); > > odp_init_local() must be also called (per thread). > Good point, the docs don't make it very clear what the sequence is. Should we add some verbiage to odp_init.h to actually lead a reader though the topic of correct initialisation ? > > > + return 0; > > +} > > + > > +static int clean_suite1(void) > > +{ > > + return 0; > > +} > > + > > +int main(void) > > +{ > > + CU_pSuite pSuite = NULL; > > + /* initialize the CUnit test registry */ > > + if (CUE_SUCCESS != CU_initialize_registry()) > > + return CU_get_error(); > > + /* add a suite to the registry */ > > + pSuite = CU_add_suite("odp_buffer_pool", init_suite1, > clean_suite1); > > + if (NULL == pSuite) { > > + CU_cleanup_registry(); > > + return CU_get_error(); > > + } > > + /* add the tests to the suite */ > > + if ((NULL == CU_ADD_TEST(pSuite, test_odp_buffer_size)) || > > + (NULL == CU_ADD_TEST(pSuite, test_odp_buffer_addr)) || > > + (NULL == CU_ADD_TEST(pSuite, test_odp_buffer_type_valid)) || > > + (NULL == CU_ADD_TEST(pSuite, test_odp_buffer_type_invalid)) || > > + (NULL == CU_ADD_TEST(pSuite, test_odp_is_valid)) || > > + (NULL == CU_ADD_TEST(pSuite, test_odp_is_scatter)) || > > + (NULL == CU_ADD_TEST(pSuite, test_odp_buffer_print))) { > > + CU_cleanup_registry(); > > + return CU_get_error(); > > + } > > + /* Run all tests using the CUnit Basic interface */ > > + CU_basic_set_mode(CU_BRM_VERBOSE); > > + CU_basic_run_tests(); > > This is single thread test. It's important to have a multi-threaded > version also (to catch race conditions, etc ...) > I agree. I was hoping to get a simple example in so that we have a place to start from, multithreaded etc are all needed. > > -Petri > > > + CU_cleanup_registry(); > > + return CU_get_error(); > > +} > > -- > > 1.9.1 > > > > > > _______________________________________________ > > lng-odp mailing list > > [email protected] > > http://lists.linaro.org/mailman/listinfo/lng-odp > -- *Mike Holmes* Linaro Technical Manager / Lead LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
