[PATCH v2 0/2] iotests: Require Python 3.6 or later

[PATCH v2 0/2] iotests: Require Python 3.6 or later

[Kevin Wolf]

v2:

- Provide the right exit code from Python instead of having a
  potentially confusing negation in the shell script

- Raised the minimal version to 3.6. If we're going to use a different
  version than QEMU as a whole anyway, we can use a version that suits
  us best. 3.5 would only be for Debian Stretch, and we don't really
  care that much about running Python test cases on it.

- Added a patch to remove Python 2 compatibility code

Kevin Wolf (2):
  iotests: Require Python 3.6 or later
  iotests: Remove Python 2 compatibility code

 tests/qemu-iotests/044                   |  3 ---
 tests/qemu-iotests/163                   |  3 ---
 tests/qemu-iotests/check                 | 13 ++++++++++++-
 tests/qemu-iotests/iotests.py            | 13 +++----------
 tests/qemu-iotests/nbd-fault-injector.py |  7 +++----
 5 files changed, 18 insertions(+), 21 deletions(-)

-- 
2.20.1

[PATCH v2 1/2] iotests: Require Python 3.6 or later

[Kevin Wolf]

Running iotests is not required to build QEMU, so we can have stricter
version requirements for Python here and can make use of new features
and drop compatibility code earlier.

This makes qemu-iotests skip all Python tests if a Python version before
3.6 is used for the build.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/check | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 875399d79f..588c453a94 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -633,6 +633,12 @@ then
     export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
 fi
 
