[v13,11/11] app/test: enable unit test on Windows

Message ID 1638928262-13177-12-git-send-email-jizh@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series app/test: enable subset of tests on Windows |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Jie Zhou Dec. 8, 2021, 1:51 a.m. UTC
  Enable a subset of unit tests for Windows CI

- For driver tests, driver owners should enable corresponding tests when
  enabling driver for Windows. For example, the cryptodev tests will be
  enabled by "patch-18949: app/test: enable crypto unit tests on Windows"
  (which depends on this patchset to be merged).
- For dump tests, currently the tests hang on Windows which require
  further investigation.

Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>

---
 app/test/meson.build | 111 ++++++++++++++++++++++---------------------
 1 file changed, 58 insertions(+), 53 deletions(-)
  

Comments

Bruce Richardson Dec. 8, 2021, 4:57 p.m. UTC | #1
On Tue, Dec 07, 2021 at 05:51:02PM -0800, Jie Zhou wrote:
> Enable a subset of unit tests for Windows CI
> 
> - For driver tests, driver owners should enable corresponding tests when
>   enabling driver for Windows. For example, the cryptodev tests will be
>   enabled by "patch-18949: app/test: enable crypto unit tests on Windows"
>   (which depends on this patchset to be merged).
> - For dump tests, currently the tests hang on Windows which require
>   further investigation.
> 
> Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> 

Mostly these changes look ok to me. One small query below.

/Bruce
> ---
>  app/test/meson.build | 111 ++++++++++++++++++++++---------------------
>  1 file changed, 58 insertions(+), 53 deletions(-)
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 97ee83029e..fcf38729e7 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -1,12 +1,6 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017 Intel Corporation
>  
> -if is_windows
> -    build = false

<snip>

>  # The following linkages are an exception to allow running the
>  # unit tests without requiring that the developer install the
>  # DPDK libraries.  Explicit linkage of drivers (plugin libraries)
> @@ -385,7 +390,7 @@ if dpdk_conf.has('RTE_LIB_METRICS')
>      test_sources += ['test_metrics.c']
>      fast_tests += [['metrics_autotest', true]]
>  endif
> -if dpdk_conf.has('RTE_LIB_TELEMETRY')
> +if not is_windows and dpdk_conf.has('RTE_LIB_TELEMETRY')
>      test_sources += ['test_telemetry_json.c', 'test_telemetry_data.c']
>      fast_tests += [['telemetry_json_autotest', true], ['telemetry_data_autotest', true]]

If telemetry is enabled, is there a reason these won't run on Windows?
  
Jie Zhou Dec. 8, 2021, 6:19 p.m. UTC | #2
On Wed, Dec 08, 2021 at 04:57:52PM +0000, Bruce Richardson wrote:
> On Tue, Dec 07, 2021 at 05:51:02PM -0800, Jie Zhou wrote:
> > Enable a subset of unit tests for Windows CI
> > 
> > - For driver tests, driver owners should enable corresponding tests when
> >   enabling driver for Windows. For example, the cryptodev tests will be
> >   enabled by "patch-18949: app/test: enable crypto unit tests on Windows"
> >   (which depends on this patchset to be merged).
> > - For dump tests, currently the tests hang on Windows which require
> >   further investigation.
> > 
> > Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> > 
> 
> Mostly these changes look ok to me. One small query below.
> 
> /Bruce
> > ---
> >  app/test/meson.build | 111 ++++++++++++++++++++++---------------------
> >  1 file changed, 58 insertions(+), 53 deletions(-)
> > 
> > diff --git a/app/test/meson.build b/app/test/meson.build
> > index 97ee83029e..fcf38729e7 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -1,12 +1,6 @@
> >  # SPDX-License-Identifier: BSD-3-Clause
> >  # Copyright(c) 2017 Intel Corporation
> >  
> > -if is_windows
> > -    build = false
> 
> <snip>
> 
> >  # The following linkages are an exception to allow running the
> >  # unit tests without requiring that the developer install the
> >  # DPDK libraries.  Explicit linkage of drivers (plugin libraries)
> > @@ -385,7 +390,7 @@ if dpdk_conf.has('RTE_LIB_METRICS')
> >      test_sources += ['test_metrics.c']
> >      fast_tests += [['metrics_autotest', true]]
> >  endif
> > -if dpdk_conf.has('RTE_LIB_TELEMETRY')
> > +if not is_windows and dpdk_conf.has('RTE_LIB_TELEMETRY')
> >      test_sources += ['test_telemetry_json.c', 'test_telemetry_data.c']
> >      fast_tests += [['telemetry_json_autotest', true], ['telemetry_data_autotest', true]]
> 
> If telemetry is enabled, is there a reason these won't run on Windows?

