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

[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 <jsnow@redhat.com>

Hurrah!!

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

> ---
>   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