Re: [PATCH 1/6] iotests: add 'qemu' package location to PYTHONPATH in testenv

2021-09-23 Thread John Snow 
On Thu, Sep 23, 2021 at 11:20 AM Vladimir Sementsov-Ogievskiy <
vsement...@virtuozzo.com> wrote:

> 23.09.2021 03:16, John Snow wrote:
> > We can drop the sys.path hacking in various places by doing
> > this. Additionally, by doing it in one place right up top, we can print
> > interesting warnings in case the environment does not look correct.
> >
> > If we ever decide to change how the environment is crafted, all of the
> > "help me find my python packages" goop is all in one place, right in one
> > function.
> >
> > Signed-off-by: John Snow 
>
> Hurrah!!
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
>
> > ---
> >   tests/qemu-iotests/235|  2 --
> >   tests/qemu-iotests/297|  6 --
> >   tests/qemu-iotests/300|  7 +++
> >   tests/qemu-iotests/iotests.py |  2 --
> >   tests/qemu-iotests/testenv.py | 14 +-
> >   tests/qemu-iotests/tests/mirror-top-perms |  7 +++
> >   6 files changed, 15 insertions(+), 23 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
> > index 8aed45f9a76..4de920c3801 100755
> > --- a/tests/qemu-iotests/235
> > +++ b/tests/qemu-iotests/235
> > @@ -24,8 +24,6 @@ import os
> >   import iotests
> >   from iotests import qemu_img_create, qemu_io, file_path, log
> >
> > -sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
> 'python'))
> > -
> >   from qemu.machine import QEMUMachine
> >
> >   iotests.script_initialize(supported_fmts=['qcow2'])
> > diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
> > index b04cba53667..467b712280e 100755
> > --- a/tests/qemu-iotests/297
> > +++ b/tests/qemu-iotests/297
> > @@ -68,12 +68,6 @@ def run_linters():
> >   # Todo notes are fine, but fixme's or xxx's should probably just be
> >   # fixed (in tests, at least)
> >   env = os.environ.copy()
> > -qemu_module_path = os.path.join(os.path.dirname(__file__),
> > -'..', '..', 'python')
> > -try:
> > -env['PYTHONPATH'] += os.pathsep + qemu_module_path
> > -except KeyError:
> > -env['PYTHONPATH'] = qemu_module_path
> >   subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX',
> *files),
> >  env=env, check=False)
> >
> > diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> > index fe94de84edd..10f9f2a8da6 100755
> > --- a/tests/qemu-iotests/300
> > +++ b/tests/qemu-iotests/300
> > @@ -24,12 +24,11 @@ import random
> >   import re
> >   from typing import Dict, List, Optional
> >
> > -import iotests
> > -
> > -# Import qemu after iotests.py has amended sys.path
> > -# pylint: disable=wrong-import-order
> >   from qemu.machine import machine
> >
> > +import iotests
> > +
> > +
> >   BlockBitmapMapping = List[Dict[str, object]]
> >
> >   mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
> > diff --git a/tests/qemu-iotests/iotests.py
> b/tests/qemu-iotests/iotests.py
> > index ce06cf56304..b06ad76e0c5 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -36,8 +36,6 @@
> >
> >   from contextlib import contextmanager
> >
> > -# pylint: disable=import-error, wrong-import-position
> > -sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
> 'python'))
> >   from qemu.machine import qtest
> >   from qemu.qmp import QMPMessage
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index 70da0d60c80..88104dace90 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -108,12 +108,16 @@ def init_directories(self) -> None:
> >SAMPLE_IMG_DIR
> >OUTPUT_DIR
> >   """
> > +
> > +# Path where qemu goodies live in this source tree.
> > +qemu_srctree_path = Path(__file__, '../../../python').resolve()
> > +
> >   self.pythonpath = os.getenv('PYTHONPATH')
> > -if self.pythonpath:
> > -self.pythonpath = self.source_iotests + os.pathsep + \
> > -self.pythonpath
> > -else:
> > -self.pythonpath = self.source_iotests
> > +self.pythonpath = os.pathsep.join(filter(None, (
> > +self.source_iotests,
> > +str(qemu_srctree_path),
> > +self.pythonpath,
> > +)))
>
> That was simple:)
>
>
Only because a very dedicated engineer from Virtuozzo spent so much time
writing a new iotest launcher ;)


> Hmm, after you this you have
>
> self.pythonpath = ...
> self.pythonpath = something other
>
>
> If have to resend, you may just use os.getenv('PYTHONPATH') inside
> filter(), it seems to be even nicer.
>
>
Ah, yeah. I'll just fold that in. Thanks!


> >
> >   self.test_dir = os.getenv('TEST_DIR',
> > os.path.join(os.getcwd(), 'scratch'))
> > diff --git a/tests/qemu-iotests/tests/mirror-top-perms
> b/tests/qemu-iotests/tests/mirror-to

Re: [PATCH 1/6] iotests: add 'qemu' package location to PYTHONPATH in testenv

2021-09-23 Thread Vladimir Sementsov-Ogievskiy 

23.09.2021 03:16, John Snow wrote:

We can drop the sys.path hacking in various places by doing
this. Additionally, by doing it in one place right up top, we can print
interesting warnings in case the environment does not look correct.

If we ever decide to change how the environment is crafted, all of the
"help me find my python packages" goop is all in one place, right in one
function.

Signed-off-by: John Snow 


Hurrah!!

Reviewed-by: Vladimir Sementsov-Ogievskiy 


---
  tests/qemu-iotests/235|  2 --
  tests/qemu-iotests/297|  6 --
  tests/qemu-iotests/300|  7 +++
  tests/qemu-iotests/iotests.py |  2 --
  tests/qemu-iotests/testenv.py | 14 +-
  tests/qemu-iotests/tests/mirror-top-perms |  7 +++
  6 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index 8aed45f9a76..4de920c3801 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -24,8 +24,6 @@ import os
  import iotests
  from iotests import qemu_img_create, qemu_io, file_path, log
  
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))

-
  from qemu.machine import QEMUMachine
  
  iotests.script_initialize(supported_fmts=['qcow2'])

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b04cba53667..467b712280e 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -68,12 +68,6 @@ def run_linters():
  # Todo notes are fine, but fixme's or xxx's should probably just be
  # fixed (in tests, at least)
  env = os.environ.copy()
-qemu_module_path = os.path.join(os.path.dirname(__file__),
-'..', '..', 'python')
-try:
-env['PYTHONPATH'] += os.pathsep + qemu_module_path
-except KeyError:
-env['PYTHONPATH'] = qemu_module_path
  subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
 env=env, check=False)
  
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300

index fe94de84edd..10f9f2a8da6 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -24,12 +24,11 @@ import random
  import re
  from typing import Dict, List, Optional
  
-import iotests

-
-# Import qemu after iotests.py has amended sys.path
-# pylint: disable=wrong-import-order
  from qemu.machine import machine
  
+import iotests

+
+
  BlockBitmapMapping = List[Dict[str, object]]
  
  mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ce06cf56304..b06ad76e0c5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -36,8 +36,6 @@
  
  from contextlib import contextmanager
  
-# pylint: disable=import-error, wrong-import-position

-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
  from qemu.machine import qtest
  from qemu.qmp import QMPMessage
  
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py

index 70da0d60c80..88104dace90 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -108,12 +108,16 @@ def init_directories(self) -> None:
   SAMPLE_IMG_DIR
   OUTPUT_DIR
  """
