Re: [PATCH 02/15] tests/functional: automatically clean up scratch files after tests

[Thomas Huth <thuth@redhat.com>]

On 19/11/2024 17.21, Alex Bennée wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
>> The build/tests/functional subdirectories are consuming huge amounts
>> of disk space.
>>
>> Split the location for scratch files into a 'scratch' sub-directory,
>> separate from log files, and delete it upon completion of each test.
>> The new env variable QEMU_TEST_KEEP_SCRATCH can be set to preserve
>> this scratch dir for debugging access if required.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   docs/devel/testing/functional.rst      |  6 ++++++
>>   tests/functional/qemu_test/testcase.py | 14 +++++++++-----
>>   2 files changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
>> index bf6f1bb81e..6b5d0c5b98 100644
>> --- a/docs/devel/testing/functional.rst
>> +++ b/docs/devel/testing/functional.rst
>> @@ -65,6 +65,12 @@ to the QEMU binary that should be used for the test, for example::
>>     $ export QEMU_TEST_QEMU_BINARY=$PWD/qemu-system-x86_64
>>     $ python3 ../tests/functional/test_file.py
>>   
>> +The test framework will automatically purge any scratch files created during
>> +the tests. If needing to debug a failed test, it is possible to keep these
>> +files around on disk by setting ```QEMU_TEST_KEEP_SCRATCH=1``` as an env
>> +variable.  Any preserved files will be deleted the next time the test is run
>> +without this variable set.
>> +
>>   Overview
>>   --------
>>   
>> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
>> index 411978b5ef..b9418e2ac0 100644
>> --- a/tests/functional/qemu_test/testcase.py
>> +++ b/tests/functional/qemu_test/testcase.py
>> @@ -13,8 +13,9 @@
>>   
>>   import logging
>>   import os
>> -import subprocess
>>   import pycotap
>> +import shutil
>> +import subprocess
>>   import sys
>>   import unittest
>>   import uuid
>> @@ -40,11 +41,12 @@ def setUp(self, bin_prefix):
>>           self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set')
>>           self.arch = self.qemu_bin.split('-')[-1]
>>   
>> -        self.workdir = os.path.join(BUILD_DIR, 'tests/functional', self.arch,
>> -                                    self.id())
>> +        self.outputdir = os.path.join(BUILD_DIR, 'tests', 'functional',
>> +                                      self.arch, self.id())
> 
> I don't think you need save self.outputdir as only self.logdir is used
> by the other functions.

I guess it might be useful to have this variable as a reference point in 
future patches, so I'm in favor of keeping it.

Reviewed-by: Thomas Huth <thuth@redhat.com>