From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4D55DC433EF for ; Fri, 13 May 2022 14:41:14 +0000 (UTC) Received: from localhost ([::1]:39592 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1npWTh-0005gI-CP for qemu-devel@archiver.kernel.org; Fri, 13 May 2022 10:41:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58324) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1npWQq-0003EC-UT for qemu-devel@nongnu.org; Fri, 13 May 2022 10:38:17 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:49369) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1npWQo-0005Pv-Pm for qemu-devel@nongnu.org; Fri, 13 May 2022 10:38:16 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1652452694; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uklJgan4LFlB8d1G50lxwiJO4Vn6n84S03sKRViRJIM=; b=eGoq0CausgxcG3Q12eZQHOVBqqBJz/cl+t/lDD/9wCi7+31EHGcmodMXB/i4LfxQMu5Frl mhPTItT0AYuNzepRDQ7jdv/mJ2AlNATUFi1Lz85vlVMassBZzMZWUC1+wmVh6yaImzOAkK Ft8NlD8RfpZw7Ubzp4/tS9ZHvV/CvcM= Received: from mail-vk1-f198.google.com (mail-vk1-f198.google.com [209.85.221.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-316-IE5bQeHSNB-oMdU4LEZarw-1; Fri, 13 May 2022 10:38:12 -0400 X-MC-Unique: IE5bQeHSNB-oMdU4LEZarw-1 Received: by mail-vk1-f198.google.com with SMTP id t62-20020a1f5f41000000b00352618e430bso1163222vkb.16 for ; Fri, 13 May 2022 07:38:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uklJgan4LFlB8d1G50lxwiJO4Vn6n84S03sKRViRJIM=; b=rdBsm5kpl5h8MYNSN05vAqTYhs/knGsws+dFvFK14nfG7kv9W70X+vm4fLa5+fVZAe TAAdd04LHOnWNIjDgIZKdoBM/t7XXkv+yiV17j0+18Ra1rrT+mYc/2KMDUmUoC0DGfsG gtKtdtaiV+IlcHTNDSu3l5k/Z+8/j6knUelnHHs1/tVBDMHcidgPR6VdXfzySJlUzP2S AW2KT1FSJB5wAIY28o6T0c9LdMJjaCXlcx77bCV3arvT+cugyNgSZLi27NyGT4sztBJ8 xl7m6FYXWdhbOUVkFcppJ5i/bHhCVXD8hWF2vNZ0d6MATTHPm9ryKyBi1X2t0Cw4ey/1 YnZQ== X-Gm-Message-State: AOAM5315iTnFHAjKYDf1CVn7rwIH9yTE3tjyx5XvpXwGVhRB+kcSCTeM fAvz/gEBMRkc8i8ah0PEkHjhThKLmYvMTzHes5d1FZfC7+gpLNeoisN4jTg+IGkhHcIQAoRLFYg zEun3818iKYuNBFZ/DKXtt9Dj9N2rHrs= X-Received: by 2002:a05:6102:320c:b0:32c:ffc1:ff1c with SMTP id r12-20020a056102320c00b0032cffc1ff1cmr2182529vsf.35.1652452692426; Fri, 13 May 2022 07:38:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxOpYa+4kLVh1wpfRz+XwRZ2cj91aXckLG7x6+THcqUHrY6dryCImJQcTpN36/ENaHJWRiIJqjAZQt6oJAmoII= X-Received: by 2002:a05:6102:320c:b0:32c:ffc1:ff1c with SMTP id r12-20020a056102320c00b0032cffc1ff1cmr2182519vsf.35.1652452692214; Fri, 13 May 2022 07:38:12 -0700 (PDT) MIME-Version: 1.0 References: <20220513000609.197906-1-jsnow@redhat.com> <20220513000609.197906-10-jsnow@redhat.com> In-Reply-To: From: John Snow Date: Fri, 13 May 2022 10:38:01 -0400 Message-ID: Subject: Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests To: Paolo Bonzini Cc: qemu-devel , Qemu-block , Cleber Rosa , =?UTF-8?B?QWxleCBCZW5uw6ll?= , Hanna Reitz , Thomas Huth , Daniel Berrange , Kevin Wolf , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Wainer dos Santos Moschetta , Beraldo Leal Content-Type: multipart/alternative; boundary="000000000000ba5abc05dee59fd3" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000ba5abc05dee59fd3 Content-Type: text/plain; charset="UTF-8" On Fri, May 13, 2022, 4:38 AM Paolo Bonzini wrote: > On 5/13/22 02:06, John Snow wrote: > > Essentially, this: > > > > (A) adjusts the python binary to be the one found in the venv (which is > > a symlink to the python binary chosen at configure time) > > > > (B) adds a new VIRTUAL_ENV export variable > > > > (C) changes PATH to front-load the venv binary directory. > > (amending my commit message/rfc notes while I'm here:) I'll add that this way of entering a venv is more complex than the method used for VM tests and Avocado tests because it allows the possibility of shell tests (et al) invoking python utilities and having those be "in the venv" as well. i.e. it's more rigorous and it matches the behavior of the "activate" shell script bundled with every venv. > > If the venv directory isn't found, raise a friendly exception that tries > > to give the human operator a friendly clue as to what's gone wrong. In > > the very near future, I'd like to teach iotests how to fix this problem > > entirely of its own volition, but that's a trick for a little later. > > > > Signed-off-by: John Snow > > --- > > tests/qemu-iotests/testenv.py | 24 +++++++++++++++++------- > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > diff --git a/tests/qemu-iotests/testenv.py > b/tests/qemu-iotests/testenv.py > > index 0007da3f06c..fd3720ed7e7 100644 > > --- a/tests/qemu-iotests/testenv.py > > +++ b/tests/qemu-iotests/testenv.py > > @@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']): > > # lot of them. Silence pylint: > > # pylint: disable=too-many-instance-attributes > > > > - env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', > 'SAMPLE_IMG_DIR', > > - 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG', > > + env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON', > > + 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR', > > + 'QEMU_PROG', 'QEMU_IMG_PROG', > > 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG', > > 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS', > > 'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT', > > @@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]: > > if val is not None: > > env[v] = val > > > > + env['PATH'] = os.pathsep.join(( > > + os.path.join(self.virtual_env, 'bin'), > > + os.environ['PATH'] > > + )) > > return env > > > > def init_directories(self) -> None: > > @@ -107,13 +112,17 @@ def init_directories(self) -> None: > > SOCK_DIR > > SAMPLE_IMG_DIR > > """ > > - > > - # Path where qemu goodies live in this source tree. > > - qemu_srctree_path = Path(__file__, '../../../python').resolve() > > + venv_path = Path(self.build_root, 'tests/venv/') > > + if not venv_path.exists(): > > + raise FileNotFoundError( > > + f"Virtual environment \"{venv_path!s}\" isn't found." > > + " (Maybe you need to run 'make check-venv'" > > + " from the build dir?)" > > + ) > > + self.virtual_env: str = str(venv_path) > > > > self.pythonpath = os.pathsep.join(filter(None, ( > > self.source_iotests, > > - str(qemu_srctree_path), > > os.getenv('PYTHONPATH'), > > ))) > > > > @@ -138,7 +147,7 @@ def init_binaries(self) -> None: > > PYTHON (for bash tests) > > QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, > QSD_PROG > > """ > > - self.python = sys.executable > > + self.python: str = os.path.join(self.virtual_env, 'bin', > 'python3') > > Is this guaranteed even if, say, only a /usr/bin/python3.9 exists? > os.path.basename(sys.executable) might be more weirdness-proof than > 'python3'. > > Paolo > It *should*, because "#!/usr/bin/env python3" is the preferred shebang for Python scripts. https://peps.python.org/pep-0394/ 'python3' "should" be available. 'python' may not be. Probably the "python" name in Makefile for TESTS_PYTHON should actually be "python3" as well. In practice, all permutations (python, python3, python3.9, etc.) are symlinks* to the binary used to create the venv. Which links are present may be site configurable, but pep394 should guarantee that python3 is always available. (* not on Windows, where it'll be a copy.) > --000000000000ba5abc05dee59fd3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Fri, May 13, 2022, 4:38 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
On 5/13/22 02:06, John Snow wrote:
> Essentially, this:
>
> (A) adjusts the python binary to be the one found in the venv (which i= s
> a symlink to the python binary chosen at configure time)
>
> (B) adds a new VIRTUAL_ENV export variable
>
> (C) changes PATH to front-load the venv binary directory.
>

= (amending my commit message/rfc notes while I'm here:)

I'll add that this way of entering a= venv is more complex than the method used for VM tests and Avocado tests b= ecause it allows the possibility of shell tests (et al) invoking python uti= lities and having those be "in the venv" as well.

i.e. it's more rigorous and it ma= tches the behavior of the "activate" shell script bundled with ev= ery venv.


> If the venv directory isn't found, raise a friendly exception that= tries
> to give the human operator a friendly clue as to what's gone wrong= . In
> the very near future, I'd like to teach iotests how to fix this pr= oblem
> entirely of its own volition, but that's a trick for a little late= r.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>=C2=A0 =C2=A0tests/qemu-iotests/testenv.py | 24 +++++++++++++++++------= -
>=C2=A0 =C2=A01 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testen= v.py
> index 0007da3f06c..fd3720ed7e7 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']): >=C2=A0 =C2=A0 =C2=A0 =C2=A0# lot of them. Silence pylint:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0# pylint: disable=3Dtoo-many-instance-attrib= utes
>=C2=A0 =C2=A0
> -=C2=A0 =C2=A0 env_variables =3D ['PYTHONPATH', 'TEST_DIR&= #39;, 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> +=C2=A0 =C2=A0 env_variables =3D ['PYTHONPATH', 'VIRTUAL_E= NV', 'PYTHON',
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR', > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0'QEMU_PROG', 'QEMU_IMG_PROG',
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PRO= G',
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',<= br> > @@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if val is not No= ne:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0en= v[v] =3D val
>=C2=A0 =C2=A0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 env['PATH'] =3D os.pathsep.join((=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 os.path.join(self.virtual_e= nv, 'bin'),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 os.environ['PATH']<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return env
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0def init_directories(self) -> None:
> @@ -107,13 +112,17 @@ def init_directories(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SOCK_DIR
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SAMPLE_IMG_DIR<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"""
> -
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Path where qemu goodies live in this so= urce tree.
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_srctree_path =3D Path(__file__, '= ;../../../python').resolve()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 venv_path =3D Path(self.build_root, '= tests/venv/')
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if not venv_path.exists():
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise FileNotFoundError( > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 f"Virtua= l environment \"{venv_path!s}\" isn't found."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 " (Maybe= you need to run 'make check-venv'"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 " from t= he build dir?)"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.virtual_env: str =3D str(venv_path)<= br> >=C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.pythonpath =3D os.pathsep= .join(filter(None, (
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.source_iote= sts,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 str(qemu_srctree_path),
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0os.getenv('P= YTHONPATH'),
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0)))
>=C2=A0 =C2=A0
> @@ -138,7 +147,7 @@ def init_binaries(self) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 PYTHON (for bas= h tests)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 QEMU_PROG, QEMU= _IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"""
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.python =3D sys.executable
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.python: str =3D os.path.join(self.vi= rtual_env, 'bin', 'python3')

Is this guaranteed even if, say, only a /usr/bin/python3.9 exists?
os.path.basename(sys.executable) might be more weirdness-proof than
'python3'.

Paolo

It *should*, because "#!/usr/bin/env python3" is the preferr= ed shebang for Python scripts.


'= ;python3' "should" be available. 'python' may not be.=

Probably the "pyth= on" name in Makefile for TESTS_PYTHON should actually be "python3= " as well. In practice, all permutations (python, python3, python3.9, = etc.)=C2=A0are symlinks* to the binary used to create the venv. Which links= are present may be site configurable, but pep394 should guarantee that pyt= hon3 is always available.

(* not on Windows, where it'll be a copy.)
--000000000000ba5abc05dee59fd3--