Re: [PULL 08/28] tests/functional: honour self.workdir in ACPI bits tests

[Ani Sinha]

On Mon, Nov 25, 2024 at 8:51 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> The ACPI bits test sets up its own private temporary directory into it
> creates scratch files. This is justified by a suggestion that we need
> to be able to preserve the scratch files. We have the ability to
> preserve the scratch dir with our functional harness, so there's no
> reason to diverge from standard practice in file placement.

Thanks for fixing this.

Acked-by: Ani Sinha <anisinha@redhat.com>

>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-Id: <20241121154218.1423005-9-berrange@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20241121165806.476008-9-alex.bennee@linaro.org>
>
> diff --git a/tests/functional/test_acpi_bits.py 
> b/tests/functional/test_acpi_bits.py
> index 4c192d95cc..3498b96787 100755
> --- a/tests/functional/test_acpi_bits.py
> +++ b/tests/functional/test_acpi_bits.py
> @@ -150,7 +150,6 @@ class AcpiBitsTest(QemuBaseTest): #pylint: 
> disable=too-many-instance-attributes
>      def __init__(self, *args, **kwargs):
>          super().__init__(*args, **kwargs)
>          self._vm = None
> -        self._workDir = None
>          self._baseDir = None
>
>          self._debugcon_addr = '0x403'
> @@ -169,7 +168,7 @@ def copy_bits_config(self):
>          config_file = 'bits-cfg.txt'
>          bits_config_dir = os.path.join(self._baseDir, 'acpi-bits',
>                                         'bits-config')
> -        target_config_dir = os.path.join(self._workDir,
> +        target_config_dir = os.path.join(self.workdir,
>                                           'bits-%d' %self.BITS_INTERNAL_VER,
>                                           'boot')
>          self.assertTrue(os.path.exists(bits_config_dir))
> @@ -186,7 +185,7 @@ def copy_test_scripts(self):
>
>          bits_test_dir = os.path.join(self._baseDir, 'acpi-bits',
>                                       'bits-tests')
> -        target_test_dir = os.path.join(self._workDir,
> +        target_test_dir = os.path.join(self.workdir,
>                                         'bits-%d' %self.BITS_INTERNAL_VER,
>                                         'boot', 'python')
>
> @@ -225,8 +224,8 @@ def fix_mkrescue(self, mkrescue):
>              the directory where we have extracted our pre-built bits grub
>              tarball.
>          """
> -        grub_x86_64_mods = os.path.join(self._workDir, 
> 'grub-inst-x86_64-efi')
> -        grub_i386_mods = os.path.join(self._workDir, 'grub-inst')
> +        grub_x86_64_mods = os.path.join(self.workdir, 'grub-inst-x86_64-efi')
> +        grub_i386_mods = os.path.join(self.workdir, 'grub-inst')
>
>          self.assertTrue(os.path.exists(grub_x86_64_mods))
>          self.assertTrue(os.path.exists(grub_i386_mods))
> @@ -247,11 +246,11 @@ def generate_bits_iso(self):
>          """ Uses grub-mkrescue to generate a fresh bits iso with the python
>              test scripts
>          """
> -        bits_dir = os.path.join(self._workDir,
> +        bits_dir = os.path.join(self.workdir,
>                                  'bits-%d' %self.BITS_INTERNAL_VER)
> -        iso_file = os.path.join(self._workDir,
> +        iso_file = os.path.join(self.workdir,
>                                  'bits-%d.iso' %self.BITS_INTERNAL_VER)
> -        mkrescue_script = os.path.join(self._workDir,
> +        mkrescue_script = os.path.join(self.workdir,
>                                         'grub-inst-x86_64-efi', 'bin',
>                                         'grub-mkrescue')
>
> @@ -290,17 +289,7 @@ def setUp(self): # pylint: disable=arguments-differ
>
>          self._baseDir = Path(__file__).parent
>
> -        # workdir could also be avocado's own workdir in self.workdir.
> -        # At present, I prefer to maintain my own temporary working
> -        # directory. It gives us more control over the generated bits
> -        # log files and also for debugging, we may chose not to remove
> -        # this working directory so that the logs and iso can be
> -        # inspected manually and archived if needed.
> -        self._workDir = tempfile.mkdtemp(prefix='acpi-bits-',
> -                                         suffix='.tmp')
> -        self.logger.info('working dir: %s', self._workDir)
> -
> -        prebuiltDir = os.path.join(self._workDir, 'prebuilt')
> +        prebuiltDir = os.path.join(self.workdir, 'prebuilt')
>          if not os.path.isdir(prebuiltDir):
>              os.mkdir(prebuiltDir, mode=0o775)
>
> @@ -321,10 +310,10 @@ def setUp(self): # pylint: disable=arguments-differ
>
>          # extract the bits software in the temp working directory
>          with zipfile.ZipFile(bits_zip_file, 'r') as zref:
> -            zref.extractall(self._workDir)
> +            zref.extractall(self.workdir)
>
>          with tarfile.open(grub_tar_file, 'r', encoding='utf-8') as tarball:
> -            tarball.extractall(self._workDir)
> +            tarball.extractall(self.workdir)
>
>          self.copy_test_scripts()
>          self.copy_bits_config()
> @@ -334,7 +323,7 @@ def parse_log(self):
>          """parse the log generated by running bits tests and
>             check for failures.
>          """
> -        debugconf = os.path.join(self._workDir, self._debugcon_log)
> +        debugconf = os.path.join(self.workdir, self._debugcon_log)
>          log = ""
>          with open(debugconf, 'r', encoding='utf-8') as filehandle:
>              log = filehandle.read()
> @@ -360,25 +349,18 @@ def tearDown(self):
>          """
>          if self._vm:
>              self.assertFalse(not self._vm.is_running)
> -        if not os.getenv('BITS_DEBUG') and self._workDir:
> -            self.logger.info('removing the work directory %s', self._workDir)
> -            shutil.rmtree(self._workDir)
> -        else:
> -            self.logger.info('not removing the work directory %s ' \
> -                             'as BITS_DEBUG is ' \
> -                             'passed in the environment', self._workDir)
>          super().tearDown()
>
>      def test_acpi_smbios_bits(self):
>          """The main test case implementation."""
>
> -        iso_file = os.path.join(self._workDir,
> +        iso_file = os.path.join(self.workdir,
>                                  'bits-%d.iso' %self.BITS_INTERNAL_VER)
>
>          self.assertTrue(os.access(iso_file, os.R_OK))
>
>          self._vm = QEMUBitsMachine(binary=self.qemu_bin,
> -                                   base_temp_dir=self._workDir,
> +                                   base_temp_dir=self.workdir,
>                                     debugcon_log=self._debugcon_log,
>                                     debugcon_addr=self._debugcon_addr)
>
> --
> 2.39.5
>