| Message ID | 1432810692-25231-3-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, May 28, 2015 at 12:58:08PM +0200, Christophe Milard wrote: > Renaming of things which may be, one day, exported in a lib. > This renaming is important, as it creates consistency between test > symbols, which is needed if things get eventually exported in the lib. > Also, tests are often created from other tests: Fixing the first exemples > will help geting future tests better. > > Also defining local test main. > > Things that are candidate to be exported in the lib in the future > have been named as follows: > -Tests, i.e. functions which are used in CUNIT testsuites are named: > <Module>_test_* > -Test arrays, i.e. arrays of CU_TestInfo, listing the test functions > belonging to a suite, are called: > <Module>_suite[_*] > where the possible suffix can be used if many suites are declared. > -CUNIT suite init and termination functions are called: > <Module>_suite[_*]_init() and <Module>_suite[_*]_term() > respectively. > -Suite arrays, i.e. arrays of CU_SuiteInfo used in executables are called: > <Module>_suites[_*] > where the possible suffix identifies the executable using it, if many. > -Main function(s), are called: > <Module>_main[_*] > where the possible suffix identifies the executable using it > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > --- > test/Makefile.inc | 7 +++- > test/validation/Makefile.am | 8 ++--- > test/validation/odp_pktio.c | 81 +++++++++++++++++++++++++-------------------- > 3 files changed, 55 insertions(+), 41 deletions(-) > > diff --git a/test/Makefile.inc b/test/Makefile.inc > index e20a848..c579c46 100644 > --- a/test/Makefile.inc > +++ b/test/Makefile.inc > @@ -1,7 +1,12 @@ > include $(top_srcdir)/Makefile.inc > include $(top_srcdir)/platform/@with_platform@/Makefile.inc > LIB = $(top_builddir)/lib > -LDADD = $(LIB)/libodp.la > + > +#in the following line, the libs using the symbols should come before > +#the libs using them! The includer is given a chance to add things before libodp > +#by setting PRE_LDDAD before the inclusion. > +LDADD = $(PRE_LDDAD) $(LIB)/libodp.la > + > INCFLAGS = -I$(srcdir) \ > -I$(top_srcdir)/test \ > -I$(top_srcdir)/platform/@with_platform@/include \ > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am > index 965f603..8299e01 100644 > --- a/test/validation/Makefile.am > +++ b/test/validation/Makefile.am > @@ -1,13 +1,9 @@ > +PRE_LDDAD = $(top_builddir)/test/validation/common/libcunit_common_as_main.a > include $(top_srcdir)/test/Makefile.inc Shouldn't this be named PRE_LDADD? > > AM_CFLAGS += -I$(srcdir)/common > AM_LDFLAGS += -static > > -#warning: in the following line, the libs using the symbols should come before > -#the libs using them! > -LDADD = $(top_builddir)/test/validation/common/libcunit_common_as_main.a \ > - $(LIB)/libodp.la > - > TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} TEST_DIR=${builddir} > > EXECUTABLES = odp_buffer \ > @@ -62,6 +58,8 @@ dist_odp_shared_memory_SOURCES = odp_shared_memory.c > dist_odp_synchronizers_SOURCES = odp_synchronizers.c > dist_odp_time_SOURCES = odp_time.c > dist_odp_timer_SOURCES = odp_timer.c > +odp_pktio_LDADD = $(top_builddir)/test/validation/common/libcunit_common.a \ > + $(LIB)/libodp.la > dist_odp_pktio_SOURCES = odp_pktio.c > dist_odp_packet_SOURCES = odp_packet.c > dist_odp_pool_SOURCES = odp_pool.c > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > index d4bf7bf..9806376 100644 > --- a/test/validation/odp_pktio.c > +++ b/test/validation/odp_pktio.c > @@ -420,7 +420,7 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, > CU_ASSERT(i == num_pkts); > } > > -static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) > +static void test_txrx(odp_queue_type_t q_type, int num_pkts) > { > int ret, i, if_b; > pktio_info_t pktios[MAX_NUM_IFACES]; > @@ -456,34 +456,34 @@ static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) > } > } > > -static void test_odp_pktio_poll_queue(void) > +static void pktio_test_poll_queue(void) > { > - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 1); > + test_txrx(ODP_QUEUE_TYPE_POLL, 1); > } > > -static void test_odp_pktio_poll_multi(void) > +static void pktio_test_poll_multi(void) > { > - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 4); > + test_txrx(ODP_QUEUE_TYPE_POLL, 4); > } > > -static void test_odp_pktio_sched_queue(void) > +static void pktio_test_sched_queue(void) > { > - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 1); > + test_txrx(ODP_QUEUE_TYPE_SCHED, 1); > } > > -static void test_odp_pktio_sched_multi(void) > +static void pktio_test_sched_multi(void) > { > - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 4); > + test_txrx(ODP_QUEUE_TYPE_SCHED, 4); > } > > -static void test_odp_pktio_jumbo(void) > +static void pktio_test_jumbo(void) > { > packet_len = PKT_LEN_JUMBO; > - test_odp_pktio_sched_multi(); > + pktio_test_sched_multi(); > packet_len = PKT_LEN_NORMAL; > } > > -static void test_odp_pktio_mtu(void) > +static void pktio_test_mtu(void) > { > int ret; > int mtu; > @@ -500,7 +500,7 @@ static void test_odp_pktio_mtu(void) > return; > } > > -static void test_odp_pktio_promisc(void) > +static void pktio_test_promisc(void) > { > int ret; > odp_pktio_t pktio = create_pktio(iface_name[0]); > @@ -525,7 +525,7 @@ static void test_odp_pktio_promisc(void) > return; > } > > -static void test_odp_pktio_mac(void) > +static void pktio_test_mac(void) > { > unsigned char mac_addr[ODPH_ETHADDR_LEN]; > int mac_len; > @@ -551,7 +551,7 @@ static void test_odp_pktio_mac(void) > return; > } > > -static void test_odp_pktio_inq_remdef(void) > +static void pktio_test_inq_remdef(void) > { > odp_pktio_t pktio = create_pktio(iface_name[0]); > odp_queue_t inq; > @@ -575,7 +575,7 @@ static void test_odp_pktio_inq_remdef(void) > CU_ASSERT(odp_pktio_close(pktio) == 0); > } > > -static void test_odp_pktio_open(void) > +static void pktio_test_open(void) > { > odp_pktio_t pktio; > int i; > @@ -591,7 +591,7 @@ static void test_odp_pktio_open(void) > CU_ASSERT(pktio == ODP_PKTIO_INVALID); > } > > -static void test_odp_pktio_lookup(void) > +static void pktio_test_lookup(void) > { > odp_pktio_t pktio, pktio_inval; > > @@ -609,7 +609,7 @@ static void test_odp_pktio_lookup(void) > CU_ASSERT(odp_pktio_lookup(iface_name[0]) == ODP_PKTIO_INVALID); > } > > -static void test_odp_pktio_inq(void) > +static void pktio_test_inq(void) > { > odp_pktio_t pktio; > > @@ -621,7 +621,7 @@ static void test_odp_pktio_inq(void) > CU_ASSERT(odp_pktio_close(pktio) == 0); > } > > -static int init_pktio_suite(void) > +static int pktio_suite_init(void) > { > odp_atomic_init_u32(&ip_seq, 0); > iface_name[0] = getenv("ODP_PKTIO_IF0"); > @@ -647,7 +647,7 @@ static int init_pktio_suite(void) > return 0; > } > > -static int term_pktio_suite(void) > +static int pktio_suite_term(void) > { > char pool_name[ODP_POOL_NAME_LEN]; > odp_pool_t pool; > @@ -676,24 +676,35 @@ static int term_pktio_suite(void) > return ret; > } Are all of these renames really necessary given that these symbols are all still static? We use static so that we don't have to worry about global namespace pollution and naming decisions are local. This waypoint of naming things to account for the fact they may one day be exported but leaving them static isn't natural and will be hard to enforce as nothing actually breaks if you don't do it and we have no tool that checks for it. I think it would be better to defer the renaming until it's actually needed. > > -CU_TestInfo pktio_tests[] = { > - {"pktio open", test_odp_pktio_open}, > - {"pktio lookup", test_odp_pktio_lookup}, > - {"pktio inq", test_odp_pktio_inq}, > - {"pktio poll queues", test_odp_pktio_poll_queue}, > - {"pktio poll multi", test_odp_pktio_poll_multi}, > - {"pktio sched queues", test_odp_pktio_sched_queue}, > - {"pktio sched multi", test_odp_pktio_sched_multi}, > - {"pktio jumbo frames", test_odp_pktio_jumbo}, > - {"pktio mtu", test_odp_pktio_mtu}, > - {"pktio promisc mode", test_odp_pktio_promisc}, > - {"pktio mac", test_odp_pktio_mac}, > - {"pktio inq_remdef", test_odp_pktio_inq_remdef}, > +CU_TestInfo pktio_suite[] = { > + {"pktio open", pktio_test_open}, > + {"pktio lookup", pktio_test_lookup}, > + {"pktio inq", pktio_test_inq}, > + {"pktio poll queues", pktio_test_poll_queue}, > + {"pktio poll multi", pktio_test_poll_multi}, > + {"pktio sched queues", pktio_test_sched_queue}, > + {"pktio sched multi", pktio_test_sched_multi}, > + {"pktio jumbo frames", pktio_test_jumbo}, > + {"pktio mtu", pktio_test_mtu}, > + {"pktio promisc mode", pktio_test_promisc}, > + {"pktio mac", pktio_test_mac}, > + {"pktio inq_remdef", pktio_test_inq_remdef}, > CU_TEST_INFO_NULL > }; > > -CU_SuiteInfo odp_testsuites[] = { > +static CU_SuiteInfo pktio_suites[] = { > {"Packet I/O", > - init_pktio_suite, term_pktio_suite, NULL, NULL, pktio_tests}, > + pktio_suite_init, pktio_suite_term, NULL, NULL, pktio_suite}, > CU_SUITE_INFO_NULL > }; Any reason why pktio_suites is static and pktio_suite isn't? > + > +static int pktio_main(void) > +{ > + return odp_cunit_run(pktio_suites); > +} > + > +/* the following main function will be separated when lib is created */ > +int main(void) > +{ > + return pktio_main(); > +} > -- > 1.9.1 >
On 2 June 2015 at 11:22, Stuart Haslam <stuart.haslam@linaro.org> wrote: > On Thu, May 28, 2015 at 12:58:08PM +0200, Christophe Milard wrote: > > Renaming of things which may be, one day, exported in a lib. > > This renaming is important, as it creates consistency between test > > symbols, which is needed if things get eventually exported in the lib. > > Also, tests are often created from other tests: Fixing the first exemples > > will help geting future tests better. > > > > Also defining local test main. > > > > Things that are candidate to be exported in the lib in the future > > have been named as follows: > > -Tests, i.e. functions which are used in CUNIT testsuites are named: > > <Module>_test_* > > -Test arrays, i.e. arrays of CU_TestInfo, listing the test functions > > belonging to a suite, are called: > > <Module>_suite[_*] > > where the possible suffix can be used if many suites are declared. > > -CUNIT suite init and termination functions are called: > > <Module>_suite[_*]_init() and <Module>_suite[_*]_term() > > respectively. > > -Suite arrays, i.e. arrays of CU_SuiteInfo used in executables are > called: > > <Module>_suites[_*] > > where the possible suffix identifies the executable using it, if many. > > -Main function(s), are called: > > <Module>_main[_*] > > where the possible suffix identifies the executable using it > > > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > > --- > > test/Makefile.inc | 7 +++- > > test/validation/Makefile.am | 8 ++--- > > test/validation/odp_pktio.c | 81 > +++++++++++++++++++++++++-------------------- > > 3 files changed, 55 insertions(+), 41 deletions(-) > > > > diff --git a/test/Makefile.inc b/test/Makefile.inc > > index e20a848..c579c46 100644 > > --- a/test/Makefile.inc > > +++ b/test/Makefile.inc > > @@ -1,7 +1,12 @@ > > include $(top_srcdir)/Makefile.inc > > include $(top_srcdir)/platform/@with_platform@/Makefile.inc > > LIB = $(top_builddir)/lib > > -LDADD = $(LIB)/libodp.la > > + > > +#in the following line, the libs using the symbols should come before > > +#the libs using them! The includer is given a chance to add things > before libodp > > +#by setting PRE_LDDAD before the inclusion. > > +LDADD = $(PRE_LDDAD) $(LIB)/libodp.la > > + > > INCFLAGS = -I$(srcdir) \ > > -I$(top_srcdir)/test \ > > -I$(top_srcdir)/platform/@with_platform@/include \ > > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am > > index 965f603..8299e01 100644 > > --- a/test/validation/Makefile.am > > +++ b/test/validation/Makefile.am > > @@ -1,13 +1,9 @@ > > +PRE_LDDAD = > $(top_builddir)/test/validation/common/libcunit_common_as_main.a > > include $(top_srcdir)/test/Makefile.inc > > Shouldn't this be named PRE_LDADD? > Yes, of course. will be fix in V2 > > > > > AM_CFLAGS += -I$(srcdir)/common > > AM_LDFLAGS += -static > > > > -#warning: in the following line, the libs using the symbols should come > before > > -#the libs using them! > > -LDADD = > $(top_builddir)/test/validation/common/libcunit_common_as_main.a \ > > - $(LIB)/libodp.la > > - > > TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} TEST_DIR=${builddir} > > > > EXECUTABLES = odp_buffer \ > > @@ -62,6 +58,8 @@ dist_odp_shared_memory_SOURCES = > odp_shared_memory.c > > dist_odp_synchronizers_SOURCES = odp_synchronizers.c > > dist_odp_time_SOURCES = odp_time.c > > dist_odp_timer_SOURCES = odp_timer.c > > +odp_pktio_LDADD = > $(top_builddir)/test/validation/common/libcunit_common.a \ > > + $(LIB)/libodp.la > > dist_odp_pktio_SOURCES = odp_pktio.c > > dist_odp_packet_SOURCES = odp_packet.c > > dist_odp_pool_SOURCES = odp_pool.c > > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > > index d4bf7bf..9806376 100644 > > --- a/test/validation/odp_pktio.c > > +++ b/test/validation/odp_pktio.c > > @@ -420,7 +420,7 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, > pktio_info_t *pktio_b, > > CU_ASSERT(i == num_pkts); > > } > > > > -static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) > > +static void test_txrx(odp_queue_type_t q_type, int num_pkts) > > { > > int ret, i, if_b; > > pktio_info_t pktios[MAX_NUM_IFACES]; > > @@ -456,34 +456,34 @@ static void pktio_test_txrx(odp_queue_type_t > q_type, int num_pkts) > > } > > } > > > > -static void test_odp_pktio_poll_queue(void) > > +static void pktio_test_poll_queue(void) > > { > > - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 1); > > + test_txrx(ODP_QUEUE_TYPE_POLL, 1); > > } > > > > -static void test_odp_pktio_poll_multi(void) > > +static void pktio_test_poll_multi(void) > > { > > - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 4); > > + test_txrx(ODP_QUEUE_TYPE_POLL, 4); > > } > > > > -static void test_odp_pktio_sched_queue(void) > > +static void pktio_test_sched_queue(void) > > { > > - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 1); > > + test_txrx(ODP_QUEUE_TYPE_SCHED, 1); > > } > > > > -static void test_odp_pktio_sched_multi(void) > > +static void pktio_test_sched_multi(void) > > { > > - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 4); > > + test_txrx(ODP_QUEUE_TYPE_SCHED, 4); > > } > > > > -static void test_odp_pktio_jumbo(void) > > +static void pktio_test_jumbo(void) > > { > > packet_len = PKT_LEN_JUMBO; > > - test_odp_pktio_sched_multi(); > > + pktio_test_sched_multi(); > > packet_len = PKT_LEN_NORMAL; > > } > > > > -static void test_odp_pktio_mtu(void) > > +static void pktio_test_mtu(void) > > { > > int ret; > > int mtu; > > @@ -500,7 +500,7 @@ static void test_odp_pktio_mtu(void) > > return; > > } > > > > -static void test_odp_pktio_promisc(void) > > +static void pktio_test_promisc(void) > > { > > int ret; > > odp_pktio_t pktio = create_pktio(iface_name[0]); > > @@ -525,7 +525,7 @@ static void test_odp_pktio_promisc(void) > > return; > > } > > > > -static void test_odp_pktio_mac(void) > > +static void pktio_test_mac(void) > > { > > unsigned char mac_addr[ODPH_ETHADDR_LEN]; > > int mac_len; > > @@ -551,7 +551,7 @@ static void test_odp_pktio_mac(void) > > return; > > } > > > > -static void test_odp_pktio_inq_remdef(void) > > +static void pktio_test_inq_remdef(void) > > { > > odp_pktio_t pktio = create_pktio(iface_name[0]); > > odp_queue_t inq; > > @@ -575,7 +575,7 @@ static void test_odp_pktio_inq_remdef(void) > > CU_ASSERT(odp_pktio_close(pktio) == 0); > > } > > > > -static void test_odp_pktio_open(void) > > +static void pktio_test_open(void) > > { > > odp_pktio_t pktio; > > int i; > > @@ -591,7 +591,7 @@ static void test_odp_pktio_open(void) > > CU_ASSERT(pktio == ODP_PKTIO_INVALID); > > } > > > > -static void test_odp_pktio_lookup(void) > > +static void pktio_test_lookup(void) > > { > > odp_pktio_t pktio, pktio_inval; > > > > @@ -609,7 +609,7 @@ static void test_odp_pktio_lookup(void) > > CU_ASSERT(odp_pktio_lookup(iface_name[0]) == ODP_PKTIO_INVALID); > > } > > > > -static void test_odp_pktio_inq(void) > > +static void pktio_test_inq(void) > > { > > odp_pktio_t pktio; > > > > @@ -621,7 +621,7 @@ static void test_odp_pktio_inq(void) > > CU_ASSERT(odp_pktio_close(pktio) == 0); > > } > > > > -static int init_pktio_suite(void) > > +static int pktio_suite_init(void) > > { > > odp_atomic_init_u32(&ip_seq, 0); > > iface_name[0] = getenv("ODP_PKTIO_IF0"); > > @@ -647,7 +647,7 @@ static int init_pktio_suite(void) > > return 0; > > } > > > > -static int term_pktio_suite(void) > > +static int pktio_suite_term(void) > > { > > char pool_name[ODP_POOL_NAME_LEN]; > > odp_pool_t pool; > > @@ -676,24 +676,35 @@ static int term_pktio_suite(void) > > return ret; > > } > > Are all of these renames really necessary given that these symbols are > all still static? We use static so that we don't have to worry about > global namespace pollution and naming decisions are local. This waypoint > of naming things to account for the fact they may one day be exported > but leaving them static isn't natural and will be hard to enforce as > nothing actually breaks if you don't do it and we have no tool that > checks for it. I think it would be better to defer the renaming until > it's actually needed. > > I am not sure I agree with this: As the number of tests increases, the number of things to (possibly) be fixed in the future will increase. Trying to set a naming convention now will just make things more consistent, and possibly reduce the work in the future. At worse, it is just a bit better :-). > > > > -CU_TestInfo pktio_tests[] = { > > - {"pktio open", test_odp_pktio_open}, > > - {"pktio lookup", test_odp_pktio_lookup}, > > - {"pktio inq", test_odp_pktio_inq}, > > - {"pktio poll queues", test_odp_pktio_poll_queue}, > > - {"pktio poll multi", test_odp_pktio_poll_multi}, > > - {"pktio sched queues", test_odp_pktio_sched_queue}, > > - {"pktio sched multi", test_odp_pktio_sched_multi}, > > - {"pktio jumbo frames", test_odp_pktio_jumbo}, > > - {"pktio mtu", test_odp_pktio_mtu}, > > - {"pktio promisc mode", test_odp_pktio_promisc}, > > - {"pktio mac", test_odp_pktio_mac}, > > - {"pktio inq_remdef", test_odp_pktio_inq_remdef}, > > +CU_TestInfo pktio_suite[] = { > > + {"pktio open", pktio_test_open}, > > + {"pktio lookup", pktio_test_lookup}, > > + {"pktio inq", pktio_test_inq}, > > + {"pktio poll queues", pktio_test_poll_queue}, > > + {"pktio poll multi", pktio_test_poll_multi}, > > + {"pktio sched queues", pktio_test_sched_queue}, > > + {"pktio sched multi", pktio_test_sched_multi}, > > + {"pktio jumbo frames", pktio_test_jumbo}, > > + {"pktio mtu", pktio_test_mtu}, > > + {"pktio promisc mode", pktio_test_promisc}, > > + {"pktio mac", pktio_test_mac}, > > + {"pktio inq_remdef", pktio_test_inq_remdef}, > > CU_TEST_INFO_NULL > > }; > > > > -CU_SuiteInfo odp_testsuites[] = { > > +static CU_SuiteInfo pktio_suites[] = { > > {"Packet I/O", > > - init_pktio_suite, term_pktio_suite, NULL, NULL, > pktio_tests}, > > + pktio_suite_init, pktio_suite_term, NULL, NULL, > pktio_suite}, > > CU_SUITE_INFO_NULL > > }; > > Any reason why pktio_suites is static and pktio_suite isn't? > no. Thanks. will be fixed in V2 Thanks, Christophe. > > > + > > +static int pktio_main(void) > > +{ > > + return odp_cunit_run(pktio_suites); > > +} > > + > > +/* the following main function will be separated when lib is created */ > > +int main(void) > > +{ > > + return pktio_main(); > > +} > > -- > > 1.9.1 > > > > -- > Stuart. >
On Tue, Jun 02, 2015 at 04:51:09PM +0200, Christophe Milard wrote: > On 2 June 2015 at 11:22, Stuart Haslam <stuart.haslam@linaro.org> wrote: > > > On Thu, May 28, 2015 at 12:58:08PM +0200, Christophe Milard wrote: > > > Renaming of things which may be, one day, exported in a lib. > > > This renaming is important, as it creates consistency between test > > > symbols, which is needed if things get eventually exported in the lib. > > > Also, tests are often created from other tests: Fixing the first exemples > > > will help geting future tests better. > > > > > > Also defining local test main. > > > > > > Things that are candidate to be exported in the lib in the future > > > have been named as follows: > > > -Tests, i.e. functions which are used in CUNIT testsuites are named: > > > <Module>_test_* > > > -Test arrays, i.e. arrays of CU_TestInfo, listing the test functions > > > belonging to a suite, are called: > > > <Module>_suite[_*] > > > where the possible suffix can be used if many suites are declared. > > > -CUNIT suite init and termination functions are called: > > > <Module>_suite[_*]_init() and <Module>_suite[_*]_term() > > > respectively. > > > -Suite arrays, i.e. arrays of CU_SuiteInfo used in executables are > > called: > > > <Module>_suites[_*] > > > where the possible suffix identifies the executable using it, if many. > > > -Main function(s), are called: > > > <Module>_main[_*] > > > where the possible suffix identifies the executable using it > > > > > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > > > --- > > > test/Makefile.inc | 7 +++- > > > test/validation/Makefile.am | 8 ++--- > > > test/validation/odp_pktio.c | 81 > > +++++++++++++++++++++++++-------------------- > > > 3 files changed, 55 insertions(+), 41 deletions(-) > > > > > > diff --git a/test/Makefile.inc b/test/Makefile.inc > > > index e20a848..c579c46 100644 > > > --- a/test/Makefile.inc > > > +++ b/test/Makefile.inc > > > @@ -1,7 +1,12 @@ > > > include $(top_srcdir)/Makefile.inc > > > include $(top_srcdir)/platform/@with_platform@/Makefile.inc > > > LIB = $(top_builddir)/lib > > > -LDADD = $(LIB)/libodp.la > > > + > > > +#in the following line, the libs using the symbols should come before > > > +#the libs using them! The includer is given a chance to add things > > before libodp > > > +#by setting PRE_LDDAD before the inclusion. > > > +LDADD = $(PRE_LDDAD) $(LIB)/libodp.la > > > + > > > INCFLAGS = -I$(srcdir) \ > > > -I$(top_srcdir)/test \ > > > -I$(top_srcdir)/platform/@with_platform@/include \ > > > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am > > > index 965f603..8299e01 100644 > > > --- a/test/validation/Makefile.am > > > +++ b/test/validation/Makefile.am > > > @@ -1,13 +1,9 @@ > > > +PRE_LDDAD = > > $(top_builddir)/test/validation/common/libcunit_common_as_main.a > > > include $(top_srcdir)/test/Makefile.inc > > > > Shouldn't this be named PRE_LDADD? > > > > Yes, of course. will be fix in V2 > > > > > > > > > > AM_CFLAGS += -I$(srcdir)/common > > > AM_LDFLAGS += -static > > > > > > -#warning: in the following line, the libs using the symbols should come > > before > > > -#the libs using them! > > > -LDADD = > > $(top_builddir)/test/validation/common/libcunit_common_as_main.a \ > > > - $(LIB)/libodp.la > > > - > > > TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} TEST_DIR=${builddir} > > > > > > EXECUTABLES = odp_buffer \ > > > @@ -62,6 +58,8 @@ dist_odp_shared_memory_SOURCES = > > odp_shared_memory.c > > > dist_odp_synchronizers_SOURCES = odp_synchronizers.c > > > dist_odp_time_SOURCES = odp_time.c > > > dist_odp_timer_SOURCES = odp_timer.c > > > +odp_pktio_LDADD = > > $(top_builddir)/test/validation/common/libcunit_common.a \ > > > + $(LIB)/libodp.la > > > dist_odp_pktio_SOURCES = odp_pktio.c > > > dist_odp_packet_SOURCES = odp_packet.c > > > dist_odp_pool_SOURCES = odp_pool.c > > > diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c > > > index d4bf7bf..9806376 100644 > > > --- a/test/validation/odp_pktio.c > > > +++ b/test/validation/odp_pktio.c > > > @@ -420,7 +420,7 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, > > pktio_info_t *pktio_b, > > > CU_ASSERT(i == num_pkts); > > > } > > > > > > -static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) > > > +static void test_txrx(odp_queue_type_t q_type, int num_pkts) > > > { > > > int ret, i, if_b; > > > pktio_info_t pktios[MAX_NUM_IFACES]; > > > @@ -456,34 +456,34 @@ static void pktio_test_txrx(odp_queue_type_t > > q_type, int num_pkts) > > > } > > > } > > > > > > -static void test_odp_pktio_poll_queue(void) > > > +static void pktio_test_poll_queue(void) > > > { > > > - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 1); > > > + test_txrx(ODP_QUEUE_TYPE_POLL, 1); > > > } > > > > > > -static void test_odp_pktio_poll_multi(void) > > > +static void pktio_test_poll_multi(void) > > > { > > > - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 4); > > > + test_txrx(ODP_QUEUE_TYPE_POLL, 4); > > > } > > > > > > -static void test_odp_pktio_sched_queue(void) > > > +static void pktio_test_sched_queue(void) > > > { > > > - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 1); > > > + test_txrx(ODP_QUEUE_TYPE_SCHED, 1); > > > } > > > > > > -static void test_odp_pktio_sched_multi(void) > > > +static void pktio_test_sched_multi(void) > > > { > > > - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 4); > > > + test_txrx(ODP_QUEUE_TYPE_SCHED, 4); > > > } > > > > > > -static void test_odp_pktio_jumbo(void) > > > +static void pktio_test_jumbo(void) > > > { > > > packet_len = PKT_LEN_JUMBO; > > > - test_odp_pktio_sched_multi(); > > > + pktio_test_sched_multi(); > > > packet_len = PKT_LEN_NORMAL; > > > } > > > > > > -static void test_odp_pktio_mtu(void) > > > +static void pktio_test_mtu(void) > > > { > > > int ret; > > > int mtu; > > > @@ -500,7 +500,7 @@ static void test_odp_pktio_mtu(void) > > > return; > > > } > > > > > > -static void test_odp_pktio_promisc(void) > > > +static void pktio_test_promisc(void) > > > { > > > int ret; > > > odp_pktio_t pktio = create_pktio(iface_name[0]); > > > @@ -525,7 +525,7 @@ static void test_odp_pktio_promisc(void) > > > return; > > > } > > > > > > -static void test_odp_pktio_mac(void) > > > +static void pktio_test_mac(void) > > > { > > > unsigned char mac_addr[ODPH_ETHADDR_LEN]; > > > int mac_len; > > > @@ -551,7 +551,7 @@ static void test_odp_pktio_mac(void) > > > return; > > > } > > > > > > -static void test_odp_pktio_inq_remdef(void) > > > +static void pktio_test_inq_remdef(void) > > > { > > > odp_pktio_t pktio = create_pktio(iface_name[0]); > > > odp_queue_t inq; > > > @@ -575,7 +575,7 @@ static void test_odp_pktio_inq_remdef(void) > > > CU_ASSERT(odp_pktio_close(pktio) == 0); > > > } > > > > > > -static void test_odp_pktio_open(void) > > > +static void pktio_test_open(void) > > > { > > > odp_pktio_t pktio; > > > int i; > > > @@ -591,7 +591,7 @@ static void test_odp_pktio_open(void) > > > CU_ASSERT(pktio == ODP_PKTIO_INVALID); > > > } > > > > > > -static void test_odp_pktio_lookup(void) > > > +static void pktio_test_lookup(void) > > > { > > > odp_pktio_t pktio, pktio_inval; > > > > > > @@ -609,7 +609,7 @@ static void test_odp_pktio_lookup(void) > > > CU_ASSERT(odp_pktio_lookup(iface_name[0]) == ODP_PKTIO_INVALID); > > > } > > > > > > -static void test_odp_pktio_inq(void) > > > +static void pktio_test_inq(void) > > > { > > > odp_pktio_t pktio; > > > > > > @@ -621,7 +621,7 @@ static void test_odp_pktio_inq(void) > > > CU_ASSERT(odp_pktio_close(pktio) == 0); > > > } > > > > > > -static int init_pktio_suite(void) > > > +static int pktio_suite_init(void) > > > { > > > odp_atomic_init_u32(&ip_seq, 0); > > > iface_name[0] = getenv("ODP_PKTIO_IF0"); > > > @@ -647,7 +647,7 @@ static int init_pktio_suite(void) > > > return 0; > > > } > > > > > > -static int term_pktio_suite(void) > > > +static int pktio_suite_term(void) > > > { > > > char pool_name[ODP_POOL_NAME_LEN]; > > > odp_pool_t pool; > > > @@ -676,24 +676,35 @@ static int term_pktio_suite(void) > > > return ret; > > > } > > > > Are all of these renames really necessary given that these symbols are > > all still static? We use static so that we don't have to worry about > > global namespace pollution and naming decisions are local. This waypoint > > of naming things to account for the fact they may one day be exported > > but leaving them static isn't natural and will be hard to enforce as > > nothing actually breaks if you don't do it and we have no tool that > > checks for it. I think it would be better to defer the renaming until > > it's actually needed. > > > > > I am not sure I agree with this I thought you might say that :) > As the number of tests increases, the number of things > to (possibly) be fixed in the future will increase. Trying to set a naming > convention > now will just make things more consistent, and possibly reduce the work in > the future. It's the "possibly" bit that is the issue. We're taking on extra work now (not just the initial renaming but the ongoing convention and associated review etc) to avoid future work that may not actually be needed. How likely are these to be exported?.. I actually thought we'd concluded that it wasn't a good idea to export individual test cases an instead just export the entire suite. > At worse, it is just a bit better :-). Actually worst case is we do a bunch of work for no benefit.
diff --git a/test/Makefile.inc b/test/Makefile.inc index e20a848..c579c46 100644 --- a/test/Makefile.inc +++ b/test/Makefile.inc @@ -1,7 +1,12 @@ include $(top_srcdir)/Makefile.inc include $(top_srcdir)/platform/@with_platform@/Makefile.inc LIB = $(top_builddir)/lib -LDADD = $(LIB)/libodp.la + +#in the following line, the libs using the symbols should come before +#the libs using them! The includer is given a chance to add things before libodp +#by setting PRE_LDDAD before the inclusion. +LDADD = $(PRE_LDDAD) $(LIB)/libodp.la + INCFLAGS = -I$(srcdir) \ -I$(top_srcdir)/test \ -I$(top_srcdir)/platform/@with_platform@/include \ diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am index 965f603..8299e01 100644 --- a/test/validation/Makefile.am +++ b/test/validation/Makefile.am @@ -1,13 +1,9 @@ +PRE_LDDAD = $(top_builddir)/test/validation/common/libcunit_common_as_main.a include $(top_srcdir)/test/Makefile.inc AM_CFLAGS += -I$(srcdir)/common AM_LDFLAGS += -static -#warning: in the following line, the libs using the symbols should come before -#the libs using them! -LDADD = $(top_builddir)/test/validation/common/libcunit_common_as_main.a \ - $(LIB)/libodp.la - TESTS_ENVIRONMENT = ODP_PLATFORM=${with_platform} TEST_DIR=${builddir} EXECUTABLES = odp_buffer \ @@ -62,6 +58,8 @@ dist_odp_shared_memory_SOURCES = odp_shared_memory.c dist_odp_synchronizers_SOURCES = odp_synchronizers.c dist_odp_time_SOURCES = odp_time.c dist_odp_timer_SOURCES = odp_timer.c +odp_pktio_LDADD = $(top_builddir)/test/validation/common/libcunit_common.a \ + $(LIB)/libodp.la dist_odp_pktio_SOURCES = odp_pktio.c dist_odp_packet_SOURCES = odp_packet.c dist_odp_pool_SOURCES = odp_pool.c diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c index d4bf7bf..9806376 100644 --- a/test/validation/odp_pktio.c +++ b/test/validation/odp_pktio.c @@ -420,7 +420,7 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, CU_ASSERT(i == num_pkts); } -static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) +static void test_txrx(odp_queue_type_t q_type, int num_pkts) { int ret, i, if_b; pktio_info_t pktios[MAX_NUM_IFACES]; @@ -456,34 +456,34 @@ static void pktio_test_txrx(odp_queue_type_t q_type, int num_pkts) } } -static void test_odp_pktio_poll_queue(void) +static void pktio_test_poll_queue(void) { - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 1); + test_txrx(ODP_QUEUE_TYPE_POLL, 1); } -static void test_odp_pktio_poll_multi(void) +static void pktio_test_poll_multi(void) { - pktio_test_txrx(ODP_QUEUE_TYPE_POLL, 4); + test_txrx(ODP_QUEUE_TYPE_POLL, 4); } -static void test_odp_pktio_sched_queue(void) +static void pktio_test_sched_queue(void) { - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 1); + test_txrx(ODP_QUEUE_TYPE_SCHED, 1); } -static void test_odp_pktio_sched_multi(void) +static void pktio_test_sched_multi(void) { - pktio_test_txrx(ODP_QUEUE_TYPE_SCHED, 4); + test_txrx(ODP_QUEUE_TYPE_SCHED, 4); } -static void test_odp_pktio_jumbo(void) +static void pktio_test_jumbo(void) { packet_len = PKT_LEN_JUMBO; - test_odp_pktio_sched_multi(); + pktio_test_sched_multi(); packet_len = PKT_LEN_NORMAL; } -static void test_odp_pktio_mtu(void) +static void pktio_test_mtu(void) { int ret; int mtu; @@ -500,7 +500,7 @@ static void test_odp_pktio_mtu(void) return; } -static void test_odp_pktio_promisc(void) +static void pktio_test_promisc(void) { int ret; odp_pktio_t pktio = create_pktio(iface_name[0]); @@ -525,7 +525,7 @@ static void test_odp_pktio_promisc(void) return; } -static void test_odp_pktio_mac(void) +static void pktio_test_mac(void) { unsigned char mac_addr[ODPH_ETHADDR_LEN]; int mac_len; @@ -551,7 +551,7 @@ static void test_odp_pktio_mac(void) return; } -static void test_odp_pktio_inq_remdef(void) +static void pktio_test_inq_remdef(void) { odp_pktio_t pktio = create_pktio(iface_name[0]); odp_queue_t inq; @@ -575,7 +575,7 @@ static void test_odp_pktio_inq_remdef(void) CU_ASSERT(odp_pktio_close(pktio) == 0); } -static void test_odp_pktio_open(void) +static void pktio_test_open(void) { odp_pktio_t pktio; int i; @@ -591,7 +591,7 @@ static void test_odp_pktio_open(void) CU_ASSERT(pktio == ODP_PKTIO_INVALID); } -static void test_odp_pktio_lookup(void) +static void pktio_test_lookup(void) { odp_pktio_t pktio, pktio_inval; @@ -609,7 +609,7 @@ static void test_odp_pktio_lookup(void) CU_ASSERT(odp_pktio_lookup(iface_name[0]) == ODP_PKTIO_INVALID); } -static void test_odp_pktio_inq(void) +static void pktio_test_inq(void) { odp_pktio_t pktio; @@ -621,7 +621,7 @@ static void test_odp_pktio_inq(void) CU_ASSERT(odp_pktio_close(pktio) == 0); } -static int init_pktio_suite(void) +static int pktio_suite_init(void) { odp_atomic_init_u32(&ip_seq, 0); iface_name[0] = getenv("ODP_PKTIO_IF0"); @@ -647,7 +647,7 @@ static int init_pktio_suite(void) return 0; } -static int term_pktio_suite(void) +static int pktio_suite_term(void) { char pool_name[ODP_POOL_NAME_LEN]; odp_pool_t pool; @@ -676,24 +676,35 @@ static int term_pktio_suite(void) return ret; } -CU_TestInfo pktio_tests[] = { - {"pktio open", test_odp_pktio_open}, - {"pktio lookup", test_odp_pktio_lookup}, - {"pktio inq", test_odp_pktio_inq}, - {"pktio poll queues", test_odp_pktio_poll_queue}, - {"pktio poll multi", test_odp_pktio_poll_multi}, - {"pktio sched queues", test_odp_pktio_sched_queue}, - {"pktio sched multi", test_odp_pktio_sched_multi}, - {"pktio jumbo frames", test_odp_pktio_jumbo}, - {"pktio mtu", test_odp_pktio_mtu}, - {"pktio promisc mode", test_odp_pktio_promisc}, - {"pktio mac", test_odp_pktio_mac}, - {"pktio inq_remdef", test_odp_pktio_inq_remdef}, +CU_TestInfo pktio_suite[] = { + {"pktio open", pktio_test_open}, + {"pktio lookup", pktio_test_lookup}, + {"pktio inq", pktio_test_inq}, + {"pktio poll queues", pktio_test_poll_queue}, + {"pktio poll multi", pktio_test_poll_multi}, + {"pktio sched queues", pktio_test_sched_queue}, + {"pktio sched multi", pktio_test_sched_multi}, + {"pktio jumbo frames", pktio_test_jumbo}, + {"pktio mtu", pktio_test_mtu}, + {"pktio promisc mode", pktio_test_promisc}, + {"pktio mac", pktio_test_mac}, + {"pktio inq_remdef", pktio_test_inq_remdef}, CU_TEST_INFO_NULL }; -CU_SuiteInfo odp_testsuites[] = { +static CU_SuiteInfo pktio_suites[] = { {"Packet I/O", - init_pktio_suite, term_pktio_suite, NULL, NULL, pktio_tests}, + pktio_suite_init, pktio_suite_term, NULL, NULL, pktio_suite}, CU_SUITE_INFO_NULL }; + +static int pktio_main(void) +{ + return odp_cunit_run(pktio_suites); +} + +/* the following main function will be separated when lib is created */ +int main(void) +{ + return pktio_main(); +}
Renaming of things which may be, one day, exported in a lib. This renaming is important, as it creates consistency between test symbols, which is needed if things get eventually exported in the lib. Also, tests are often created from other tests: Fixing the first exemples will help geting future tests better. Also defining local test main. Things that are candidate to be exported in the lib in the future have been named as follows: -Tests, i.e. functions which are used in CUNIT testsuites are named: <Module>_test_* -Test arrays, i.e. arrays of CU_TestInfo, listing the test functions belonging to a suite, are called: <Module>_suite[_*] where the possible suffix can be used if many suites are declared. -CUNIT suite init and termination functions are called: <Module>_suite[_*]_init() and <Module>_suite[_*]_term() respectively. -Suite arrays, i.e. arrays of CU_SuiteInfo used in executables are called: <Module>_suites[_*] where the possible suffix identifies the executable using it, if many. -Main function(s), are called: <Module>_main[_*] where the possible suffix identifies the executable using it Signed-off-by: Christophe Milard <christophe.milard@linaro.org> --- test/Makefile.inc | 7 +++- test/validation/Makefile.am | 8 ++--- test/validation/odp_pktio.c | 81 +++++++++++++++++++++++++-------------------- 3 files changed, 55 insertions(+), 41 deletions(-)