autotest: fix for pure python3 environments

Message ID 20200507072629.2374881-1-christian.ehrhardt@canonical.com (mailing list archive)
State Not Applicable, archived
Delegated to: David Marchand
Headers
Series autotest: fix for pure python3 environments |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/Intel-compilation fail Compilation issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Christian Ehrhardt May 7, 2020, 7:26 a.m. UTC
  Without this fix in a pure python3 environment this will run into
issues like:
  ModuleNotFoundError: No module named 'StringIO'
or later string encoding issues on check_output.

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 app/test/autotest_runner.py | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
  

Comments

Luca Boccassi May 7, 2020, 9:36 a.m. UTC | #1
On Thu, 2020-05-07 at 09:26 +0200, Christian Ehrhardt wrote:
> Without this fix in a pure python3 environment this will run into
> issues like:
>   ModuleNotFoundError: No module named 'StringIO'
> or later string encoding issues on check_output.
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  app/test/autotest_runner.py | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)

Acked-by: Luca Boccassi <bluca@debian.org>

Note that this makes the file python3-only (as far as I can see) - I
think it's fine at this point, given python2 is EOL?
  
Robin Jarry May 7, 2020, 9:43 a.m. UTC | #2
Hi Christian,

2020-05-07, Christian Ehrhardt:
> Without this fix in a pure python3 environment this will run into
> issues like:
>   ModuleNotFoundError: No module named 'StringIO'
> or later string encoding issues on check_output.
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>


> @@ -45,11 +45,9 @@ def get_numa_nodes():
>  def first_cpu_on_node(node_nr):
>      cpu_path = glob.glob("/sys/devices/system/node/node%d/cpu*" % node_nr)
>      r = re.compile(r"cpu(\d+)")
> -    cpu_name = filter(None,
> -            map(r.match,
> -                map(os.path.basename, cpu_path)
> -            )
> -    )
> +    cpu_name = [_f for _f in map(r.match,
> +                list(map(os.path.basename, cpu_path))
> +            ) if _f]

Since filter() is not available on Python 3, maybe this function could
be made a little simpler:

def first_cpu_on_node(node_nr):
    for fname in os.listdir("/sys/devices/system/node/node%d" % node_nr):
        match = re.match(r"cpu(\d+)", fname)
        if match:
            return int(match.group(1))
    raise ValueError("no cpu on node %d" % node_nr)

The rest looks good to me.
  
Bruce Richardson May 7, 2020, 11:25 a.m. UTC | #3
On Thu, May 07, 2020 at 10:36:35AM +0100, Luca Boccassi wrote:
> On Thu, 2020-05-07 at 09:26 +0200, Christian Ehrhardt wrote:
> > Without this fix in a pure python3 environment this will run into
> > issues like:
> >   ModuleNotFoundError: No module named 'StringIO'
> > or later string encoding issues on check_output.
> > 
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > ---
> >  app/test/autotest_runner.py | 16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> Acked-by: Luca Boccassi <bluca@debian.org>
> 
> Note that this makes the file python3-only (as far as I can see) - I
> think it's fine at this point, given python2 is EOL?
> 
I think for 20.11 we should aim to have all python in DPDK python3 only.

/Bruce
  
Christian Ehrhardt June 4, 2020, 2:43 p.m. UTC | #4
On Thu, May 7, 2020 at 1:25 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, May 07, 2020 at 10:36:35AM +0100, Luca Boccassi wrote:
> > On Thu, 2020-05-07 at 09:26 +0200, Christian Ehrhardt wrote:
> > > Without this fix in a pure python3 environment this will run into
> > > issues like:
> > >   ModuleNotFoundError: No module named 'StringIO'
> > > or later string encoding issues on check_output.
> > >
> > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> > > ---
> > >  app/test/autotest_runner.py | 16 +++++++---------
> > >  1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > Acked-by: Luca Boccassi <bluca@debian.org>
> >
> > Note that this makes the file python3-only (as far as I can see) - I
> > think it's fine at this point, given python2 is EOL?
> >
> I think for 20.11 we should aim to have all python in DPDK python3 only.
>

IMHO Robin's feeback wasn't forcing a mandatory v2 (I wa not planning on
one).
Nothing else happened on this, since we failed to hit 20.05 I wanted to
ping again on this.


> /Bruce
>
  
