David Marchand <david.march...@redhat.com> writes: > On Fri, Mar 6, 2020 at 4:57 PM Aaron Conole <acon...@redhat.com> wrote: >> >> Ruifeng Wang <ruifeng.w...@arm.com> writes: >> >> >> -----Original Message----- >> >> From: Aaron Conole <acon...@redhat.com> >> >> Sent: Thursday, March 5, 2020 22:37 >> >> To: Ruifeng Wang <ruifeng.w...@arm.com> >> >> Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; dev@dpdk.org; >> >> david.march...@redhat.com; Gavin Hu <gavin...@arm.com>; Honnappa >> >> Nagarahalli <honnappa.nagaraha...@arm.com>; juraj.lin...@pantheon.tech; >> >> nd <n...@arm.com> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage >> >> >> >> Ruifeng Wang <ruifeng.w...@arm.com> writes: >> >> >> >> >> -----Original Message----- >> >> >> From: Aaron Conole <acon...@redhat.com> >> >> >> Sent: Thursday, March 5, 2020 01:31 >> >> >> To: Ruifeng Wang <ruifeng.w...@arm.com> >> >> >> Cc: maicolgabr...@hotmail.com; bruce.richard...@intel.com; >> >> >> dev@dpdk.org; david.march...@redhat.com; Gavin Hu >> >> <gavin...@arm.com>; >> >> >> Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>; >> >> >> juraj.lin...@pantheon.tech; nd <n...@arm.com> >> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage >> >> >> >> >> >> Ruifeng Wang <ruifeng.w...@arm.com> writes: >> >> >> >> >> >> > This test suite is derived from fast-tests suite. Cases in this >> >> >> > suite are run with '--no-huge' flag. >> >> >> > >> >> >> > The suite aims to cover as many as possible test cases out of the >> >> >> > fast-tests suites in the environments without huge pages support, >> >> >> > like containers. >> >> >> > >> >> >> > Signed-off-by: Ruifeng Wang <ruifeng.w...@arm.com> >> >> >> > Reviewed-by: Gavin Hu <gavin...@arm.com> >> >> >> > --- >> >> >> >> >> >> I like this much more. Few comments. >> >> >> >> >> >> > .travis.yml | 10 +++++-- >> >> >> > app/test/meson.build | 71 >> >> >> > ++++++++++++++++++++++++++++++++++++++++++++ >> >> >> >> >> >> You should update doc/guides/prog_guide/meson_ut.rst to include some >> >> >> detail about the new tests suite. >> >> >> >> >> > Thanks. Will update document in next version. >> >> > >> >> >> > 2 files changed, 79 insertions(+), 2 deletions(-) >> >> >> > >> >> >> > diff --git a/.travis.yml b/.travis.yml index b64a81bd0..eed1d96db >> >> >> > 100644 >> >> >> > --- a/.travis.yml >> >> >> > +++ b/.travis.yml >> >> >> > @@ -40,7 +40,7 @@ jobs: >> >> >> > - env: DEF_LIB="static" >> >> >> > arch: amd64 >> >> >> > compiler: gcc >> >> >> > - - env: DEF_LIB="shared" RUN_TESTS=1 >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="fast-tests >> >> >> nohuge-tests" >> >> >> > arch: amd64 >> >> >> > compiler: gcc >> >> >> > - env: DEF_LIB="shared" BUILD_DOCS=1 @@ -63,7 +63,7 @@ jobs: >> >> >> > - env: DEF_LIB="static" >> >> >> > arch: amd64 >> >> >> > compiler: clang >> >> >> > - - env: DEF_LIB="shared" RUN_TESTS=1 >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="fast-tests >> >> >> nohuge-tests" >> >> >> > arch: amd64 >> >> >> > compiler: clang >> >> >> > - env: DEF_LIB="shared" BUILD_DOCS=1 @@ -101,6 +101,9 @@ jobs: >> >> >> > - env: DEF_LIB="static" >> >> >> > arch: arm64 >> >> >> > compiler: gcc >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="nohuge-tests" >> >> >> > + arch: arm64 >> >> >> > + compiler: gcc >> >> >> > - env: DEF_LIB="shared" BUILD_DOCS=1 >> >> >> > arch: arm64 >> >> >> > compiler: gcc >> >> >> > @@ -124,3 +127,6 @@ jobs: >> >> >> > - env: DEF_LIB="shared" >> >> >> > arch: arm64 >> >> >> > compiler: clang >> >> >> > + - env: DEF_LIB="shared" RUN_TESTS=1 TEST_SUITES="nohuge-tests" >> >> >> > + arch: arm64 >> >> >> > + compiler: clang >> >> >> > diff --git a/app/test/meson.build b/app/test/meson.build index >> >> >> > 0a2ce710f..162a1a76f 100644 >> >> >> > --- a/app/test/meson.build >> >> >> > +++ b/app/test/meson.build >> >> >> > @@ -237,6 +237,60 @@ fast_test_names = [ >> >> >> > 'thash_autotest', >> >> >> > ] >> >> >> >> >> >> Shouldn't we also trim the list of fast-tests? Otherwise, these >> >> >> tests will run twice. >> >> >> >> >> > I think you mean to have exclusive lists for fast-tests and >> >> > nohuge-tests. >> >> >> >> That's what I was thinking. >> >> >> >> > Overlapped cases will run twice if both test suites are opted in. >> >> > But the two runs are not the same, one runs with hugepage and the >> >> > other runs in no-huge mode. >> >> >> >> Is it really so different between huge and no-huge? Most of the libraries >> >> won't care - they call the rte_**alloc functions, and it returns blocks of >> >> memory. Maybe I am simplifying it too much. >> >> >> >> > If fast-tests list is splited, we will need to always run multiple >> >> > suites to cover all fast tests. >> >> > We can keep x86 to run only fast-tests suite to avoid extra test runs >> >> > if they are not necessary. Thoughts? >> >> >> >> I guess since most DPDK usage will be with hugepages, we should prefer to >> >> test with it. I don't care too much about the color of this particular >> >> shed. If >> >> you want to do it that way, it's okay by me - it gives us the coverage, >> >> and >> >> doesn't duplicate tests between those environments. >> >> >> >> BUT it means when we add a new test to the suite, we need to remember to >> >> add it in two places - fast_test and nohuge_test. That almost guarantees >> >> we >> >> will miss tests (because we accidentally don't add it to nohuge). Maybe >> >> there's another way, like keep a list of all the tests and some >> >> information on >> >> whether the test needs hugepages to run. Then if there are no hugepages >> >> available, we can write that we SKIP the tests that don't support huge >> >> pages. >> >> In that way, we don't need two different lists - and if there are >> >> hugepages, >> >> we will run all the test cases. >> >> WDYT? >> >> >> > Yes. Agree with you that having duplicate tests in suites is error prone. >> >> Cool! >> >> > IIUC, cases in a suite is determined at build time, as well as command >> > options to run cases. >> > This implies hugepage availability needs to be detected at build time if >> > we want to run only >> > suitable cases in suite in an environment. It could be something we don't >> > want. >> > >> > I'll trim fast-tests in next version to remove duplicated cases. >> >> I think it might be better to make the array something like (just a >> psuedo-code example): >> >> # psuedo-code to check for hugepages >> has_hugepages = check_for_hugepages() >> >> ... >> >> fast_test_names = [ >> ['acl_autotest', true], >> ['alarm_autotest', true], >> ['atomic_autotest', true], >> ... >> >> Then in the code: >> >> foreach arg : fast_test_names >> .... >> if not arg[1] >> test(arg[0], ...) >> if has_hugepages and arg[1] >> test(arg[0], ) >> >> Does it make sense? Do you see a problem? > > I just hope meson won't create a dependency on hugepage availability... > Thinking about stuff like: > https://git.dpdk.org/dpdk/commit/?id=599d67b6a4bf96096352cc5fbc8fc28e54a1ca62
Yes, I agree. We probably need to do some kind of similar abstraction.