+
+# Path where qemu goodies live in this source tree.
+qemu_srctree_path = Path(__file__, '../../../python').resolve()
+
  self.pythonpath = os.getenv('PYTHONPATH')
-if self.pythonpath:
-self.pythonpath = self.source_iotests + os.pathsep + \
-self.pythonpath
-else:
-self.pythonpath = self.source_iotests
+self.pythonpath = os.pathsep.join(filter(None, (
+self.source_iotests,
+str(qemu_srctree_path),
+self.pythonpath,
+)))


That was simple:)

Hmm, after you this you have

self.pythonpath = ...
self.pythonpath = something other


If have to resend, you may just use os.getenv('PYTHONPATH') inside filter(), it 
seems to be even nicer.

  
  self.test_dir = os.getenv('TEST_DIR',

os.path.join(os.getcwd(), 'scratch'))
diff --git a/tests/qemu-iotests/tests/mirror-top-perms 
b/tests/qemu-iotests/tests/mirror-top-perms
index 2fc8dd66e0a..73138a0ef91 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -20,13 +20,12 @@
  #
  
  import os

+
+import qemu
+
  import iotests
  from iotests import qemu_img
  
-# Import qemu after iotests.py has amended sys.path

-# pylint: disable=wrong-import-order
-import qemu
-
  
  image_size = 1 * 1024 * 1024

  source = os.path.join(iotests.test_dir, 'source.img')