+python_usable=false
+if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
+then
+    python_usable=true
+fi
+
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
 default_alias_machine=$($QEMU_PROG -machine help | \
    sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
@@ -809,7 +815,12 @@ do
         start=$(_wallclock)
 
         if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
-            run_command="$PYTHON $seq"
+            if $python_usable; then
+                run_command="$PYTHON $seq"
+            else
+                run_command="false"
+                echo "Unsupported Python version" > $seq.notrun
+            fi
         else
             run_command="./$seq"
         fi
-- 
2.20.1

[PATCH v2 2/2] iotests: Remove Python 2 compatibility code

[Kevin Wolf]

Some scripts check the Python version number and have two code paths to
accomodate both Python 2 and 3. Remove the code specific to Python 2 and
assert the minimum version of 3.6 instead (check skips Python tests in
this case, so the assertion would only ever trigger if a Python script
is executed manually).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/044                   |  3 ---
 tests/qemu-iotests/163                   |  3 ---
 tests/qemu-iotests/iotests.py            | 13 +++----------
 tests/qemu-iotests/nbd-fault-injector.py |  7 +++----
 4 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
index 05ea1f49c5..8b2afa2a11 100755
--- a/tests/qemu-iotests/044
+++ b/tests/qemu-iotests/044
@@ -28,9 +28,6 @@ import struct
 import subprocess
 import sys
 
-if sys.version_info.major == 2:
-    range = xrange
-
 test_img = os.path.join(iotests.test_dir, 'test.img')
 
 class TestRefcountTableGrowth(iotests.QMPTestCase):
diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
index 081ccc8ac1..d94728e080 100755
--- a/tests/qemu-iotests/163
+++ b/tests/qemu-iotests/163
@@ -21,9 +21,6 @@
 import os, random, iotests, struct, qcow2, sys
 from iotests import qemu_img, qemu_io, image_size
 
-if sys.version_info.major == 2:
-    range = xrange
-
 test_img = os.path.join(iotests.test_dir, 'test.img')
 check_img = os.path.join(iotests.test_dir, 'check.img')
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b26271187c..9fb5181c3d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -35,6 +35,7 @@ from collections import OrderedDict
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qtest
 
+assert sys.version_info >= (3,6)
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
@@ -250,10 +251,7 @@ def image_size(img):
     return json.loads(r)['virtual-size']
 
 def is_str(val):
-    if sys.version_info.major >= 3:
-        return isinstance(val, str)
-    else:
-        return isinstance(val, str) or isinstance(val, unicode)
+    return isinstance(val, str)
 
 test_dir_re = re.compile(r"%s" % test_dir)
 def filter_test_dir(msg):
@@ -935,12 +933,7 @@ def execute_test(test_function=None,
     else:
         # We need to filter out the time taken from the output so that
         # qemu-iotest can reliably diff the results against master output.
-        if sys.version_info.major >= 3:
-            output = io.StringIO()
-        else:
-            # io.StringIO is for unicode strings, which is not what
-            # 2.x's test runner emits.
-            output = io.BytesIO()
+        output = io.StringIO()
 
     logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
 
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index 6b2d659dee..43f095ceef 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -48,10 +48,9 @@ import sys
 import socket
 import struct
 import collections
-if sys.version_info.major >= 3:
-    import configparser
-else:
-    import ConfigParser as configparser
+import configparser
+
+assert sys.version_info >= (3,6)
 
 FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
 
-- 
2.20.1

Re: [PATCH v2 1/2] iotests: Require Python 3.6 or later

[Eduardo Habkost]

On Thu, Sep 19, 2019 at 06:29:04PM +0200, Kevin Wolf wrote:
> Running iotests is not required to build QEMU, so we can have stricter
> version requirements for Python here and can make use of new features
> and drop compatibility code earlier.
> 
> This makes qemu-iotests skip all Python tests if a Python version before
> 3.6 is used for the build.
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH v2 2/2] iotests: Remove Python 2 compatibility code

[Eduardo Habkost]

On Thu, Sep 19, 2019 at 06:29:05PM +0200, Kevin Wolf wrote:
> Some scripts check the Python version number and have two code paths to
> accomodate both Python 2 and 3. Remove the code specific to Python 2 and
> assert the minimum version of 3.6 instead (check skips Python tests in
> this case, so the assertion would only ever trigger if a Python script
> is executed manually).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [PATCH v2 1/2] iotests: Require Python 3.6 or later

[Thomas Huth]

On 19/09/2019 18.29, Kevin Wolf wrote:
> Running iotests is not required to build QEMU, so we can have stricter
> version requirements for Python here and can make use of new features
> and drop compatibility code earlier.
> 
> This makes qemu-iotests skip all Python tests if a Python version before
> 3.6 is used for the build.
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/check | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 875399d79f..588c453a94 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -633,6 +633,12 @@ then
>      export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>  fi
>  
> +python_usable=false
> +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
> +then
> +    python_usable=true
> +fi
> +
>  default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
>  default_alias_machine=$($QEMU_PROG -machine help | \
>     sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
> @@ -809,7 +815,12 @@ do
>          start=$(_wallclock)
>  
>          if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
> -            run_command="$PYTHON $seq"
> +            if $python_usable; then
> +                run_command="$PYTHON $seq"
> +            else
> +                run_command="false"
> +                echo "Unsupported Python version" > $seq.notrun
> +            fi
>          else
>              run_command="./$seq"
>          fi
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 2/2] iotests: Remove Python 2 compatibility code

[Thomas Huth]

On 19/09/2019 18.29, Kevin Wolf wrote:
> Some scripts check the Python version number and have two code paths to
> accomodate both Python 2 and 3. Remove the code specific to Python 2 and
> assert the minimum version of 3.6 instead (check skips Python tests in
> this case, so the assertion would only ever trigger if a Python script
> is executed manually).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/044                   |  3 ---
>  tests/qemu-iotests/163                   |  3 ---
>  tests/qemu-iotests/iotests.py            | 13 +++----------
>  tests/qemu-iotests/nbd-fault-injector.py |  7 +++----
>  4 files changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
> index 05ea1f49c5..8b2afa2a11 100755
> --- a/tests/qemu-iotests/044
> +++ b/tests/qemu-iotests/044
> @@ -28,9 +28,6 @@ import struct
>  import subprocess
>  import sys
>  
> -if sys.version_info.major == 2:
> -    range = xrange
> -
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  
>  class TestRefcountTableGrowth(iotests.QMPTestCase):
> diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
> index 081ccc8ac1..d94728e080 100755
> --- a/tests/qemu-iotests/163
> +++ b/tests/qemu-iotests/163
> @@ -21,9 +21,6 @@
>  import os, random, iotests, struct, qcow2, sys
>  from iotests import qemu_img, qemu_io, image_size
>  
> -if sys.version_info.major == 2:
> -    range = xrange
> -
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  check_img = os.path.join(iotests.test_dir, 'check.img')
>  
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index b26271187c..9fb5181c3d 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -35,6 +35,7 @@ from collections import OrderedDict
>  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>  from qemu import qtest
>  
> +assert sys.version_info >= (3,6)
>  
>  # This will not work if arguments contain spaces but is necessary if we
>  # want to support the override options that ./check supports.
> @@ -250,10 +251,7 @@ def image_size(img):
>      return json.loads(r)['virtual-size']
>  
>  def is_str(val):
> -    if sys.version_info.major >= 3:
> -        return isinstance(val, str)
> -    else:
> -        return isinstance(val, str) or isinstance(val, unicode)
> +    return isinstance(val, str)
>  
>  test_dir_re = re.compile(r"%s" % test_dir)
>  def filter_test_dir(msg):
> @@ -935,12 +933,7 @@ def execute_test(test_function=None,
>      else:
>          # We need to filter out the time taken from the output so that
>          # qemu-iotest can reliably diff the results against master output.
> -        if sys.version_info.major >= 3:
> -            output = io.StringIO()
> -        else:
> -            # io.StringIO is for unicode strings, which is not what
> -            # 2.x's test runner emits.
> -            output = io.BytesIO()
> +        output = io.StringIO()
>  
>      logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>  
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> index 6b2d659dee..43f095ceef 100755
> --- a/tests/qemu-iotests/nbd-fault-injector.py
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -48,10 +48,9 @@ import sys
>  import socket
>  import struct
>  import collections
> -if sys.version_info.major >= 3:
> -    import configparser
> -else:
> -    import ConfigParser as configparser
> +import configparser
> +
> +assert sys.version_info >= (3,6)
>  
>  FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
>  

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 0/2] iotests: Require Python 3.6 or later

[Philippe Mathieu-Daudé]

On 9/19/19 6:29 PM, Kevin Wolf wrote:
> v2:
> 
> - Provide the right exit code from Python instead of having a
>   potentially confusing negation in the shell script
> 
> - Raised the minimal version to 3.6. If we're going to use a different
>   version than QEMU as a whole anyway, we can use a version that suits
>   us best. 3.5 would only be for Debian Stretch, and we don't really
>   care that much about running Python test cases on it.
> 
> - Added a patch to remove Python 2 compatibility code
> 
> Kevin Wolf (2):
>   iotests: Require Python 3.6 or later
>   iotests: Remove Python 2 compatibility code
> 
>  tests/qemu-iotests/044                   |  3 ---
>  tests/qemu-iotests/163                   |  3 ---
>  tests/qemu-iotests/check                 | 13 ++++++++++++-
>  tests/qemu-iotests/iotests.py            | 13 +++----------
>  tests/qemu-iotests/nbd-fault-injector.py |  7 +++----
>  5 files changed, 18 insertions(+), 21 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 2/2] iotests: Remove Python 2 compatibility code

