On Tue, Jun 02, 2015 at 04:51:09PM +0200, Christophe Milard wrote: > On 2 June 2015 at 11:22, Stuart Haslam <[email protected]> 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 <[email protected]> > > > --- > > > 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. -- Stuart. _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