--
Best regards,
Vladimir

Re: [PATCH 1/6] iotests: add 'qemu' package location to PYTHONPATH in testenv

2021-09-23 Thread Philippe Mathieu-Daudé 

On 9/23/21 02:16, John Snow wrote:

We can drop the sys.path hacking in various places by doing
this. Additionally, by doing it in one place right up top, we can print
interesting warnings in case the environment does not look correct.

If we ever decide to change how the environment is crafted, all of the
"help me find my python packages" goop is all in one place, right in one
function.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/235|  2 --
  tests/qemu-iotests/297|  6 --
  tests/qemu-iotests/300|  7 +++
  tests/qemu-iotests/iotests.py |  2 --
  tests/qemu-iotests/testenv.py | 14 +-
  tests/qemu-iotests/tests/mirror-top-perms |  7 +++
  6 files changed, 15 insertions(+), 23 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

[PATCH 1/6] iotests: add 'qemu' package location to PYTHONPATH in testenv

2021-09-22 Thread John Snow 
We can drop the sys.path hacking in various places by doing
this. Additionally, by doing it in one place right up top, we can print
interesting warnings in case the environment does not look correct.

If we ever decide to change how the environment is crafted, all of the
"help me find my python packages" goop is all in one place, right in one
function.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/235|  2 --
 tests/qemu-iotests/297|  6 --
 tests/qemu-iotests/300|  7 +++
 tests/qemu-iotests/iotests.py |  2 --
 tests/qemu-iotests/testenv.py | 14 +-
 tests/qemu-iotests/tests/mirror-top-perms |  7 +++
 6 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index 8aed45f9a76..4de920c3801 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -24,8 +24,6 @@ import os
 import iotests
 from iotests import qemu_img_create, qemu_io, file_path, log
 
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-
 from qemu.machine import QEMUMachine
 
 iotests.script_initialize(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b04cba53667..467b712280e 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -68,12 +68,6 @@ def run_linters():
 # Todo notes are fine, but fixme's or xxx's should probably just be
 # fixed (in tests, at least)
 env = os.environ.copy()
-qemu_module_path = os.path.join(os.path.dirname(__file__),
-'..', '..', 'python')
-try:
-env['PYTHONPATH'] += os.pathsep + qemu_module_path
-except KeyError:
-env['PYTHONPATH'] = qemu_module_path
 subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
env=env, check=False)
 
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index fe94de84edd..10f9f2a8da6 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -24,12 +24,11 @@ import random
 import re
 from typing import Dict, List, Optional
 
-import iotests
-
-# Import qemu after iotests.py has amended sys.path
-# pylint: disable=wrong-import-order
 from qemu.machine import machine
 
+import iotests
+
+
 BlockBitmapMapping = List[Dict[str, object]]
 
 mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ce06cf56304..b06ad76e0c5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -36,8 +36,6 @@
 
 from contextlib import contextmanager
 
-# pylint: disable=import-error, wrong-import-position
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import qtest
 from qemu.qmp import QMPMessage
 
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 70da0d60c80..88104dace90 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -108,12 +108,16 @@ def init_directories(self) -> None:
  SAMPLE_IMG_DIR
  OUTPUT_DIR
 """
+
+# Path where qemu goodies live in this source tree.
+qemu_srctree_path = Path(__file__, '../../../python').resolve()
+
 self.pythonpath = os.getenv('PYTHONPATH')
-if self.pythonpath:
-self.pythonpath = self.source_iotests + os.pathsep + \
-self.pythonpath
-else:
-self.pythonpath = self.source_iotests
+self.pythonpath = os.pathsep.join(filter(None, (
+self.source_iotests,
+str(qemu_srctree_path),
+self.pythonpath,
+)))
 
 self.test_dir = os.getenv('TEST_DIR',
   os.path.join(os.getcwd(), 'scratch'))
diff --git a/tests/qemu-iotests/tests/mirror-top-perms 
b/tests/qemu-iotests/tests/mirror-top-perms
index 2fc8dd66e0a..73138a0ef91 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -20,13 +20,12 @@
 #
 
 import os
+
+import qemu
+
 import iotests
 from iotests import qemu_img
 
-# Import qemu after iotests.py has amended sys.path
-# pylint: disable=wrong-import-order
-import qemu
-
 
 image_size = 1 * 1024 * 1024
 source = os.path.join(iotests.test_dir, 'source.img')
-- 
2.31.1