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