Re: [dpdk-dev] [PATCH v4] doc: add meson ut info in prog guide

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Jan 25, 2019 at 06:20:51AM +0000, Hari Kumar Vemula wrote:
> Add a programmer's guide section for meson ut
> 
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
> v4: Typos corrected
> v3: Modified
> v2: Removed enhancement details
> ---

Some sections I think need rewording to be written more from an end-user
rather than developer point of view, since this text is focused on running unit
tests rather than on writing them. Some additional spelling/grammar
corrections below also.

NOTE: Since this is in the programmers guide, maybe the intention is to
have it more at the programmer level. In which case, you need more info on
writing tests, and what return values should be used etc. other than
"TEST_SKIPPED", and can skip some of the detail on running tests.

Regards,
/Bruce

>  doc/guides/prog_guide/index.rst    |   1 +
>  doc/guides/prog_guide/meson_ut.rst | 159 +++++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+)
>  create mode 100644 doc/guides/prog_guide/meson_ut.rst
> 
> diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
> index 6726b1e8d..f4274573f 100644
> --- a/doc/guides/prog_guide/index.rst
> +++ b/doc/guides/prog_guide/index.rst
> @@ -58,6 +58,7 @@ Programmer's Guide
>      source_org
>      dev_kit_build_system
>      dev_kit_root_make_help
> +    meson_ut
>      extend_dpdk
>      build_app
>      ext_app_lib_make_help
> diff --git a/doc/guides/prog_guide/meson_ut.rst b/doc/guides/prog_guide/meson_ut.rst
> new file mode 100644
> index 000000000..9485b1e63
> --- /dev/null
> +++ b/doc/guides/prog_guide/meson_ut.rst
> @@ -0,0 +1,159 @@
> +..  SPDX-License-Identifier: BSD-3-Clause
> +
> +    Copyright(c) 2014-2018 Intel Corporation.

Should be just 2019, or 2018-2019, since this doc was not started in 2014.

> +
> +.. _Meson:
> +
> +Meson_UT
> +========
> +
> +The meson build for unit tests under different categories is supported using 'test/test/meson.build'.
> +
This line is not wrong, just probably unnecessary. Consider omitting it, I
think, since this doc is about using the unit tests rather than about the
source code layout or individual files.

> +This document describes the below list in detail.
> +
> +*  Building and Running the unit tests.
> +*  Grouping of testcases.
> +*  Parallel and non parallel tests.
> +*  Test suites.
Are test suites not the same as the grouping of test cases?

> +*  How to run different test suites.
> +*  Support for skipped tests.
> +
> +
> +
> +Building and Running the unit tests
> +-----------------------------------
> +
> +*  Create the meson build output folder using command.
> +
> +   ``$ meson <build_dir>``
> +
> +*  Enter into build output folder, which was created by above command.
> +
> +   ``$ cd build``
> +
> +*  Compile DPDK using `$ ninja`.

Like the other commands above, this should be put on its own line.

> +   The output file of the build will be available in meson build folder.
> +   After successful ninja command, binary `dpdk-test` is created in `build/test/test/`.
> +
> +*  Run the unit testcases.
> +
> +   ``$ ninja test`` (or) ``$ meson test``
> +
> +*   To run specific test case via meson command.
> +
> +   ``$ meson test <test case>`` (or) ``$ ninja test <test case>``
> +
> +
> +
> +Grouping of testcases
> +---------------------
> +
> +Testcases has been grouped into below four different groups based on conditions

has -> have.
Remove the word "below" as it's unnecessary.

> +of time duration and performance of the individual testcase.
> +
> +*  fast_parallel_test_names
> +*  fast_non_parallel_test_names
> +*  perf_test_names
> +*  driver_test_names
> +*  dump_test_names

Since you are listing the groups, I think you should just put the groups
without using the official variable names, which looks wrong and doesn't
add any info. Instead the list should be:

* fast tests which can be run in parallel
* fast tests which must run serially
* performance tests
* driver tests
* tests which produce lists of objects as output, and therefore that need manual checking

> +
> +Parallel and non parallel tests
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +By default, meson will run test cases in parallel. However `is_parallel` argument
> +of `test()` in meson.build can be used to run tests in parallel or non-parallel
> +mode based on its functionality. Test cases marked with `is_parallel : true` will
> +run in parallel and tests marked with `is_parallel : false` will run in non-parallel.
> +While non-parallel test is running, no other test should run.
> +Parallel and non parallel test cases are grouped under the `fast_parallel_test_names`
> +and `fast_non_parallel_test_names`.
> +

