[v1,01/11] test/bbdev: fix python script subprocess

Message ID 20230929181328.104311-2-hernan.vargas@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series test-bbdev changes for 23.11 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Hernan Vargas Sept. 29, 2023, 6:13 p.m. UTC
  test-bbdev.py relying on non-recommended subprocess Popen.
This can lead to unstability where the process cannot be stopped with a
sig TERM.
Use subprocess run with proper timeout argument.

Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 app/test-bbdev/test-bbdev.py | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)
  

Comments

Maxime Coquelin Oct. 17, 2023, 7:05 p.m. UTC | #1
On 9/29/23 20:13, Hernan Vargas wrote:
> test-bbdev.py relying on non-recommended subprocess Popen.
> This can lead to unstability where the process cannot be stopped with a

s/unstability/instabilities/

> sig TERM.
> Use subprocess run with proper timeout argument.
> 
> Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   app/test-bbdev/test-bbdev.py | 29 +++++++++++++----------------
>   1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/app/test-bbdev/test-bbdev.py b/app/test-bbdev/test-bbdev.py
> index 291c80b0f54f..9cdb4659724d 100755
> --- a/app/test-bbdev/test-bbdev.py
> +++ b/app/test-bbdev/test-bbdev.py
> @@ -91,21 +91,18 @@ def kill(process):
>           params_string = " ".join(call_params)
>   
>           print("Executing: {}".format(params_string))
> -        app_proc = subprocess.Popen(call_params)
> -        if args.timeout > 0:
> -            timer = Timer(args.timeout, kill, [app_proc])
> -            timer.start()
> -
>           try:
> -            app_proc.communicate()
> -        except:
> -            print("Error: failed to execute: {}".format(params_string))
> -        finally:
> -            timer.cancel()
> -
> -        if app_proc.returncode != 0:
> -            exit_status = 1
> -            print("ERROR TestCase failed. Failed test for vector {}. Return code: {}".format(
> -                vector, app_proc.returncode))
> -
> +          output = subprocess.run(call_params, timeout=args.timeout, universal_newlines=True)
> +        except subprocess.TimeoutExpired as e:
> +          print("Starting Test Suite : BBdev TimeOut Tests")

The indentation does not seem consistent with the code it replaces, is
it?

> +          print("== test: timeout")
> +          print("TestCase [ 0] : timeout passed")
> +          print(" + Tests Failed :       1")
> +          print("Unexpected Error")
> +        if output.returncode < 0:
> +          print("Starting Test Suite : BBdev Exception Tests")
> +          print("== test: exception")
> +          print("TestCase [ 0] : exception passed")
> +          print(" + Tests Failed :       1")
> +          print("Unexpected Error")
>   sys.exit(exit_status)
  

Patch

diff --git a/app/test-bbdev/test-bbdev.py b/app/test-bbdev/test-bbdev.py
index 291c80b0f54f..9cdb4659724d 100755
--- a/app/test-bbdev/test-bbdev.py
+++ b/app/test-bbdev/test-bbdev.py
@@ -91,21 +91,18 @@  def kill(process):
         params_string = " ".join(call_params)
 
         print("Executing: {}".format(params_string))
-        app_proc = subprocess.Popen(call_params)
-        if args.timeout > 0:
-            timer = Timer(args.timeout, kill, [app_proc])
-            timer.start()
-
         try:
-            app_proc.communicate()
-        except:
-            print("Error: failed to execute: {}".format(params_string))
-        finally:
-            timer.cancel()
-
-        if app_proc.returncode != 0:
-            exit_status = 1
-            print("ERROR TestCase failed. Failed test for vector {}. Return code: {}".format(
-                vector, app_proc.returncode))
-
+          output = subprocess.run(call_params, timeout=args.timeout, universal_newlines=True)
+        except subprocess.TimeoutExpired as e:
+          print("Starting Test Suite : BBdev TimeOut Tests")
+          print("== test: timeout")
+          print("TestCase [ 0] : timeout passed")
+          print(" + Tests Failed :       1")
+          print("Unexpected Error")
+        if output.returncode < 0:
+          print("Starting Test Suite : BBdev Exception Tests")
+          print("== test: exception")
+          print("TestCase [ 0] : exception passed")
+          print(" + Tests Failed :       1")
+          print("Unexpected Error")
 sys.exit(exit_status)