diff mbox series

[net-next,v2] selftests: net: py: support verbose printing, display executed commands

Message ID 20240807002445.3833895-1-mohsin.bashr@gmail.com (mailing list archive)
State New
Headers show
Series [net-next,v2] selftests: net: py: support verbose printing, display executed commands | expand

Commit Message

Mohsin Bashir Aug. 7, 2024, 12:24 a.m. UTC
Add verbosity support to show the commands executed while
running tests. Enable verbosity if either an environment
variable 'VERBOSE' is set to a non-zero number or it is defined
in a config file under driver tests as discussed here:
https://github.com/linux-netdev/nipa/wiki/Running-driver-tests.

Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
Changes in v2:
- change verbosity_ctl to set_verbosity
- remove redundency in the code

v1: https://lore.kernel.org/netdev/20240715030723.1768360-1-mohsin.bashr@gmail.com

 .../selftests/drivers/net/lib/py/env.py       |  6 ++++-
 .../testing/selftests/net/lib/py/__init__.py  |  4 +++
 tools/testing/selftests/net/lib/py/utils.py   | 26 +++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

Comments

Petr Machata Aug. 9, 2024, 12:36 p.m. UTC | #1
Mohsin Bashir <mohsin.bashr@gmail.com> writes:

> Add verbosity support to show the commands executed while
> running tests. Enable verbosity if either an environment
> variable 'VERBOSE' is set to a non-zero number or it is defined
> in a config file under driver tests as discussed here:
> https://github.com/linux-netdev/nipa/wiki/Running-driver-tests.
>
> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> ---
> Changes in v2:
> - change verbosity_ctl to set_verbosity
> - remove redundency in the code
>
> v1: https://lore.kernel.org/netdev/20240715030723.1768360-1-mohsin.bashr@gmail.com
>
>  .../selftests/drivers/net/lib/py/env.py       |  6 ++++-
>  .../testing/selftests/net/lib/py/__init__.py  |  4 +++
>  tools/testing/selftests/net/lib/py/utils.py   | 26 +++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
> index 1ea9bb695e94..c7cf52d9b988 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/env.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/env.py
> @@ -5,7 +5,7 @@ import time
>  from pathlib import Path
>  from lib.py import KsftSkipEx, KsftXfailEx
>  from lib.py import ksft_setup
> -from lib.py import cmd, ethtool, ip
> +from lib.py import cmd, ethtool, ip, set_verbosity
>  from lib.py import NetNS, NetdevSimDev
>  from .remote import Remote
>  
> @@ -31,6 +31,10 @@ def _load_env_file(src_path):
>              if len(pair) != 2:
>                  raise Exception("Can't parse configuration line:", full_file)
>              env[pair[0]] = pair[1]
> +
> +    env_level = env.get('VERBOSE')
> +    set_verbosity(env_level)
> +

Actually, the ksft_setup() here was merged last week, and I think that
would be a better place to put this stuff. It already handles
DISRUPTIVE, it should IMHO handle VERBOSE as well.

>      return ksft_setup(env)
>  
>  
> diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
> index b6d498d125fe..eb4860dea26a 100644
> --- a/tools/testing/selftests/net/lib/py/__init__.py
> +++ b/tools/testing/selftests/net/lib/py/__init__.py
> @@ -1,8 +1,12 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> +import os
>  from .consts import KSRC
>  from .ksft import *
>  from .netns import NetNS
>  from .nsim import *
>  from .utils import *
>  from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily
> +
> +env_level = os.environ.get('VERBOSE')
> +set_verbosity(env_level)
> diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
> index 72590c3f90f1..d475f131a598 100644
> --- a/tools/testing/selftests/net/lib/py/utils.py
> +++ b/tools/testing/selftests/net/lib/py/utils.py
> @@ -8,11 +8,35 @@ import socket
>  import subprocess
>  import time
>  
> +VERBOSITY_LEVEL = 0
> +
>  
>  class CmdExitFailure(Exception):
>      pass
>  
>  
> +def set_verbosity(level=None):
> +    global VERBOSITY_LEVEL
> +
> +    if level is not None:
> +        try:
> +            level = int(level)
> +        except ValueError as e:
> +            print(f'Ignoring \'VERBOSE\'. Unknown value \'{level}\'')
> +            level = 0
> +
> +        VERBOSITY_LEVEL = level
> +
> +    return VERBOSITY_LEVEL
> +
> +
> +def verbose(*objs, **kwargs):
> +    global VERBOSITY_LEVEL
> +
> +    if VERBOSITY_LEVEL >= 1:
> +        print(*objs, **kwargs)
> +
> +
>  class cmd:
>      def __init__(self, comm, shell=True, fail=True, ns=None, background=False, host=None, timeout=5):
>          if ns:
> @@ -22,6 +46,8 @@ class cmd:
>          self.stderr = None
>          self.ret = None
>  
> +        verbose("#cmd|", comm)
> +
>          self.comm = comm
>          if host:
>              self.proc = host.cmd(comm)
Jakub Kicinski Aug. 10, 2024, 4:03 a.m. UTC | #2
On Fri, 9 Aug 2024 14:36:17 +0200 Petr Machata wrote:
> > +    env_level = env.get('VERBOSE')
> > +    set_verbosity(env_level)
> > +  
> 
> Actually, the ksft_setup() here was merged last week, and I think that
> would be a better place to put this stuff. It already handles
> DISRUPTIVE, it should IMHO handle VERBOSE as well.