The above paragraph is too low level. It can probably be dropped or
abbreviated to a single sentence included in the previous section. Again,
this text seems targeted at users, not developers, so variable names in the
meson.build file are irrelevant.

> +
> +
> +Test suites
> +~~~~~~~~~~~
> +
> +Tests are grouped into test suites in meson.build and these suite names can be provided
> +as argument to `meson test` as `--suite ???project_name:label???`.
> +
> +    Ex: ``$ meson test --suite ???DPDK:fast-tests???``
> +
> +Running different test suites
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Below are commands to run testcases using option `suite`
> +
> +*  Test cases from the groups `fast_parallel_test_names` and `fast_non_parallel_test_names`

Again, drop the variable names. Not relevant for users.

> +   are run under 10seconds and below is the meson command to run them.

"are run under 10seconds" -> "should take less than 10 seconds"

> +
> +   ``$ meson test --suite DPDK:fast-tests``
> +
> +*  Test cases from the group `perf_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$ meson test --suite DPDK:perf-tests``
> +
> +*  Test cases from the group `driver_test_names` are run under 600 seconds
> +   and below is the meson command to run them.
> +
> +   ``$ meson test --suite DPDK:driver-tests``
> +
> +*  Test cases from the group `dump_test_names` are run under 600 seconds

Since these just output things, do they really need a 600 second timeout?
Do we even need to mention it?

> +   and below is the meson command to run them.
> +
> +   ``$ meson test --suite DPDK:dump-tests``
> +
> +
> +
> +Skipped testcases
> +-----------------
> +
> +Some unit test cases have dependency on external libraries, driver modules or

"have dependency" -> "have a dependency"

> +config flags, without which the test cases cannot be run. Such test cases
> +should return TEST_SKIPPED when mentioned dependencies are not enabled. To make
> +test cases run user should enable relevant dependencies. Below are the few
> +current scenarios when test cases are skipped:

Needs rewording for a more user-focused point of view, since TEST_SKIPPED
is an internal "magic number" in the code. Something like:
"Such test cases will be reported out as skipped if they cannot run. To
enable those test cases, the user should ensure the required dependencies
are met. Below are a few possible causes why tests may be skipped and how
they may be resolved:"

> +
> +#. External library dependency paths are not set.

The high-level problem is more that the library is not found, not that the
path is unset. The solution is to set the path to the dependency if
necessary.

> +#. Config flag for the dependent library is not enabled.
> +#. Dependent driver modules are not installed on the system.
> +
> +Dependent library paths can be set using below

"To help find missing libraries, the user can specify addition search paths
for those libraries as below:"

> +
> +*  Single path ``$ export LIBRARY_PATH=path``
> +
> +*  Multiple paths ``$ export LIBRARY_PATH=path1:path2:path3``
> +
> +Dependent library headers path can be stated as part of meson build command as below.

"Some functionality may be disabled due to library headers being missed as part of
the build. To specify an addition search path for headers at configuration
time, use one of the commands below:"

> +
> +*  Single path ``$ CFLAGS=-I/path meson build``
> +
> +*  Multiple paths ``$ CFLAGS=-I/path1 -I/path2 meson build``
> +
> +Below examples shows how to export libraries and their header paths.
> +
> +To specify single library at a time.
> +
> +        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/``
> +        ``$ CFLAGS=-I/root/wireless_libs/zuc/include meson build``
> +
> +To specify multiple libraries at a time.
> +
> +        ``$ export LIBRARY_PATH=/root/wireless_libs/zuc/:/root/wireless_libs/`` \
> +                                                    ``libsso_kasumi/build/``
> +        ``$ CFLAGS=-I/root/wireless_libs/zuc/include -I/root/wireless_libs/`` \
> +                                        ``libsso_kasumi/include meson build``
> +
> +
> +
> +Summery of Commands to run meson UTs

"Summary"

> +------------------------------------
> +
> +*   To run all test cases
> +       ``$ meson test``
> +*   To run specific test
> +       ``$ meson test testcase_name``
> +        Ex:``$ meson test acl_autotest``
> +*   To run specific test suite
> +       ``$ meson test --suite DPDK:suite_name``
> +        Ex:``$ meson test --suite DPDK:fast-tests``
> -- 
> 2.17.2
>