Hi Bruce, the telemetry test has many POSIX socket specific codes which requires replacement for Windows. We will work on investigation of enabling more tests (including the telemetry tests) after this patchset. Sorry I forgot to mention that in the commit message, but it is on my list. The current goal is to get this patchset merged asap before another upstream churn to rebase, rework, etc. If you prefer me to send out a V14 to add this info into commit message or remove this change from meson.build but add test stub into the specific test files, please just let me know. Thanks for your understanding. -- Jie
  
Bruce Richardson Dec. 8, 2021, 6:23 p.m. UTC | #3
On Wed, Dec 08, 2021 at 10:19:27AM -0800, Jie Zhou wrote:
> On Wed, Dec 08, 2021 at 04:57:52PM +0000, Bruce Richardson wrote:
> > On Tue, Dec 07, 2021 at 05:51:02PM -0800, Jie Zhou wrote:
> > > Enable a subset of unit tests for Windows CI
> > > 
> > > - For driver tests, driver owners should enable corresponding tests when
> > >   enabling driver for Windows. For example, the cryptodev tests will be
> > >   enabled by "patch-18949: app/test: enable crypto unit tests on Windows"
> > >   (which depends on this patchset to be merged).
> > > - For dump tests, currently the tests hang on Windows which require
> > >   further investigation.
> > > 
> > > Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> > > 
> > 
> > Mostly these changes look ok to me. One small query below.
> > 
> > /Bruce
> > > ---
> > >  app/test/meson.build | 111 ++++++++++++++++++++++---------------------
> > >  1 file changed, 58 insertions(+), 53 deletions(-)
> > > 
> > > diff --git a/app/test/meson.build b/app/test/meson.build
> > > index 97ee83029e..fcf38729e7 100644
> > > --- a/app/test/meson.build
> > > +++ b/app/test/meson.build
> > > @@ -1,12 +1,6 @@
> > >  # SPDX-License-Identifier: BSD-3-Clause
> > >  # Copyright(c) 2017 Intel Corporation
> > >  
> > > -if is_windows
> > > -    build = false
> > 
> > <snip>
> > 
> > >  # The following linkages are an exception to allow running the
> > >  # unit tests without requiring that the developer install the
> > >  # DPDK libraries.  Explicit linkage of drivers (plugin libraries)
> > > @@ -385,7 +390,7 @@ if dpdk_conf.has('RTE_LIB_METRICS')
> > >      test_sources += ['test_metrics.c']
> > >      fast_tests += [['metrics_autotest', true]]
> > >  endif
> > > -if dpdk_conf.has('RTE_LIB_TELEMETRY')
> > > +if not is_windows and dpdk_conf.has('RTE_LIB_TELEMETRY')
> > >      test_sources += ['test_telemetry_json.c', 'test_telemetry_data.c']
> > >      fast_tests += [['telemetry_json_autotest', true], ['telemetry_data_autotest', true]]
> > 
> > If telemetry is enabled, is there a reason these won't run on Windows?
> 
> Hi Bruce, the telemetry test has many POSIX socket specific codes which requires replacement for Windows. We will work on investigation of enabling more tests (including the telemetry tests) after this patchset. Sorry I forgot to mention that in the commit message, but it is on my list. The current goal is to get this patchset merged asap before another upstream churn to rebase, rework, etc. If you prefer me to send out a V14 to add this info into commit message or remove this change from meson.build but add test stub into the specific test files, please just let me know. Thanks for your understanding. -- Jie

