RE: [PATCH v5 00/10] dts: add hello world testcase

[Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>]

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Thursday, February 23, 2023 10:29 AM
> To: thomas@monjalon.net; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; lijuan.tu@intel.com;
> bruce.richardson@intel.com; probb@iol.unh.edu
> Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
> Subject: [PATCH v5 00/10] dts: add hello world testcase
> 
> Add code needed to run the HelloWorld testcase which just runs the hello world
> dpdk application.
> 
> The patchset currently heavily refactors this original DTS code needed to run
> the testcase:
> * The whole architecture has been redone into more sensible class
>   hierarchy
> * DPDK build on the System under Test
> * DPDK eal args construction, app running and shutting down
> * Optional SUT hugepage memory configuration
>   The optional part is DTS either configuring them or not. They still must be
> configured even the user doesn't want DTS to do that.
> * Test runner
> * Test results
> * TestSuite class
> * Test runner parts interfacing with TestSuite
> * The HelloWorld testsuite itself
> 
> The code is divided into sub-packages, some of which are divided further.
> 
> There patch may need to be divided into smaller chunks. If so, proposals on
> where exactly to split it would be very helpful.
> 
> v4:
> Made hugepage config optional, users may now specify that in the main config
> file.
> Removed HelloWorld test plan and incorporated parts of it into the test suite
> python file.
> Updated documentation.
> 
> v5:
> Documentation updates about running as root and hugepage configuration.
> Fixed multiple problems with cpu filtering.
> Other minor issues, such as typos and renaming variables.
> 

Hi Juraj,
Everything looks good except for couple of comments/suggestions.
If I’m not mistaken dpdk tarball is copied to the SUT over scp. However, scp is already deprecated [1,2]. Is it possible to use rsync over ssh instead?
Looks like ssh password needs to be stored in the configuration file which is not a good practice. Suggests giving users two options (a) using an ssh key instead of password (b) prompting for user password if no key is provided.
It's somewhat cumbersome for a developer to create a tarball every time they run a test case. Therefore, would it be possible to automate the creation of tarball from a git repo + branch or a local directory when user doesn’t provide a tarball?

[1] https://lwn.net/Articles/835962/
[2] https://www.redhat.com/en/blog/openssh-scp-deprecation-rhel-9-what-you-need-know

> Juraj Linkeš (10):
>   dts: add node and os abstractions
>   dts: add ssh command verification
>   dts: add dpdk build on sut
>   dts: add dpdk execution handling
>   dts: add node memory setup
>   dts: add test suite module
>   dts: add hello world testsuite
>   dts: add test suite config and runner
>   dts: add test results module
>   doc: update DTS setup and test suite cookbook
> 
>  doc/guides/tools/dts.rst                      | 165 ++++++++-
>  dts/conf.yaml                                 |  22 +-
>  dts/framework/config/__init__.py              | 130 ++++++-
>  dts/framework/config/conf_yaml_schema.json    | 172 +++++++++-
>  dts/framework/dts.py                          | 185 ++++++++--
>  dts/framework/exception.py                    | 100 +++++-
>  dts/framework/logger.py                       |  24 +-
>  dts/framework/remote_session/__init__.py      |  30 +-
>  dts/framework/remote_session/linux_session.py | 107 ++++++
>  dts/framework/remote_session/os_session.py    | 175 ++++++++++
>  dts/framework/remote_session/posix_session.py | 222 ++++++++++++
>  .../remote_session/remote/__init__.py         |  16 +
>  .../remote_session/remote/remote_session.py   | 155 +++++++++
>  .../{ => remote}/ssh_session.py               |  92 ++++-
>  .../remote_session/remote_session.py          |  95 ------
>  dts/framework/settings.py                     |  81 ++++-
>  dts/framework/test_result.py                  | 316 ++++++++++++++++++
>  dts/framework/test_suite.py                   | 254 ++++++++++++++
>  dts/framework/testbed_model/__init__.py       |  20 +-
>  dts/framework/testbed_model/dpdk.py           |  78 +++++
>  dts/framework/testbed_model/hw/__init__.py    |  27 ++
>  dts/framework/testbed_model/hw/cpu.py         | 274 +++++++++++++++
>  .../testbed_model/hw/virtual_device.py        |  16 +
>  dts/framework/testbed_model/node.py           | 159 +++++++--
>  dts/framework/testbed_model/sut_node.py       | 260 ++++++++++++++
>  dts/framework/utils.py                        |  39 ++-
>  dts/tests/TestSuite_hello_world.py            |  64 ++++
>  27 files changed, 3068 insertions(+), 210 deletions(-)  create mode 100644
> dts/framework/remote_session/linux_session.py
>  create mode 100644 dts/framework/remote_session/os_session.py
>  create mode 100644 dts/framework/remote_session/posix_session.py
>  create mode 100644 dts/framework/remote_session/remote/__init__.py
>  create mode 100644
> dts/framework/remote_session/remote/remote_session.py
>  rename dts/framework/remote_session/{ => remote}/ssh_session.py (64%)
> delete mode 100644 dts/framework/remote_session/remote_session.py
>  create mode 100644 dts/framework/test_result.py  create mode 100644
> dts/framework/test_suite.py  create mode 100644
> dts/framework/testbed_model/dpdk.py
>  create mode 100644 dts/framework/testbed_model/hw/__init__.py
>  create mode 100644 dts/framework/testbed_model/hw/cpu.py
>  create mode 100644 dts/framework/testbed_model/hw/virtual_device.py
>  create mode 100644 dts/framework/testbed_model/sut_node.py
>  create mode 100644 dts/tests/TestSuite_hello_world.py
> 
> --
> 2.30.2
>