[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the syst
From: |
Cleber Rosa |
Subject: |
Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH |
Date: |
Fri, 15 Nov 2019 18:17:35 -0500 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Tue, Nov 12, 2019 at 12:00:20PM -0200, Wainer dos Santos Moschetta wrote:
>
> On 11/11/19 8:49 PM, Cleber Rosa wrote:
> > On Thu, Nov 07, 2019 at 05:46:13PM -0200, Wainer dos Santos Moschetta wrote:
> > > On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > > > So that when binaries such as qemu-img are searched for, those in the
> > > > build tree will be favored. As a clarification, SRC_ROOT_DIR is
> > > > dependent on the location from where tests are executed, so they are
> > > > equal to the build directory if one is being used.
> > > >
> > > > The original motivation is that Avocado libraries such as
> > > > avocado.utils.vmimage.get() may use the matching binaries, but it may
> > > > also apply to any other binary that test code may eventually attempt
> > > > to execute.
> > > >
> > > > Signed-off-by: Cleber Rosa <address@hidden>
> > > > ---
> > > > tests/acceptance/avocado_qemu/__init__.py | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/tests/acceptance/avocado_qemu/__init__.py
> > > > b/tests/acceptance/avocado_qemu/__init__.py
> > > > index 17ce583c87..a4bb796a47 100644
> > > > --- a/tests/acceptance/avocado_qemu/__init__.py
> > > > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > > > @@ -110,6 +110,12 @@ class Test(avocado.Test):
> > > > return None
> > > > def setUp(self):
> > > > + # Some utility code uses binaries from the system's PATH. For
> > > > + # instance, avocado.utils.vmimage.get() uses qemu-img, to
> > > > + # create a snapshot image. This is a transparent way of
> > > Because PATH is changed in a transparent way, wouldn't be better to also
> > > self.log.info() that fact?
> > >
> > I don't have a problem with logging it, but because it will happen for
> > *every single* test, it seems like it will become noise. I think it's
> > better to properly document this aspect of "avocado_qemu.Test" instead
> > (which is currently missing here). Something like:
> >
> > "Tests based on avocado_qemu.Test will have, as a convenience, the
> > QEMU build directory added to their PATH environment variable. The goal
> > is to allow tests to seamless use matching built binaries, instead of
> > binaries installed elsewhere in the system".
> >
> > How does it sound?
>
>
> It does.
>
>
> >
> > > > + # making sure those utilities find and use binaries on the
> > > > + # build tree by default.
> > > > + os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR,
> > > > os.environ['PATH'])
> > > I think PATH should be set only once at class initialization. Perhaps in
> > > setUpClass()?
> > >
> > > - Wainer
> > >
> > The Avocado test isolation model makes setUpClass() unnecessary,
> > unsupported and pointless, so we only support setUp().
> >
> > Every test already runs on its own process, and with the nrunner
> > model, should be able to run on completely different systems. That's
> > why we don't want to support a setUpClass() like approach.
>
> Okay, thanks for the explanation.
>
And thanks for the review. Given the level of controversy here, I've
decided to take a different approach on v8. Basically, I'm adding an
interface to avocado.utils.vmimage[1], so that we can explicitly
control the qemu-img binary used.
Looking forward to your opinion on v8.
Thanks,
- Cleber.
[1] - https://github.com/avocado-framework/avocado/pull/3374
> Thanks,
>
> Wainer
>
> >
> > - Cleber.
> >
> >
>
>
- Re: [PATCH v7 3/8] Acceptance tests: use avocado tags for machine type, (continued)
[PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir, Cleber Rosa, 2019/11/04
[PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH, Cleber Rosa, 2019/11/04
Re: [PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH, Philippe Mathieu-Daudé, 2019/11/08
[PATCH v7 7/8] Acceptance tests: depend on qemu-img, Cleber Rosa, 2019/11/04
[PATCH v7 8/8] Acceptance test: add "boot_linux" tests, Cleber Rosa, 2019/11/04