acpi/tests/avocado/bits: don't remove the work directory when V is in env

[PATCH] acpi/tests/avocado/bits: don't remove the work directory when V is in env

Posted by Ani Sinha 1 year, 5 months ago

Debugging bits issue often involves running the QEMU command line manually
outside of the avocado environment with the generated ISO. Hence, its
inconvenient if the iso gets cleaned up after the test has finished. This change
makes sure that the work directory is kept after the test finishes if the test
is run with V=1 in the environment so that the iso is available for use with the
QEMU command line.

CC: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 tests/avocado/acpi-bits.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/acpi-bits.py b/tests/avocado/acpi-bits.py
index 8745a58a76..7657343f2a 100644
--- a/tests/avocado/acpi-bits.py
+++ b/tests/avocado/acpi-bits.py
@@ -354,7 +354,11 @@ def tearDown(self):
         if self._vm:
             self.assertFalse(not self._vm.is_running)
         self.logger.info('removing the work directory %s', self._workDir)
-        shutil.rmtree(self._workDir)
+        if not os.getenv('V'):
+            shutil.rmtree(self._workDir)
+        else:
+            self.logger.info('not removing the work directory %s as V is ' \
+                             'passed in the environment', self._workDir)
         super().tearDown()
 
     def test_acpi_smbios_bits(self):
-- 
2.34.1

Re: [PATCH] acpi/tests/avocado/bits: don't remove the work directory when V is in env

Posted by Daniel P. Berrangé 1 year, 5 months ago

On Thu, Nov 17, 2022 at 01:16:29PM +0530, Ani Sinha wrote:
> Debugging bits issue often involves running the QEMU command line manually
> outside of the avocado environment with the generated ISO. Hence, its
> inconvenient if the iso gets cleaned up after the test has finished. This change
> makes sure that the work directory is kept after the test finishes if the test
> is run with V=1 in the environment so that the iso is available for use with the
> QEMU command line.
> 
> CC: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  tests/avocado/acpi-bits.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/avocado/acpi-bits.py b/tests/avocado/acpi-bits.py
> index 8745a58a76..7657343f2a 100644
> --- a/tests/avocado/acpi-bits.py
> +++ b/tests/avocado/acpi-bits.py
> @@ -354,7 +354,11 @@ def tearDown(self):
>          if self._vm:
>              self.assertFalse(not self._vm.is_running)
>          self.logger.info('removing the work directory %s', self._workDir)
> -        shutil.rmtree(self._workDir)
> +        if not os.getenv('V'):
> +            shutil.rmtree(self._workDir)
> +        else:
> +            self.logger.info('not removing the work directory %s as V is ' \
> +                             'passed in the environment', self._workDir)
>          super().tearDown()

I don't think it is a good idea to hook into 'V=1'.

That is something commonly used simply to get a record of the verbose
build process. It shouldn't affect the functional operation at all.
So leaving around undeleted state is an undesirable side effect.

If you want a means for debugging invent a new env variable such
as BIOSBITS_DEBUG=1 or whatever name..

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] acpi/tests/avocado/bits: don't remove the work directory when V is in env

Posted by Ani Sinha 1 year, 5 months ago

On Thu, Nov 17, 2022 at 2:18 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Nov 17, 2022 at 01:16:29PM +0530, Ani Sinha wrote:
> > Debugging bits issue often involves running the QEMU command line manually
> > outside of the avocado environment with the generated ISO. Hence, its
> > inconvenient if the iso gets cleaned up after the test has finished. This change
> > makes sure that the work directory is kept after the test finishes if the test
> > is run with V=1 in the environment so that the iso is available for use with the
> > QEMU command line.
> >
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >  tests/avocado/acpi-bits.py | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/avocado/acpi-bits.py b/tests/avocado/acpi-bits.py
> > index 8745a58a76..7657343f2a 100644
> > --- a/tests/avocado/acpi-bits.py
> > +++ b/tests/avocado/acpi-bits.py
> > @@ -354,7 +354,11 @@ def tearDown(self):
> >          if self._vm:
> >              self.assertFalse(not self._vm.is_running)
> >          self.logger.info('removing the work directory %s', self._workDir)
> > -        shutil.rmtree(self._workDir)
> > +        if not os.getenv('V'):
> > +            shutil.rmtree(self._workDir)
> > +        else:
> > +            self.logger.info('not removing the work directory %s as V is ' \
> > +                             'passed in the environment', self._workDir)
> >          super().tearDown()
>
> I don't think it is a good idea to hook into 'V=1'.
>
> That is something commonly used simply to get a record of the verbose
> build process. It shouldn't affect the functional operation at all.

yeah you are right. I have sent out a v2. introduced BITS_DEBUG for
it. much better.

> So leaving around undeleted state is an undesirable side effect.
>
> If you want a means for debugging invent a new env variable such
> as BIOSBITS_DEBUG=1 or whatever name..
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>