[v2,2/2] ci: add test suite run without hugepage

Message ID 20200228041904.195597-3-ruifeng.wang@arm.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series no-huge test suite |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed

Commit Message

Ruifeng Wang Feb. 28, 2020, 4:19 a.m. UTC
  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.wang@arm.com>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
---
 .travis.yml          | 10 +++++--
 app/test/meson.build | 71 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 2 deletions(-)
  

Comments

Aaron Conole March 4, 2020, 5:31 p.m. UTC | #1
Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> Reviewed-by: Gavin Hu <gavin.hu@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.

>  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.

ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684

> +nohuge_test_names = [
> +        'byteorder_autotest',
> +        'cmdline_autotest',
> +        'common_autotest',
> +        'cpuflags_autotest',
> +        'cycles_autotest',
> +        'debug_autotest',
> +        'eal_flags_n_opt_autotest',
> +        'eal_flags_no_huge_autotest',
> +        'eal_flags_vdev_opt_autotest',
> +        'eal_fs_autotest',
> +        'errno_autotest',
> +        'event_ring_autotest',
> +        'fib_autotest',
> +        'fib6_autotest',
> +        'interrupt_autotest',
> +        'logs_autotest',
> +        'lpm_autotest',
> +        'lpm6_autotest',
> +        'memcpy_autotest',
> +        'meter_autotest',
> +        'per_lcore_autotest',
> +        'prefetch_autotest',
> +        'rcu_qsbr_autotest',
> +        'red_autotest',
> +        'rib_autotest',
> +        'rib6_autotest',
> +        'ring_autotest',
> +        'rwlock_rda_autotest',
> +        'rwlock_rds_wrm_autotest',
> +        'rwlock_rde_wro_autotest',
> +        'sched_autotest',
> +        'spinlock_autotest',
> +        'string_autotest',
> +        'tailq_autotest',
> +        'user_delay_us',
> +        'version_autotest',
> +        'crc_autotest',
> +        'delay_us_sleep_autotest',
> +        'eventdev_common_autotest',
> +        'fbarray_autotest',
> +        'ipsec_autotest',
> +        'kni_autotest',
> +        'kvargs_autotest',
> +        'member_autotest',
> +        'metrics_autotest',
> +        'power_cpufreq_autotest',
> +        'power_autotest',
> +        'power_kvm_vm_autotest',
> +        'reorder_autotest',
> +        'service_autotest',
> +        'thash_autotest',
> +]
> +
>  perf_test_names = [
>          'ring_perf_autotest',
>          'mempool_perf_autotest',
> @@ -341,6 +395,10 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
>  	fast_test_names += 'latencystats_autotest'
>  	driver_test_names += 'link_bonding_mode4_autotest'
>  	fast_test_names += 'pdump_autotest'
> +	nohuge_test_names += 'ring_pmd_autotest'
> +	nohuge_test_names += 'bitratestats_autotest'
> +	nohuge_test_names += 'latencystats_autotest'
> +	nohuge_test_names += 'pdump_autotest'
>  endif
>  
>  if dpdk_conf.has('RTE_LIBRTE_POWER')
> @@ -430,6 +488,19 @@ foreach arg : fast_test_names
>  	endif
>  endforeach
>  
> +foreach arg : nohuge_test_names
> +	if host_machine.system() == 'linux'
> +		test(arg, dpdk_test,
> +			  env : ['DPDK_TEST=' + arg],
> +			  args : test_args +
> +				 ['--no-huge'] + ['-m 1024'] +
> +				 ['--file-prefix=@0@'.format(arg)],
> +		timeout : timeout_seconds_fast,
> +		is_parallel : false,
> +		suite : 'nohuge-tests')
> +	endif
> +endforeach
> +
>  foreach arg : perf_test_names
>  	test(arg, dpdk_test,
>  	env : ['DPDK_TEST=' + arg],
  
Ruifeng Wang March 5, 2020, 3:41 a.m. UTC | #2
> -----Original Message-----
> From: Aaron Conole <aconole@redhat.com>
> Sent: Thursday, March 5, 2020 01:31
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
> nd <nd@arm.com>
> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> > Reviewed-by: Gavin Hu <gavin.hu@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.
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.

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?

> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
> 
> > +nohuge_test_names = [
> > +        'byteorder_autotest',
> > +        'cmdline_autotest',
> > +        'common_autotest',
> > +        'cpuflags_autotest',
> > +        'cycles_autotest',
> > +        'debug_autotest',
> > +        'eal_flags_n_opt_autotest',
> > +        'eal_flags_no_huge_autotest',
> > +        'eal_flags_vdev_opt_autotest',
> > +        'eal_fs_autotest',
> > +        'errno_autotest',
> > +        'event_ring_autotest',
> > +        'fib_autotest',
> > +        'fib6_autotest',
> > +        'interrupt_autotest',
> > +        'logs_autotest',
> > +        'lpm_autotest',
> > +        'lpm6_autotest',
> > +        'memcpy_autotest',
> > +        'meter_autotest',
> > +        'per_lcore_autotest',
> > +        'prefetch_autotest',
> > +        'rcu_qsbr_autotest',
> > +        'red_autotest',
> > +        'rib_autotest',
> > +        'rib6_autotest',
> > +        'ring_autotest',
> > +        'rwlock_rda_autotest',
> > +        'rwlock_rds_wrm_autotest',
> > +        'rwlock_rde_wro_autotest',
> > +        'sched_autotest',
> > +        'spinlock_autotest',
> > +        'string_autotest',
> > +        'tailq_autotest',
> > +        'user_delay_us',
> > +        'version_autotest',
> > +        'crc_autotest',
> > +        'delay_us_sleep_autotest',
> > +        'eventdev_common_autotest',
> > +        'fbarray_autotest',
> > +        'ipsec_autotest',
> > +        'kni_autotest',
> > +        'kvargs_autotest',
> > +        'member_autotest',
> > +        'metrics_autotest',
> > +        'power_cpufreq_autotest',
> > +        'power_autotest',
> > +        'power_kvm_vm_autotest',
> > +        'reorder_autotest',
> > +        'service_autotest',
> > +        'thash_autotest',
> > +]
> > +
> >  perf_test_names = [
> >          'ring_perf_autotest',
> >          'mempool_perf_autotest',
> > @@ -341,6 +395,10 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >  	fast_test_names += 'latencystats_autotest'
> >  	driver_test_names += 'link_bonding_mode4_autotest'
> >  	fast_test_names += 'pdump_autotest'
> > +	nohuge_test_names += 'ring_pmd_autotest'
> > +	nohuge_test_names += 'bitratestats_autotest'
> > +	nohuge_test_names += 'latencystats_autotest'
> > +	nohuge_test_names += 'pdump_autotest'
> >  endif
> >
> >  if dpdk_conf.has('RTE_LIBRTE_POWER')
> > @@ -430,6 +488,19 @@ foreach arg : fast_test_names
> >  	endif
> >  endforeach
> >
> > +foreach arg : nohuge_test_names
> > +	if host_machine.system() == 'linux'
> > +		test(arg, dpdk_test,
> > +			  env : ['DPDK_TEST=' + arg],
> > +			  args : test_args +
> > +				 ['--no-huge'] + ['-m 1024'] +
> > +				 ['--file-prefix=@0@'.format(arg)],
> > +		timeout : timeout_seconds_fast,
> > +		is_parallel : false,
> > +		suite : 'nohuge-tests')
> > +	endif
> > +endforeach
> > +
> >  foreach arg : perf_test_names
> >  	test(arg, dpdk_test,
> >  	env : ['DPDK_TEST=' + arg],
  
Aaron Conole March 5, 2020, 2:36 p.m. UTC | #3
Ruifeng Wang <Ruifeng.Wang@arm.com> writes:

>> -----Original Message-----
>> From: Aaron Conole <aconole@redhat.com>
>> Sent: Thursday, March 5, 2020 01:31
>> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
>> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
>> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
>> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
>> nd <nd@arm.com>
>> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
>> 
>> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
>> > Reviewed-by: Gavin Hu <gavin.hu@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?

>> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
>> 
>> > +nohuge_test_names = [
>> > +        'byteorder_autotest',
>> > +        'cmdline_autotest',
>> > +        'common_autotest',
>> > +        'cpuflags_autotest',
>> > +        'cycles_autotest',
>> > +        'debug_autotest',
>> > +        'eal_flags_n_opt_autotest',
>> > +        'eal_flags_no_huge_autotest',
>> > +        'eal_flags_vdev_opt_autotest',
>> > +        'eal_fs_autotest',
>> > +        'errno_autotest',
>> > +        'event_ring_autotest',
>> > +        'fib_autotest',
>> > +        'fib6_autotest',
>> > +        'interrupt_autotest',
>> > +        'logs_autotest',
>> > +        'lpm_autotest',
>> > +        'lpm6_autotest',
>> > +        'memcpy_autotest',
>> > +        'meter_autotest',
>> > +        'per_lcore_autotest',
>> > +        'prefetch_autotest',
>> > +        'rcu_qsbr_autotest',
>> > +        'red_autotest',
>> > +        'rib_autotest',
>> > +        'rib6_autotest',
>> > +        'ring_autotest',
>> > +        'rwlock_rda_autotest',
>> > +        'rwlock_rds_wrm_autotest',
>> > +        'rwlock_rde_wro_autotest',
>> > +        'sched_autotest',
>> > +        'spinlock_autotest',
>> > +        'string_autotest',
>> > +        'tailq_autotest',
>> > +        'user_delay_us',
>> > +        'version_autotest',
>> > +        'crc_autotest',
>> > +        'delay_us_sleep_autotest',
>> > +        'eventdev_common_autotest',
>> > +        'fbarray_autotest',
>> > +        'ipsec_autotest',
>> > +        'kni_autotest',
>> > +        'kvargs_autotest',
>> > +        'member_autotest',
>> > +        'metrics_autotest',
>> > +        'power_cpufreq_autotest',
>> > +        'power_autotest',
>> > +        'power_kvm_vm_autotest',
>> > +        'reorder_autotest',
>> > +        'service_autotest',
>> > +        'thash_autotest',
>> > +]
>> > +
>> >  perf_test_names = [
>> >          'ring_perf_autotest',
>> >          'mempool_perf_autotest',
>> > @@ -341,6 +395,10 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
>> >  	fast_test_names += 'latencystats_autotest'
>> >  	driver_test_names += 'link_bonding_mode4_autotest'
>> >  	fast_test_names += 'pdump_autotest'
>> > +	nohuge_test_names += 'ring_pmd_autotest'
>> > +	nohuge_test_names += 'bitratestats_autotest'
>> > +	nohuge_test_names += 'latencystats_autotest'
>> > +	nohuge_test_names += 'pdump_autotest'
>> >  endif
>> >
>> >  if dpdk_conf.has('RTE_LIBRTE_POWER')
>> > @@ -430,6 +488,19 @@ foreach arg : fast_test_names
>> >  	endif
>> >  endforeach
>> >
>> > +foreach arg : nohuge_test_names
>> > +	if host_machine.system() == 'linux'
>> > +		test(arg, dpdk_test,
>> > +			  env : ['DPDK_TEST=' + arg],
>> > +			  args : test_args +
>> > +				 ['--no-huge'] + ['-m 1024'] +
>> > +				 ['--file-prefix=@0@'.format(arg)],
>> > +		timeout : timeout_seconds_fast,
>> > +		is_parallel : false,
>> > +		suite : 'nohuge-tests')
>> > +	endif
>> > +endforeach
>> > +
>> >  foreach arg : perf_test_names
>> >  	test(arg, dpdk_test,
>> >  	env : ['DPDK_TEST=' + arg],
  
Ruifeng Wang March 6, 2020, 8:09 a.m. UTC | #4
> -----Original Message-----
> From: Aaron Conole <aconole@redhat.com>
> Sent: Thursday, March 5, 2020 22:37
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
> nd <nd@arm.com>
> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> 
> >> -----Original Message-----
> >> From: Aaron Conole <aconole@redhat.com>
> >> Sent: Thursday, March 5, 2020 01:31
> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> <Gavin.Hu@arm.com>;
> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >>
> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> >> > Reviewed-by: Gavin Hu <gavin.hu@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.

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.
Thank you.

> >> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
> >>
> >> > +nohuge_test_names = [
> >> > +        'byteorder_autotest',
> >> > +        'cmdline_autotest',
> >> > +        'common_autotest',
> >> > +        'cpuflags_autotest',
> >> > +        'cycles_autotest',
> >> > +        'debug_autotest',
> >> > +        'eal_flags_n_opt_autotest',
> >> > +        'eal_flags_no_huge_autotest',
> >> > +        'eal_flags_vdev_opt_autotest',
> >> > +        'eal_fs_autotest',
> >> > +        'errno_autotest',
> >> > +        'event_ring_autotest',
> >> > +        'fib_autotest',
> >> > +        'fib6_autotest',
> >> > +        'interrupt_autotest',
> >> > +        'logs_autotest',
> >> > +        'lpm_autotest',
> >> > +        'lpm6_autotest',
> >> > +        'memcpy_autotest',
> >> > +        'meter_autotest',
> >> > +        'per_lcore_autotest',
> >> > +        'prefetch_autotest',
> >> > +        'rcu_qsbr_autotest',
> >> > +        'red_autotest',
> >> > +        'rib_autotest',
> >> > +        'rib6_autotest',
> >> > +        'ring_autotest',
> >> > +        'rwlock_rda_autotest',
> >> > +        'rwlock_rds_wrm_autotest',
> >> > +        'rwlock_rde_wro_autotest',
> >> > +        'sched_autotest',
> >> > +        'spinlock_autotest',
> >> > +        'string_autotest',
> >> > +        'tailq_autotest',
> >> > +        'user_delay_us',
> >> > +        'version_autotest',
> >> > +        'crc_autotest',
> >> > +        'delay_us_sleep_autotest',
> >> > +        'eventdev_common_autotest',
> >> > +        'fbarray_autotest',
> >> > +        'ipsec_autotest',
> >> > +        'kni_autotest',
> >> > +        'kvargs_autotest',
> >> > +        'member_autotest',
> >> > +        'metrics_autotest',
> >> > +        'power_cpufreq_autotest',
> >> > +        'power_autotest',
> >> > +        'power_kvm_vm_autotest',
> >> > +        'reorder_autotest',
> >> > +        'service_autotest',
> >> > +        'thash_autotest',
> >> > +]
> >> > +
> >> >  perf_test_names = [
> >> >          'ring_perf_autotest',
> >> >          'mempool_perf_autotest',
> >> > @@ -341,6 +395,10 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >> >  	fast_test_names += 'latencystats_autotest'
> >> >  	driver_test_names += 'link_bonding_mode4_autotest'
> >> >  	fast_test_names += 'pdump_autotest'
> >> > +	nohuge_test_names += 'ring_pmd_autotest'
> >> > +	nohuge_test_names += 'bitratestats_autotest'
> >> > +	nohuge_test_names += 'latencystats_autotest'
> >> > +	nohuge_test_names += 'pdump_autotest'
> >> >  endif
> >> >
> >> >  if dpdk_conf.has('RTE_LIBRTE_POWER') @@ -430,6 +488,19 @@
> foreach
> >> > arg : fast_test_names
> >> >  	endif
> >> >  endforeach
> >> >
> >> > +foreach arg : nohuge_test_names
> >> > +	if host_machine.system() == 'linux'
> >> > +		test(arg, dpdk_test,
> >> > +			  env : ['DPDK_TEST=' + arg],
> >> > +			  args : test_args +
> >> > +				 ['--no-huge'] + ['-m 1024'] +
> >> > +				 ['--file-prefix=@0@'.format(arg)],
> >> > +		timeout : timeout_seconds_fast,
> >> > +		is_parallel : false,
> >> > +		suite : 'nohuge-tests')
> >> > +	endif
> >> > +endforeach
> >> > +
> >> >  foreach arg : perf_test_names
> >> >  	test(arg, dpdk_test,
> >> >  	env : ['DPDK_TEST=' + arg],
  
Aaron Conole March 6, 2020, 3:56 p.m. UTC | #5
Ruifeng Wang <Ruifeng.Wang@arm.com> writes:

>> -----Original Message-----
>> From: Aaron Conole <aconole@redhat.com>
>> Sent: Thursday, March 5, 2020 22:37
>> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
>> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
>> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
>> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
>> nd <nd@arm.com>
>> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
>> 
>> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
>> 
>> >> -----Original Message-----
>> >> From: Aaron Conole <aconole@redhat.com>
>> >> Sent: Thursday, March 5, 2020 01:31
>> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
>> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
>> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
>> <Gavin.Hu@arm.com>;
>> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
>> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
>> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
>> >>
>> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
>> >> > Reviewed-by: Gavin Hu <gavin.hu@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?

> Thank you.
>
>> >> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
>> >>
>> >> > +nohuge_test_names = [
>> >> > +        'byteorder_autotest',
>> >> > +        'cmdline_autotest',
>> >> > +        'common_autotest',
>> >> > +        'cpuflags_autotest',
>> >> > +        'cycles_autotest',
>> >> > +        'debug_autotest',
>> >> > +        'eal_flags_n_opt_autotest',
>> >> > +        'eal_flags_no_huge_autotest',
>> >> > +        'eal_flags_vdev_opt_autotest',
>> >> > +        'eal_fs_autotest',
>> >> > +        'errno_autotest',
>> >> > +        'event_ring_autotest',
>> >> > +        'fib_autotest',
>> >> > +        'fib6_autotest',
>> >> > +        'interrupt_autotest',
>> >> > +        'logs_autotest',
>> >> > +        'lpm_autotest',
>> >> > +        'lpm6_autotest',
>> >> > +        'memcpy_autotest',
>> >> > +        'meter_autotest',
>> >> > +        'per_lcore_autotest',
>> >> > +        'prefetch_autotest',
>> >> > +        'rcu_qsbr_autotest',
>> >> > +        'red_autotest',
>> >> > +        'rib_autotest',
>> >> > +        'rib6_autotest',
>> >> > +        'ring_autotest',
>> >> > +        'rwlock_rda_autotest',
>> >> > +        'rwlock_rds_wrm_autotest',
>> >> > +        'rwlock_rde_wro_autotest',
>> >> > +        'sched_autotest',
>> >> > +        'spinlock_autotest',
>> >> > +        'string_autotest',
>> >> > +        'tailq_autotest',
>> >> > +        'user_delay_us',
>> >> > +        'version_autotest',
>> >> > +        'crc_autotest',
>> >> > +        'delay_us_sleep_autotest',
>> >> > +        'eventdev_common_autotest',
>> >> > +        'fbarray_autotest',
>> >> > +        'ipsec_autotest',
>> >> > +        'kni_autotest',
>> >> > +        'kvargs_autotest',
>> >> > +        'member_autotest',
>> >> > +        'metrics_autotest',
>> >> > +        'power_cpufreq_autotest',
>> >> > +        'power_autotest',
>> >> > +        'power_kvm_vm_autotest',
>> >> > +        'reorder_autotest',
>> >> > +        'service_autotest',
>> >> > +        'thash_autotest',
>> >> > +]
>> >> > +
>> >> >  perf_test_names = [
>> >> >          'ring_perf_autotest',
>> >> >          'mempool_perf_autotest',
>> >> > @@ -341,6 +395,10 @@ if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
>> >> >  	fast_test_names += 'latencystats_autotest'
>> >> >  	driver_test_names += 'link_bonding_mode4_autotest'
>> >> >  	fast_test_names += 'pdump_autotest'
>> >> > +	nohuge_test_names += 'ring_pmd_autotest'
>> >> > +	nohuge_test_names += 'bitratestats_autotest'
>> >> > +	nohuge_test_names += 'latencystats_autotest'
>> >> > +	nohuge_test_names += 'pdump_autotest'
>> >> >  endif
>> >> >
>> >> >  if dpdk_conf.has('RTE_LIBRTE_POWER') @@ -430,6 +488,19 @@
>> foreach
>> >> > arg : fast_test_names
>> >> >  	endif
>> >> >  endforeach
>> >> >
>> >> > +foreach arg : nohuge_test_names
>> >> > +	if host_machine.system() == 'linux'
>> >> > +		test(arg, dpdk_test,
>> >> > +			  env : ['DPDK_TEST=' + arg],
>> >> > +			  args : test_args +
>> >> > +				 ['--no-huge'] + ['-m 1024'] +
>> >> > +				 ['--file-prefix=@0@'.format(arg)],
>> >> > +		timeout : timeout_seconds_fast,
>> >> > +		is_parallel : false,
>> >> > +		suite : 'nohuge-tests')
>> >> > +	endif
>> >> > +endforeach
>> >> > +
>> >> >  foreach arg : perf_test_names
>> >> >  	test(arg, dpdk_test,
>> >> >  	env : ['DPDK_TEST=' + arg],
  
David Marchand March 6, 2020, 4:05 p.m. UTC | #6
On Fri, Mar 6, 2020 at 4:57 PM Aaron Conole <aconole@redhat.com> wrote:
>
> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
>
> >> -----Original Message-----
> >> From: Aaron Conole <aconole@redhat.com>
> >> Sent: Thursday, March 5, 2020 22:37
> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> >> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> >> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
> >> nd <nd@arm.com>
> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >>
> >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >>
> >> >> -----Original Message-----
> >> >> From: Aaron Conole <aconole@redhat.com>
> >> >> Sent: Thursday, March 5, 2020 01:31
> >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> >> <Gavin.Hu@arm.com>;
> >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >> >>
> >> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> >> >> > Reviewed-by: Gavin Hu <gavin.hu@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
  
Aaron Conole March 6, 2020, 4:16 p.m. UTC | #7
David Marchand <david.marchand@redhat.com> writes:

> On Fri, Mar 6, 2020 at 4:57 PM Aaron Conole <aconole@redhat.com> wrote:
>>
>> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
>>
>> >> -----Original Message-----
>> >> From: Aaron Conole <aconole@redhat.com>
>> >> Sent: Thursday, March 5, 2020 22:37
>> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
>> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
>> >> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
>> >> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
>> >> nd <nd@arm.com>
>> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
>> >>
>> >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
>> >>
>> >> >> -----Original Message-----
>> >> >> From: Aaron Conole <aconole@redhat.com>
>> >> >> Sent: Thursday, March 5, 2020 01:31
>> >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
>> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
>> >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
>> >> <Gavin.Hu@arm.com>;
>> >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
>> >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
>> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
>> >> >>
>> >> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
>> >> >> > Reviewed-by: Gavin Hu <gavin.hu@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.
  
Bruce Richardson March 6, 2020, 4:33 p.m. UTC | #8
On Fri, Mar 06, 2020 at 11:16:24AM -0500, Aaron Conole wrote:
> David Marchand <david.marchand@redhat.com> writes:
> 
> > On Fri, Mar 6, 2020 at 4:57 PM Aaron Conole <aconole@redhat.com> wrote:
> >>
> >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >>
> >> >> -----Original Message-----
> >> >> From: Aaron Conole <aconole@redhat.com>
> >> >> Sent: Thursday, March 5, 2020 22:37
> >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> >> >> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> >> >> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
> >> >> nd <nd@arm.com>
> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >> >>
> >> >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >> >>
> >> >> >> -----Original Message-----
> >> >> >> From: Aaron Conole <aconole@redhat.com>
> >> >> >> Sent: Thursday, March 5, 2020 01:31
> >> >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> >> >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> >> >> <Gavin.Hu@arm.com>;
> >> >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> >> >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >> >> >>
> >> >> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> >> >> >> > Reviewed-by: Gavin Hu <gavin.hu@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.
>
Hopefully not, since this is for run/test targets rather than for rebuilds.
Try the simple approach first, and only add abstractions if it causes
problems, I think.
  
Ruifeng Wang March 7, 2020, 2:36 p.m. UTC | #9
> -----Original Message-----
> From: Aaron Conole <aconole@redhat.com>
> Sent: Friday, March 6, 2020 23:57
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; juraj.linkes@pantheon.tech;
> nd <nd@arm.com>
> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> 
> >> -----Original Message-----
> >> From: Aaron Conole <aconole@redhat.com>
> >> Sent: Thursday, March 5, 2020 22:37
> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> <Gavin.Hu@arm.com>;
> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >>
> >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >>
> >> >> -----Original Message-----
> >> >> From: Aaron Conole <aconole@redhat.com>
> >> >> Sent: Thursday, March 5, 2020 01:31
> >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> >> <Gavin.Hu@arm.com>;
> >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without
> >> >> hugepage
> >> >>
> >> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> >> >> > Reviewed-by: Gavin Hu <gavin.hu@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?
> 
Yes, this will keep tests in a single suite.

Actually, I thought about this approach, but had no idea on check_for_hugepages().
Value of "/proc/sys/vm/nr_hugepages" may be not reliable. Hugepage could be allocated at run time after project building.
I can try hugepage allocating to detect, but it looks inelegant.
Any suggestions?

Thank you.
/Ruifeng
> > Thank you.
> >
> >> >> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
> >> >>
> >> >> > +nohuge_test_names = [
> >> >> > +        'byteorder_autotest',
> >> >> > +        'cmdline_autotest',
> >> >> > +        'common_autotest',
> >> >> > +        'cpuflags_autotest',
> >> >> > +        'cycles_autotest',
> >> >> > +        'debug_autotest',
> >> >> > +        'eal_flags_n_opt_autotest',
> >> >> > +        'eal_flags_no_huge_autotest',
> >> >> > +        'eal_flags_vdev_opt_autotest',
> >> >> > +        'eal_fs_autotest',
> >> >> > +        'errno_autotest',
> >> >> > +        'event_ring_autotest',
> >> >> > +        'fib_autotest',
> >> >> > +        'fib6_autotest',
> >> >> > +        'interrupt_autotest',
> >> >> > +        'logs_autotest',
> >> >> > +        'lpm_autotest',
> >> >> > +        'lpm6_autotest',
> >> >> > +        'memcpy_autotest',
> >> >> > +        'meter_autotest',
> >> >> > +        'per_lcore_autotest',
> >> >> > +        'prefetch_autotest',
> >> >> > +        'rcu_qsbr_autotest',
> >> >> > +        'red_autotest',
> >> >> > +        'rib_autotest',
> >> >> > +        'rib6_autotest',
> >> >> > +        'ring_autotest',
> >> >> > +        'rwlock_rda_autotest',
> >> >> > +        'rwlock_rds_wrm_autotest',
> >> >> > +        'rwlock_rde_wro_autotest',
> >> >> > +        'sched_autotest',
> >> >> > +        'spinlock_autotest',
> >> >> > +        'string_autotest',
> >> >> > +        'tailq_autotest',
> >> >> > +        'user_delay_us',
> >> >> > +        'version_autotest',
> >> >> > +        'crc_autotest',
> >> >> > +        'delay_us_sleep_autotest',
> >> >> > +        'eventdev_common_autotest',
> >> >> > +        'fbarray_autotest',
> >> >> > +        'ipsec_autotest',
> >> >> > +        'kni_autotest',
> >> >> > +        'kvargs_autotest',
> >> >> > +        'member_autotest',
> >> >> > +        'metrics_autotest',
> >> >> > +        'power_cpufreq_autotest',
> >> >> > +        'power_autotest',
> >> >> > +        'power_kvm_vm_autotest',
> >> >> > +        'reorder_autotest',
> >> >> > +        'service_autotest',
> >> >> > +        'thash_autotest',
> >> >> > +]
> >> >> > +
> >> >> >  perf_test_names = [
> >> >> >          'ring_perf_autotest',
> >> >> >          'mempool_perf_autotest', @@ -341,6 +395,10 @@ if
> >> >> > dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >> >> >  	fast_test_names += 'latencystats_autotest'
> >> >> >  	driver_test_names += 'link_bonding_mode4_autotest'
> >> >> >  	fast_test_names += 'pdump_autotest'
> >> >> > +	nohuge_test_names += 'ring_pmd_autotest'
> >> >> > +	nohuge_test_names += 'bitratestats_autotest'
> >> >> > +	nohuge_test_names += 'latencystats_autotest'
> >> >> > +	nohuge_test_names += 'pdump_autotest'
> >> >> >  endif
> >> >> >
> >> >> >  if dpdk_conf.has('RTE_LIBRTE_POWER') @@ -430,6 +488,19 @@
> >> foreach
> >> >> > arg : fast_test_names
> >> >> >  	endif
> >> >> >  endforeach
> >> >> >
> >> >> > +foreach arg : nohuge_test_names
> >> >> > +	if host_machine.system() == 'linux'
> >> >> > +		test(arg, dpdk_test,
> >> >> > +			  env : ['DPDK_TEST=' + arg],
> >> >> > +			  args : test_args +
> >> >> > +				 ['--no-huge'] + ['-m 1024'] +
> >> >> > +				 ['--file-prefix=@0@'.format(arg)],
> >> >> > +		timeout : timeout_seconds_fast,
> >> >> > +		is_parallel : false,
> >> >> > +		suite : 'nohuge-tests')
> >> >> > +	endif
> >> >> > +endforeach
> >> >> > +
> >> >> >  foreach arg : perf_test_names
> >> >> >  	test(arg, dpdk_test,
> >> >> >  	env : ['DPDK_TEST=' + arg],
  
Juraj Linkeš March 12, 2020, 7:13 a.m. UTC | #10
Hi,

What if we just split the test names into fast_hugepage_test_name and fast_nohuge_test_names and create two suites (fast-suite and fast-nohuge-suite) from that? That seems like a simpler version of one list with booleans indicating whether a test is a huge/nohuge test, though it would require dev to add tests into the proper list. We could then run the full suite in x86 jobs and the nogue suite in arm jobs (and users could use either suite when they have/don't have hugepages configured).

The other option would be to detect hugepages at runtime and skip the tests in the full suite when there aren't hugepages on the system. Possibly the best option, but where should the check be? Before testcase setup (i.e. if there aren't hugepages, skip the testcase right away)?

Thoughts?
Thanks,
Juraj

-----Original Message-----
From: Ruifeng Wang <Ruifeng.Wang@arm.com> 
Sent: Saturday, March 7, 2020 3:36 PM
To: Aaron Conole <aconole@redhat.com>
Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org; david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Juraj Linkeš <juraj.linkes@pantheon.tech>; nd <nd@arm.com>; nd <nd@arm.com>
Subject: RE: [PATCH v2 2/2] ci: add test suite run without hugepage


> -----Original Message-----
> From: Aaron Conole <aconole@redhat.com>
> Sent: Friday, March 6, 2020 23:57
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; 
> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; 
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; 
> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> 
> >> -----Original Message-----
> >> From: Aaron Conole <aconole@redhat.com>
> >> Sent: Thursday, March 5, 2020 22:37
> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; 
> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> <Gavin.Hu@arm.com>;
> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; 
> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >>
> >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >>
> >> >> -----Original Message-----
> >> >> From: Aaron Conole <aconole@redhat.com>
> >> >> Sent: Thursday, March 5, 2020 01:31
> >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; 
> >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> >> <Gavin.Hu@arm.com>;
> >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; 
> >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without 
> >> >> hugepage
> >> >>
> >> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> >> >> > Reviewed-by: Gavin Hu <gavin.hu@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?
> 
Yes, this will keep tests in a single suite.

Actually, I thought about this approach, but had no idea on check_for_hugepages().
Value of "/proc/sys/vm/nr_hugepages" may be not reliable. Hugepage could be allocated at run time after project building.
I can try hugepage allocating to detect, but it looks inelegant.
Any suggestions?

Thank you.
/Ruifeng
> > Thank you.
> >
> >> >> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
> >> >>
> >> >> > +nohuge_test_names = [
> >> >> > +        'byteorder_autotest',
> >> >> > +        'cmdline_autotest',
> >> >> > +        'common_autotest',
> >> >> > +        'cpuflags_autotest',
> >> >> > +        'cycles_autotest',
> >> >> > +        'debug_autotest',
> >> >> > +        'eal_flags_n_opt_autotest',
> >> >> > +        'eal_flags_no_huge_autotest',
> >> >> > +        'eal_flags_vdev_opt_autotest',
> >> >> > +        'eal_fs_autotest',
> >> >> > +        'errno_autotest',
> >> >> > +        'event_ring_autotest',
> >> >> > +        'fib_autotest',
> >> >> > +        'fib6_autotest',
> >> >> > +        'interrupt_autotest',
> >> >> > +        'logs_autotest',
> >> >> > +        'lpm_autotest',
> >> >> > +        'lpm6_autotest',
> >> >> > +        'memcpy_autotest',
> >> >> > +        'meter_autotest',
> >> >> > +        'per_lcore_autotest',
> >> >> > +        'prefetch_autotest',
> >> >> > +        'rcu_qsbr_autotest',
> >> >> > +        'red_autotest',
> >> >> > +        'rib_autotest',
> >> >> > +        'rib6_autotest',
> >> >> > +        'ring_autotest',
> >> >> > +        'rwlock_rda_autotest',
> >> >> > +        'rwlock_rds_wrm_autotest',
> >> >> > +        'rwlock_rde_wro_autotest',
> >> >> > +        'sched_autotest',
> >> >> > +        'spinlock_autotest',
> >> >> > +        'string_autotest',
> >> >> > +        'tailq_autotest',
> >> >> > +        'user_delay_us',
> >> >> > +        'version_autotest',
> >> >> > +        'crc_autotest',
> >> >> > +        'delay_us_sleep_autotest',
> >> >> > +        'eventdev_common_autotest',
> >> >> > +        'fbarray_autotest',
> >> >> > +        'ipsec_autotest',
> >> >> > +        'kni_autotest',
> >> >> > +        'kvargs_autotest',
> >> >> > +        'member_autotest',
> >> >> > +        'metrics_autotest',
> >> >> > +        'power_cpufreq_autotest',
> >> >> > +        'power_autotest',
> >> >> > +        'power_kvm_vm_autotest',
> >> >> > +        'reorder_autotest',
> >> >> > +        'service_autotest',
> >> >> > +        'thash_autotest',
> >> >> > +]
> >> >> > +
> >> >> >  perf_test_names = [
> >> >> >          'ring_perf_autotest',
> >> >> >          'mempool_perf_autotest', @@ -341,6 +395,10 @@ if
> >> >> > dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> >> >> >  	fast_test_names += 'latencystats_autotest'
> >> >> >  	driver_test_names += 'link_bonding_mode4_autotest'
> >> >> >  	fast_test_names += 'pdump_autotest'
> >> >> > +	nohuge_test_names += 'ring_pmd_autotest'
> >> >> > +	nohuge_test_names += 'bitratestats_autotest'
> >> >> > +	nohuge_test_names += 'latencystats_autotest'
> >> >> > +	nohuge_test_names += 'pdump_autotest'
> >> >> >  endif
> >> >> >
> >> >> >  if dpdk_conf.has('RTE_LIBRTE_POWER') @@ -430,6 +488,19 @@
> >> foreach
> >> >> > arg : fast_test_names
> >> >> >  	endif
> >> >> >  endforeach
> >> >> >
> >> >> > +foreach arg : nohuge_test_names
> >> >> > +	if host_machine.system() == 'linux'
> >> >> > +		test(arg, dpdk_test,
> >> >> > +			  env : ['DPDK_TEST=' + arg],
> >> >> > +			  args : test_args +
> >> >> > +				 ['--no-huge'] + ['-m 1024'] +
> >> >> > +				 ['--file-prefix=@0@'.format(arg)],
> >> >> > +		timeout : timeout_seconds_fast,
> >> >> > +		is_parallel : false,
> >> >> > +		suite : 'nohuge-tests')
> >> >> > +	endif
> >> >> > +endforeach
> >> >> > +
> >> >> >  foreach arg : perf_test_names
> >> >> >  	test(arg, dpdk_test,
> >> >> >  	env : ['DPDK_TEST=' + arg],
  
Ruifeng Wang March 12, 2020, 8:33 a.m. UTC | #11
> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Thursday, March 12, 2020 15:13
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; Aaron Conole
> <aconole@redhat.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd
> <nd@arm.com>
> Subject: RE: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> Hi,
> 
> What if we just split the test names into fast_hugepage_test_name and
> fast_nohuge_test_names and create two suites (fast-suite and fast-nohuge-
> suite) from that? That seems like a simpler version of one list with booleans
> indicating whether a test is a huge/nohuge test, though it would require dev
> to add tests into the proper list. We could then run the full suite in x86 jobs
> and the nogue suite in arm jobs (and users could use either suite when they
> have/don't have hugepages configured).
> 
> The other option would be to detect hugepages at runtime and skip the tests
> in the full suite when there aren't hugepages on the system. Possibly the
> best option, but where should the check be? Before testcase setup (i.e. if
> there aren't hugepages, skip the testcase right away)?
> 
I'm on this direction. Detect hugepages at build time and generate fast_tests suite with applicable cases.
nr_hugepages is checked before testcase setup.

Will send out patches for review soon.

Thanks.
/Ruifeng

> Thoughts?
> Thanks,
> Juraj
> 
> -----Original Message-----
> From: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Sent: Saturday, March 7, 2020 3:36 PM
> To: Aaron Conole <aconole@redhat.com>
> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com; dev@dpdk.org;
> david.marchand@redhat.com; Gavin Hu <Gavin.Hu@arm.com>; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v2 2/2] ci: add test suite run without hugepage
> 
> 
> > -----Original Message-----
> > From: Aaron Conole <aconole@redhat.com>
> > Sent: Friday, March 6, 2020 23:57
> > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> > dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> <Gavin.Hu@arm.com>;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > juraj.linkes@pantheon.tech; nd <nd@arm.com>
> > Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> >
> > Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> >
> > >> -----Original Message-----
> > >> From: Aaron Conole <aconole@redhat.com>
> > >> Sent: Thursday, March 5, 2020 22:37
> > >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> > >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> > <Gavin.Hu@arm.com>;
> > >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> > >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without hugepage
> > >>
> > >> Ruifeng Wang <Ruifeng.Wang@arm.com> writes:
> > >>
> > >> >> -----Original Message-----
> > >> >> From: Aaron Conole <aconole@redhat.com>
> > >> >> Sent: Thursday, March 5, 2020 01:31
> > >> >> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > >> >> Cc: maicolgabriel@hotmail.com; bruce.richardson@intel.com;
> > >> >> dev@dpdk.org; david.marchand@redhat.com; Gavin Hu
> > >> <Gavin.Hu@arm.com>;
> > >> >> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> > >> >> juraj.linkes@pantheon.tech; nd <nd@arm.com>
> > >> >> Subject: Re: [PATCH v2 2/2] ci: add test suite run without
> > >> >> hugepage
> > >> >>
> > >> >> Ruifeng Wang <ruifeng.wang@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.wang@arm.com>
> > >> >> > Reviewed-by: Gavin Hu <gavin.hu@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?
> >
> Yes, this will keep tests in a single suite.
> 
> Actually, I thought about this approach, but had no idea on
> check_for_hugepages().
> Value of "/proc/sys/vm/nr_hugepages" may be not reliable. Hugepage could
> be allocated at run time after project building.
> I can try hugepage allocating to detect, but it looks inelegant.
> Any suggestions?
> 
> Thank you.
> /Ruifeng
> > > Thank you.
> > >
> > >> >> ex: https://travis-ci.com/ovsrobot/dpdk/jobs/292037684
> > >> >>
> > >> >> > +nohuge_test_names = [
> > >> >> > +        'byteorder_autotest',
> > >> >> > +        'cmdline_autotest',
> > >> >> > +        'common_autotest',
> > >> >> > +        'cpuflags_autotest',
> > >> >> > +        'cycles_autotest',
> > >> >> > +        'debug_autotest',
> > >> >> > +        'eal_flags_n_opt_autotest',
> > >> >> > +        'eal_flags_no_huge_autotest',
> > >> >> > +        'eal_flags_vdev_opt_autotest',
> > >> >> > +        'eal_fs_autotest',
> > >> >> > +        'errno_autotest',
> > >> >> > +        'event_ring_autotest',
> > >> >> > +        'fib_autotest',
> > >> >> > +        'fib6_autotest',
> > >> >> > +        'interrupt_autotest',
> > >> >> > +        'logs_autotest',
> > >> >> > +        'lpm_autotest',
> > >> >> > +        'lpm6_autotest',
> > >> >> > +        'memcpy_autotest',
> > >> >> > +        'meter_autotest',
> > >> >> > +        'per_lcore_autotest',
> > >> >> > +        'prefetch_autotest',
> > >> >> > +        'rcu_qsbr_autotest',
> > >> >> > +        'red_autotest',
> > >> >> > +        'rib_autotest',
> > >> >> > +        'rib6_autotest',
> > >> >> > +        'ring_autotest',
> > >> >> > +        'rwlock_rda_autotest',
> > >> >> > +        'rwlock_rds_wrm_autotest',
> > >> >> > +        'rwlock_rde_wro_autotest',
> > >> >> > +        'sched_autotest',
> > >> >> > +        'spinlock_autotest',
> > >> >> > +        'string_autotest',
> > >> >> > +        'tailq_autotest',
> > >> >> > +        'user_delay_us',
> > >> >> > +        'version_autotest',
> > >> >> > +        'crc_autotest',
> > >> >> > +        'delay_us_sleep_autotest',
> > >> >> > +        'eventdev_common_autotest',
> > >> >> > +        'fbarray_autotest',
> > >> >> > +        'ipsec_autotest',
> > >> >> > +        'kni_autotest',
> > >> >> > +        'kvargs_autotest',
> > >> >> > +        'member_autotest',
> > >> >> > +        'metrics_autotest',
> > >> >> > +        'power_cpufreq_autotest',
> > >> >> > +        'power_autotest',
> > >> >> > +        'power_kvm_vm_autotest',
> > >> >> > +        'reorder_autotest',
> > >> >> > +        'service_autotest',
> > >> >> > +        'thash_autotest',
> > >> >> > +]
> > >> >> > +
> > >> >> >  perf_test_names = [
> > >> >> >          'ring_perf_autotest',
> > >> >> >          'mempool_perf_autotest', @@ -341,6 +395,10 @@ if
> > >> >> > dpdk_conf.has('RTE_LIBRTE_RING_PMD')
> > >> >> >  	fast_test_names += 'latencystats_autotest'
> > >> >> >  	driver_test_names += 'link_bonding_mode4_autotest'
> > >> >> >  	fast_test_names += 'pdump_autotest'
> > >> >> > +	nohuge_test_names += 'ring_pmd_autotest'
> > >> >> > +	nohuge_test_names += 'bitratestats_autotest'
> > >> >> > +	nohuge_test_names += 'latencystats_autotest'
> > >> >> > +	nohuge_test_names += 'pdump_autotest'
> > >> >> >  endif
> > >> >> >
> > >> >> >  if dpdk_conf.has('RTE_LIBRTE_POWER') @@ -430,6 +488,19 @@
> > >> foreach
> > >> >> > arg : fast_test_names
> > >> >> >  	endif
> > >> >> >  endforeach
> > >> >> >
> > >> >> > +foreach arg : nohuge_test_names
> > >> >> > +	if host_machine.system() == 'linux'
> > >> >> > +		test(arg, dpdk_test,
> > >> >> > +			  env : ['DPDK_TEST=' + arg],
> > >> >> > +			  args : test_args +
> > >> >> > +				 ['--no-huge'] + ['-m 1024'] +
> > >> >> > +				 ['--file-prefix=@0@'.format(arg)],
> > >> >> > +		timeout : timeout_seconds_fast,
> > >> >> > +		is_parallel : false,
> > >> >> > +		suite : 'nohuge-tests')
> > >> >> > +	endif
> > >> >> > +endforeach
> > >> >> > +
> > >> >> >  foreach arg : perf_test_names
> > >> >> >  	test(arg, dpdk_test,
> > >> >> >  	env : ['DPDK_TEST=' + arg],
  

Patch

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',
 ]
 
+nohuge_test_names = [
+        'byteorder_autotest',
+        'cmdline_autotest',
+        'common_autotest',
+        'cpuflags_autotest',
+        'cycles_autotest',
+        'debug_autotest',
+        'eal_flags_n_opt_autotest',
+        'eal_flags_no_huge_autotest',
+        'eal_flags_vdev_opt_autotest',
+        'eal_fs_autotest',
+        'errno_autotest',
+        'event_ring_autotest',
+        'fib_autotest',
+        'fib6_autotest',
+        'interrupt_autotest',
+        'logs_autotest',
+        'lpm_autotest',
+        'lpm6_autotest',
+        'memcpy_autotest',
+        'meter_autotest',
+        'per_lcore_autotest',
+        'prefetch_autotest',
+        'rcu_qsbr_autotest',
+        'red_autotest',
+        'rib_autotest',
+        'rib6_autotest',
+        'ring_autotest',
+        'rwlock_rda_autotest',
+        'rwlock_rds_wrm_autotest',
+        'rwlock_rde_wro_autotest',
+        'sched_autotest',
+        'spinlock_autotest',
+        'string_autotest',
+        'tailq_autotest',
+        'user_delay_us',
+        'version_autotest',
+        'crc_autotest',
+        'delay_us_sleep_autotest',
+        'eventdev_common_autotest',
+        'fbarray_autotest',
+        'ipsec_autotest',
+        'kni_autotest',
+        'kvargs_autotest',
+        'member_autotest',
+        'metrics_autotest',
+        'power_cpufreq_autotest',
+        'power_autotest',
+        'power_kvm_vm_autotest',
+        'reorder_autotest',
+        'service_autotest',
+        'thash_autotest',
+]
+
 perf_test_names = [
         'ring_perf_autotest',
         'mempool_perf_autotest',
@@ -341,6 +395,10 @@  if dpdk_conf.has('RTE_LIBRTE_RING_PMD')
 	fast_test_names += 'latencystats_autotest'
 	driver_test_names += 'link_bonding_mode4_autotest'
 	fast_test_names += 'pdump_autotest'
+	nohuge_test_names += 'ring_pmd_autotest'
+	nohuge_test_names += 'bitratestats_autotest'
+	nohuge_test_names += 'latencystats_autotest'
+	nohuge_test_names += 'pdump_autotest'
 endif
 
 if dpdk_conf.has('RTE_LIBRTE_POWER')
@@ -430,6 +488,19 @@  foreach arg : fast_test_names
 	endif
 endforeach
 
+foreach arg : nohuge_test_names
+	if host_machine.system() == 'linux'
+		test(arg, dpdk_test,
+			  env : ['DPDK_TEST=' + arg],
+			  args : test_args +
+				 ['--no-huge'] + ['-m 1024'] +
+				 ['--file-prefix=@0@'.format(arg)],
+		timeout : timeout_seconds_fast,
+		is_parallel : false,
+		suite : 'nohuge-tests')
+	endif
+endforeach
+
 foreach arg : perf_test_names
 	test(arg, dpdk_test,
 	env : ['DPDK_TEST=' + arg],