Bruce Richardson June 4, 2020, 4:22 p.m. UTC | #5
On Thu, Jun 04, 2020 at 04:43:50PM +0200, Christian Ehrhardt wrote:
>    On Thu, May 7, 2020 at 1:25 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Thu, May 07, 2020 at 10:36:35AM +0100, Luca Boccassi wrote:
>      > On Thu, 2020-05-07 at 09:26 +0200, Christian Ehrhardt wrote:
>      > > Without this fix in a pure python3 environment this will run
>      into
>      > > issues like:
>      > >   ModuleNotFoundError: No module named 'StringIO'
>      > > or later string encoding issues on check_output.
>      > >
>      > > Signed-off-by: Christian Ehrhardt
>      <[2]christian.ehrhardt@canonical.com>
>      > > ---
>      > >  app/test/autotest_runner.py | 16 +++++++---------
>      > >  1 file changed, 7 insertions(+), 9 deletions(-)
>      >
>      > Acked-by: Luca Boccassi <[3]bluca@debian.org>
>      >
>      > Note that this makes the file python3-only (as far as I can see) -
>      I
>      > think it's fine at this point, given python2 is EOL?
>      >
>      I think for 20.11 we should aim to have all python in DPDK python3
>      only.
> 
>    IMHO Robin's feeback wasn't forcing a mandatory v2 (I wa not planning
>    on one).
>    Nothing else happened on this, since we failed to hit 20.05 I wanted to
>    ping again on this.
>
Just to note that similar changes/fixes are included in set:
http://patches.dpdk.org/project/dpdk/list/?series=10204
  
Bruce Richardson June 4, 2020, 4:25 p.m. UTC | #6
On Thu, May 07, 2020 at 09:26:29AM +0200, Christian Ehrhardt wrote:
> Without this fix in a pure python3 environment this will run into
> issues like:
>   ModuleNotFoundError: No module named 'StringIO'
> or later string encoding issues on check_output.
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  app/test/autotest_runner.py | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/app/test/autotest_runner.py b/app/test/autotest_runner.py
> index 95e74c760d..dfa5f2b2dd 100644
> --- a/app/test/autotest_runner.py
> +++ b/app/test/autotest_runner.py
> @@ -4,7 +4,7 @@
>  # The main logic behind running autotests in parallel
>  
>  from __future__ import print_function
> -import StringIO
> +import io
>  import csv
>  from multiprocessing import Pool, Queue
>  import pexpect
> @@ -45,11 +45,9 @@ def get_numa_nodes():
>  def first_cpu_on_node(node_nr):
>      cpu_path = glob.glob("/sys/devices/system/node/node%d/cpu*" % node_nr)
>      r = re.compile(r"cpu(\d+)")
> -    cpu_name = filter(None,
> -            map(r.match,
> -                map(os.path.basename, cpu_path)
> -            )
> -    )
> +    cpu_name = [_f for _f in map(r.match,
> +                list(map(os.path.basename, cpu_path))
> +            ) if _f]
>      # for compatibility between python 3 and 2 we need to make interable out
>      # of filter return as it returns list in python 2 and a generator in 3
>      m = next(iter(cpu_name))

If this is python3 only, then you can remove the "iter()" call above and
the comment about the python2 compatibility.

/Bruce
  

Patch

diff --git a/app/test/autotest_runner.py b/app/test/autotest_runner.py
index 95e74c760d..dfa5f2b2dd 100644
--- a/app/test/autotest_runner.py
+++ b/app/test/autotest_runner.py
@@ -4,7 +4,7 @@ 
 # The main logic behind running autotests in parallel
 
 from __future__ import print_function
-import StringIO
+import io
 import csv
 from multiprocessing import Pool, Queue
 import pexpect
@@ -45,11 +45,9 @@  def get_numa_nodes():
 def first_cpu_on_node(node_nr):
     cpu_path = glob.glob("/sys/devices/system/node/node%d/cpu*" % node_nr)
     r = re.compile(r"cpu(\d+)")
-    cpu_name = filter(None,
-            map(r.match,
-                map(os.path.basename, cpu_path)
-            )
-    )
+    cpu_name = [_f for _f in map(r.match,
+                list(map(os.path.basename, cpu_path))
+            ) if _f]
     # for compatibility between python 3 and 2 we need to make interable out
     # of filter return as it returns list in python 2 and a generator in 3
     m = next(iter(cpu_name))
@@ -78,7 +76,7 @@  def pool_init(queue, result_queue):
     cmdline = "%s %s" % (cmdline, prefix_cmdline)
 
     # prepare logging of init
-    startuplog = StringIO.StringIO()
+    startuplog = io.StringIO()
 
     # run test app
     try:
@@ -138,7 +136,7 @@  def run_test(target, test):
     # create log buffer for each test
     # in multiprocessing environment, the logging would be
     # interleaved and will create a mess, hence the buffering
-    logfile = StringIO.StringIO()
+    logfile = io.StringIO()
     pool_child.logfile = logfile
 
     # make a note when the test started
@@ -210,7 +208,7 @@  def __init__(self, cmdline, target, blacklist, whitelist, n_processes):
         # parse the binary for available test commands
         binary = cmdline.split()[0]
         stripped = 'not stripped' not in \
-                   subprocess.check_output(['file', binary])
+                   subprocess.check_output(['file', binary]).decode('utf-8')
         if not stripped:
             symbols = subprocess.check_output(['nm', binary]).decode('utf-8')
             self.avail_cmds = re.findall('test_register_(\w+)', symbols)