[John Snow]

On 9/19/19 12:29 PM, Kevin Wolf wrote:
> Some scripts check the Python version number and have two code paths to
> accomodate both Python 2 and 3. Remove the code specific to Python 2 and
> assert the minimum version of 3.6 instead (check skips Python tests in
> this case, so the assertion would only ever trigger if a Python script
> is executed manually).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/044                   |  3 ---
>  tests/qemu-iotests/163                   |  3 ---
>  tests/qemu-iotests/iotests.py            | 13 +++----------
>  tests/qemu-iotests/nbd-fault-injector.py |  7 +++----
>  4 files changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
> index 05ea1f49c5..8b2afa2a11 100755
> --- a/tests/qemu-iotests/044
> +++ b/tests/qemu-iotests/044
> @@ -28,9 +28,6 @@ import struct
>  import subprocess
>  import sys
>  
> -if sys.version_info.major == 2:
> -    range = xrange
> -
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  
>  class TestRefcountTableGrowth(iotests.QMPTestCase):
> diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
> index 081ccc8ac1..d94728e080 100755
> --- a/tests/qemu-iotests/163
> +++ b/tests/qemu-iotests/163
> @@ -21,9 +21,6 @@
>  import os, random, iotests, struct, qcow2, sys
>  from iotests import qemu_img, qemu_io, image_size
>  
> -if sys.version_info.major == 2:
> -    range = xrange
> -
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  check_img = os.path.join(iotests.test_dir, 'check.img')
>  
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index b26271187c..9fb5181c3d 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -35,6 +35,7 @@ from collections import OrderedDict
>  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>  from qemu import qtest
>  
> +assert sys.version_info >= (3,6)
>  
>  # This will not work if arguments contain spaces but is necessary if we
>  # want to support the override options that ./check supports.
> @@ -250,10 +251,7 @@ def image_size(img):
>      return json.loads(r)['virtual-size']
>  
>  def is_str(val):
> -    if sys.version_info.major >= 3:
> -        return isinstance(val, str)
> -    else:
> -        return isinstance(val, str) or isinstance(val, unicode)
> +    return isinstance(val, str)
>  
>  test_dir_re = re.compile(r"%s" % test_dir)
>  def filter_test_dir(msg):
> @@ -935,12 +933,7 @@ def execute_test(test_function=None,
>      else:
>          # We need to filter out the time taken from the output so that
>          # qemu-iotest can reliably diff the results against master output.
> -        if sys.version_info.major >= 3:
> -            output = io.StringIO()
> -        else:
> -            # io.StringIO is for unicode strings, which is not what
> -            # 2.x's test runner emits.
> -            output = io.BytesIO()
> +        output = io.StringIO()
>  
>      logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>  
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> index 6b2d659dee..43f095ceef 100755
> --- a/tests/qemu-iotests/nbd-fault-injector.py
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -48,10 +48,9 @@ import sys
>  import socket
>  import struct
>  import collections
> -if sys.version_info.major >= 3:
> -    import configparser
> -else:
> -    import ConfigParser as configparser
> +import configparser
> +
> +assert sys.version_info >= (3,6)
>  
>  FAKE_DISK_SIZE = 8 * 1024 * 1024 * 1024 # 8 GB
>  
> 

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [PATCH v2 1/2] iotests: Require Python 3.6 or later

[Vladimir Sementsov-Ogievskiy]

19.09.2019 19:29, Kevin Wolf wrote:
> Running iotests is not required to build QEMU, so we can have stricter
> version requirements for Python here and can make use of new features
> and drop compatibility code earlier.
> 
> This makes qemu-iotests skip all Python tests if a Python version before
> 3.6 is used for the build.
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/check | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 875399d79f..588c453a94 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -633,6 +633,12 @@ then
>       export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>   fi
>   
> +python_usable=false
> +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
> +then
> +    python_usable=true
> +fi
> +
>   default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
>   default_alias_machine=$($QEMU_PROG -machine help | \
>      sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
> @@ -809,7 +815,12 @@ do
>           start=$(_wallclock)
>   
>           if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
> -            run_command="$PYTHON $seq"
> +            if $python_usable; then

hmm I wanted to say that this should not work, as python_usable is a string. But I checked
and see - it's work. Wow. Googled. And now I understand that here "false" or "true" command
is called, to obtain it's return value.. I don't like bash and don't know its best practice,
but I'd prefer python_usable to be just return value of your python command, like

above:

   $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
   python_usable=$?

and here:

   if [ $python_usable -eq 0 ]; then


> +                run_command="$PYTHON $seq"
> +            else
> +                run_command="false"
> +                echo "Unsupported Python version" > $seq.notrun
> +            fi
>           else
>               run_command="./$seq"
>           fi
> 


-- 
Best regards,
Vladimir

Re: [PATCH v2 2/2] iotests: Remove Python 2 compatibility code

[Vladimir Sementsov-Ogievskiy]

19.09.2019 19:29, Kevin Wolf wrote:
> Some scripts check the Python version number and have two code paths to
> accomodate both Python 2 and 3. Remove the code specific to Python 2 and
> assert the minimum version of 3.6 instead (check skips Python tests in
> this case, so the assertion would only ever trigger if a Python script
> is executed manually).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir

Re: [PATCH v2 1/2] iotests: Require Python 3.6 or later

[Kevin Wolf]

Am 20.09.2019 um 10:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 19.09.2019 19:29, Kevin Wolf wrote:
> > Running iotests is not required to build QEMU, so we can have stricter
> > version requirements for Python here and can make use of new features
> > and drop compatibility code earlier.
> > 
> > This makes qemu-iotests skip all Python tests if a Python version before
> > 3.6 is used for the build.
> > 
> > Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   tests/qemu-iotests/check | 13 ++++++++++++-
> >   1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> > index 875399d79f..588c453a94 100755
> > --- a/tests/qemu-iotests/check
> > +++ b/tests/qemu-iotests/check
> > @@ -633,6 +633,12 @@ then
> >       export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
> >   fi
> >   
> > +python_usable=false
> > +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
> > +then
> > +    python_usable=true
> > +fi
> > +
> >   default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
> >   default_alias_machine=$($QEMU_PROG -machine help | \
> >      sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
> > @@ -809,7 +815,12 @@ do
> >           start=$(_wallclock)
> >   
> >           if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
> > -            run_command="$PYTHON $seq"
> > +            if $python_usable; then
> 
> hmm I wanted to say that this should not work, as python_usable is a
> string. But I checked and see - it's work. Wow. Googled. And now I
> understand that here "false" or "true" command is called, to obtain
> it's return value.. I don't like bash and don't know its best
> practice, but I'd prefer python_usable to be just return value of your
> python command, like
> 
> above:
> 
>    $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
>    python_usable=$?
> 
> and here:
> 
>    if [ $python_usable -eq 0 ]; then

I'm just trying to be consistent with other variables used in the
'check' script. It has many boolean variables and they all end up
calling the true/false commands.

And actually I think the version used is easier to read, even if maybe
not as easy to understand why exactly it works.

Kevin

Re: [PATCH v2 1/2] iotests: Require Python 3.6 or later

[Vladimir Sementsov-Ogievskiy]

20.09.2019 12:27, Kevin Wolf wrote:
> Am 20.09.2019 um 10:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 19.09.2019 19:29, Kevin Wolf wrote:
>>> Running iotests is not required to build QEMU, so we can have stricter
>>> version requirements for Python here and can make use of new features
>>> and drop compatibility code earlier.
>>>
>>> This makes qemu-iotests skip all Python tests if a Python version before
>>> 3.6 is used for the build.
>>>
>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>    tests/qemu-iotests/check | 13 ++++++++++++-
>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>>> index 875399d79f..588c453a94 100755
>>> --- a/tests/qemu-iotests/check
>>> +++ b/tests/qemu-iotests/check
>>> @@ -633,6 +633,12 @@ then
>>>        export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>>>    fi
>>>    
>>> +python_usable=false
>>> +if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
>>> +then
>>> +    python_usable=true
>>> +fi

don't we want
else
    PYTHON="false"
fi

to prevent some occasional unchecked call below?


>>> +
>>>    default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
>>>    default_alias_machine=$($QEMU_PROG -machine help | \
>>>       sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
>>> @@ -809,7 +815,12 @@ do
>>>            start=$(_wallclock)
>>>    
>>>            if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
>>> -            run_command="$PYTHON $seq"
>>> +            if $python_usable; then
>>
>> hmm I wanted to say that this should not work, as python_usable is a
>> string. But I checked and see - it's work. Wow. Googled. And now I
>> understand that here "false" or "true" command is called, to obtain
>> it's return value.. I don't like bash and don't know its best
>> practice, but I'd prefer python_usable to be just return value of your
>> python command, like
>>
>> above:
>>
>>     $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
>>     python_usable=$?
>>
>> and here:
>>
>>     if [ $python_usable -eq 0 ]; then
> 
> I'm just trying to be consistent with other variables used in the
> 'check' script. It has many boolean variables and they all end up
> calling the true/false commands.

Missed this, sorry. Then OK, of course..

still,
    echo "Unsupported Python version (must be >= 3.6)" > $seq.notrun
may be a bit more useful, if someone see it.


With or without any of suggestions:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>



-- 
Best regards,
Vladimir