A line to two added to the commit log is probably sufficient.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  
Jie Zhou Dec. 8, 2021, 6:29 p.m. UTC | #4
On Wed, Dec 08, 2021 at 06:23:30PM +0000, Bruce Richardson wrote:
> On Wed, Dec 08, 2021 at 10:19:27AM -0800, Jie Zhou wrote:
> > On Wed, Dec 08, 2021 at 04:57:52PM +0000, Bruce Richardson wrote:
> > > On Tue, Dec 07, 2021 at 05:51:02PM -0800, Jie Zhou wrote:
> > > > Enable a subset of unit tests for Windows CI
> > > > 
> > > > - For driver tests, driver owners should enable corresponding tests when
> > > >   enabling driver for Windows. For example, the cryptodev tests will be
> > > >   enabled by "patch-18949: app/test: enable crypto unit tests on Windows"
> > > >   (which depends on this patchset to be merged).
> > > > - For dump tests, currently the tests hang on Windows which require
> > > >   further investigation.
> > > > 
> > > > Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> > > > 
> > > 
> > > Mostly these changes look ok to me. One small query below.
> > > 
> > > /Bruce
> > > > ---
> > > >  app/test/meson.build | 111 ++++++++++++++++++++++---------------------
> > > >  1 file changed, 58 insertions(+), 53 deletions(-)
> > > > 
> > > > diff --git a/app/test/meson.build b/app/test/meson.build
> > > > index 97ee83029e..fcf38729e7 100644
> > > > --- a/app/test/meson.build
> > > > +++ b/app/test/meson.build
> > > > @@ -1,12 +1,6 @@
> > > >  # SPDX-License-Identifier: BSD-3-Clause
> > > >  # Copyright(c) 2017 Intel Corporation
> > > >  
> > > > -if is_windows
> > > > -    build = false
> > > 
> > > <snip>
> > > 
> > > >  # The following linkages are an exception to allow running the
> > > >  # unit tests without requiring that the developer install the
> > > >  # DPDK libraries.  Explicit linkage of drivers (plugin libraries)
> > > > @@ -385,7 +390,7 @@ if dpdk_conf.has('RTE_LIB_METRICS')
> > > >      test_sources += ['test_metrics.c']
> > > >      fast_tests += [['metrics_autotest', true]]
> > > >  endif
> > > > -if dpdk_conf.has('RTE_LIB_TELEMETRY')
> > > > +if not is_windows and dpdk_conf.has('RTE_LIB_TELEMETRY')
> > > >      test_sources += ['test_telemetry_json.c', 'test_telemetry_data.c']
> > > >      fast_tests += [['telemetry_json_autotest', true], ['telemetry_data_autotest', true]]
> > > 
> > > If telemetry is enabled, is there a reason these won't run on Windows?
> > 
> > Hi Bruce, the telemetry test has many POSIX socket specific codes which requires replacement for Windows. We will work on investigation of enabling more tests (including the telemetry tests) after this patchset. Sorry I forgot to mention that in the commit message, but it is on my list. The current goal is to get this patchset merged asap before another upstream churn to rebase, rework, etc. If you prefer me to send out a V14 to add this info into commit message or remove this change from meson.build but add test stub into the specific test files, please just let me know. Thanks for your understanding. -- Jie
> 
> A line to two added to the commit log is probably sufficient.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Thanks Bruce. Will add the info into commit message in V14.
  

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index 97ee83029e..fcf38729e7 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -1,12 +1,6 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-if is_windows
-    build = false
-    reason = 'not supported on Windows'
-    subdir_done()
-endif
-
 if not get_option('tests')
     subdir_done()
 endif