I was wondering about that too, FWIW, but the counter argument is that
VERBOSE has little to do with ksft. It doesn't even include the #
prefix on the list it outputs by itself (unlike ksft_pr() which does).

Maybe we do as you suggest but rename verbose() to ksft_dbg() and
make it act more like ksft_pr()?
Petr Machata Aug. 12, 2024, 10:03 a.m. UTC | #3
Jakub Kicinski <kuba@kernel.org> writes:

> On Fri, 9 Aug 2024 14:36:17 +0200 Petr Machata wrote:
>> > +    env_level = env.get('VERBOSE')
>> > +    set_verbosity(env_level)
>> > +  
>> 
>> Actually, the ksft_setup() here was merged last week, and I think that
>> would be a better place to put this stuff. It already handles
>> DISRUPTIVE, it should IMHO handle VERBOSE as well.
>
> I was wondering about that too, FWIW, but the counter argument is that
> VERBOSE has little to do with ksft. It doesn't even include the #
> prefix on the list it outputs by itself (unlike ksft_pr() which does).
>
> Maybe we do as you suggest but rename verbose() to ksft_dbg() and
> make it act more like ksft_pr()?

That would make sense to me.
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index 1ea9bb695e94..c7cf52d9b988 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -5,7 +5,7 @@  import time
 from pathlib import Path
 from lib.py import KsftSkipEx, KsftXfailEx
 from lib.py import ksft_setup
-from lib.py import cmd, ethtool, ip
+from lib.py import cmd, ethtool, ip, set_verbosity
 from lib.py import NetNS, NetdevSimDev
 from .remote import Remote
 
@@ -31,6 +31,10 @@  def _load_env_file(src_path):
             if len(pair) != 2:
                 raise Exception("Can't parse configuration line:", full_file)
             env[pair[0]] = pair[1]
+
+    env_level = env.get('VERBOSE')
+    set_verbosity(env_level)
+
     return ksft_setup(env)
 
 
diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py
index b6d498d125fe..eb4860dea26a 100644
--- a/tools/testing/selftests/net/lib/py/__init__.py
+++ b/tools/testing/selftests/net/lib/py/__init__.py
@@ -1,8 +1,12 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
+import os
 from .consts import KSRC
 from .ksft import *
 from .netns import NetNS
 from .nsim import *
 from .utils import *
 from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily
+
+env_level = os.environ.get('VERBOSE')
+set_verbosity(env_level)
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 72590c3f90f1..d475f131a598 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -8,11 +8,35 @@  import socket
 import subprocess
 import time
 
+VERBOSITY_LEVEL = 0
+
 
 class CmdExitFailure(Exception):
     pass
 
 
+def set_verbosity(level=None):
+    global VERBOSITY_LEVEL
+
+    if level is not None:
+        try:
+            level = int(level)
+        except ValueError as e:
+            print(f'Ignoring \'VERBOSE\'. Unknown value \'{level}\'')
+            level = 0
+
+        VERBOSITY_LEVEL = level
+
+    return VERBOSITY_LEVEL
+
+
+def verbose(*objs, **kwargs):
+    global VERBOSITY_LEVEL
+
+    if VERBOSITY_LEVEL >= 1:
+        print(*objs, **kwargs)
+
+
 class cmd:
     def __init__(self, comm, shell=True, fail=True, ns=None, background=False, host=None, timeout=5):
         if ns:
@@ -22,6 +46,8 @@  class cmd:
         self.stderr = None
         self.ret = None
 
+        verbose("#cmd|", comm)
+
         self.comm = comm
         if host:
             self.proc = host.cmd(comm)