@@ -158,32 +152,14 @@  test_sources = files(
 )
 
 test_deps = [
-        'acl',
         'bus_pci',
         'bus_vdev',
-        'bpf',
         'cfgfile',
         'cmdline',
-        'cryptodev',
-        'distributor',
         'dmadev',
-        'efd',
         'ethdev',
-        'eventdev',
-        'fib',
-        'flow_classify',
-        'graph',
         'hash',
-        'ipsec',
-        'lpm',
-        'member',
-        'node',
-        'pipeline',
-        'port',
-        'rawdev',
         'rcu',
-        'reorder',
-        'rib',
         'ring',
         'security',
         'stack',
@@ -334,39 +310,68 @@  perf_test_names = [
 ]
 
 driver_test_names = [
-        'cryptodev_aesni_mb_autotest',
-        'cryptodev_aesni_gcm_autotest',
-        'cryptodev_cn9k_autotest',
-        'cryptodev_cn10k_autotest',
-        'cryptodev_dpaa_sec_autotest',
-        'cryptodev_dpaa2_sec_autotest',
-        'cryptodev_null_autotest',
-        'cryptodev_octeontx2_autotest',
-        'cryptodev_openssl_autotest',
-        'cryptodev_openssl_asym_autotest',
-        'cryptodev_qat_autotest',
-        'cryptodev_sw_armv8_autotest',
-        'cryptodev_sw_kasumi_autotest',
-        'cryptodev_sw_mvsam_autotest',
-        'cryptodev_sw_snow3g_autotest',
-        'cryptodev_sw_zuc_autotest',
-        'dmadev_autotest',
-        'eventdev_selftest_octeontx',
-        'eventdev_selftest_sw',
-        'rawdev_autotest',
 ]
 
 dump_test_names = [
-        'dump_struct_sizes',
-        'dump_mempool',
-        'dump_malloc_stats',
-        'dump_devargs',
-        'dump_log_types',
-        'dump_ring',
-        'dump_physmem',
-        'dump_memzone',
 ]
 
+if not is_windows
+    test_deps += [
+            'acl',
+            'bpf',
+            'cryptodev',
+            'distributor',
+            'efd',
+            'eventdev',
+            'fib',
+            'flow_classify',
+            'graph',
+            'ipsec',
+            'lpm',
+            'member',
+            'node',
+            'pipeline',
+            'port',
+            'rawdev',
+            'reorder',
+            'rib',
+    ]
+
+    driver_test_names += [
+            'cryptodev_aesni_mb_autotest',
+            'cryptodev_aesni_gcm_autotest',
+            'cryptodev_cn9k_autotest',
+            'cryptodev_cn10k_autotest',
+            'cryptodev_dpaa_sec_autotest',
+            'cryptodev_dpaa2_sec_autotest',
+            'cryptodev_null_autotest',
+            'cryptodev_octeontx2_autotest',
+            'cryptodev_openssl_autotest',
+            'cryptodev_openssl_asym_autotest',
+            'cryptodev_qat_autotest',
+            'cryptodev_sw_armv8_autotest',
+            'cryptodev_sw_kasumi_autotest',
+            'cryptodev_sw_mvsam_autotest',
+            'cryptodev_sw_snow3g_autotest',
+            'cryptodev_sw_zuc_autotest',
+            'dmadev_autotest',
+            'eventdev_selftest_octeontx',
+            'eventdev_selftest_sw',
+            'rawdev_autotest',
+    ]
+
+    dump_test_names += [
+            'dump_struct_sizes',
+            'dump_mempool',
+            'dump_malloc_stats',
+            'dump_devargs',
+            'dump_log_types',
+            'dump_ring',
+            'dump_physmem',
+            'dump_memzone',
+    ]
+endif
+
 # The following linkages are an exception to allow running the
 # unit tests without requiring that the developer install the
 # DPDK libraries.  Explicit linkage of drivers (plugin libraries)
@@ -385,7 +390,7 @@  if dpdk_conf.has('RTE_LIB_METRICS')
     test_sources += ['test_metrics.c']
     fast_tests += [['metrics_autotest', true]]
 endif
-if dpdk_conf.has('RTE_LIB_TELEMETRY')
+if not is_windows and dpdk_conf.has('RTE_LIB_TELEMETRY')
     test_sources += ['test_telemetry_json.c', 'test_telemetry_data.c']
     fast_tests += [['telemetry_json_autotest', true], ['telemetry_data_autotest', true]]
 endif