| Message ID | 20201117173635.29101-4-alex.bennee@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | testing fixes (avocado, gitlab) | expand |
On 11/17/20 2:36 PM, Alex Bennée wrote: > Previously we were leaving temporary directories behind. While the > QEMUMachine does make efforts to clean up after itself the directory > belongs to the calling function. We use TemporaryDirectory to wrap > this although we explicitly clear the reference in tearDown() as it > doesn't get cleaned up otherwise. This patch fixes the problem introduced on patch 02 of this series. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > tests/acceptance/avocado_qemu/__init__.py | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index 3033b2cabe..bf54e419da 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -171,8 +171,8 @@ class Test(avocado.Test): > self.cancel("No QEMU binary defined or found in the build tree") > > def _new_vm(self, *args): > - sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_") > - vm = QEMUMachine(self.qemu_bin, sock_dir=sd) > + self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") Double-checking that you really meant "avo" or if your fingers forgot to type the remaining letters. :) - Wainer > + vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) > if args: > vm.add_args(*args) > return vm > @@ -193,6 +193,7 @@ class Test(avocado.Test): > def tearDown(self): > for vm in self._vms.values(): > vm.shutdown() > + self._sd = None > > def fetch_asset(self, name, > asset_hash=None, algorithm=None,
Wainer dos Santos Moschetta <wainersm@redhat.com> writes: > On 11/17/20 2:36 PM, Alex Bennée wrote: >> Previously we were leaving temporary directories behind. While the >> QEMUMachine does make efforts to clean up after itself the directory >> belongs to the calling function. We use TemporaryDirectory to wrap >> this although we explicitly clear the reference in tearDown() as it >> doesn't get cleaned up otherwise. > > This patch fixes the problem introduced on patch 02 of this series. It didn't introduce the problem in patch 2, it just moved it. The mkdtemp() was never cleaned up before. > >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> tests/acceptance/avocado_qemu/__init__.py | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >> index 3033b2cabe..bf54e419da 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -171,8 +171,8 @@ class Test(avocado.Test): >> self.cancel("No QEMU binary defined or found in the build tree") >> >> def _new_vm(self, *args): >> - sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_") >> - vm = QEMUMachine(self.qemu_bin, sock_dir=sd) >> + self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") > > Double-checking that you really meant "avo" or if your fingers forgot to > type the remaining letters. :) Hmm yeah I should probably just be consistent with the name in both patches. > > - Wainer > >> + vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) >> if args: >> vm.add_args(*args) >> return vm >> @@ -193,6 +193,7 @@ class Test(avocado.Test): >> def tearDown(self): >> for vm in self._vms.values(): >> vm.shutdown() >> + self._sd = None >> >> def fetch_asset(self, name, >> asset_hash=None, algorithm=None,
On 11/23/20 6:23 AM, Alex Bennée wrote: > Wainer dos Santos Moschetta <wainersm@redhat.com> writes: > >> On 11/17/20 2:36 PM, Alex Bennée wrote: >>> Previously we were leaving temporary directories behind. While the >>> QEMUMachine does make efforts to clean up after itself the directory >>> belongs to the calling function. We use TemporaryDirectory to wrap >>> this although we explicitly clear the reference in tearDown() as it >>> doesn't get cleaned up otherwise. >> This patch fixes the problem introduced on patch 02 of this series. > It didn't introduce the problem in patch 2, it just moved it. The > mkdtemp() was never cleaned up before. True. My bad. - Wainer > >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> tests/acceptance/avocado_qemu/__init__.py | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >>> index 3033b2cabe..bf54e419da 100644 >>> --- a/tests/acceptance/avocado_qemu/__init__.py >>> +++ b/tests/acceptance/avocado_qemu/__init__.py >>> @@ -171,8 +171,8 @@ class Test(avocado.Test): >>> self.cancel("No QEMU binary defined or found in the build tree") >>> >>> def _new_vm(self, *args): >>> - sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_") >>> - vm = QEMUMachine(self.qemu_bin, sock_dir=sd) >>> + self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") >> Double-checking that you really meant "avo" or if your fingers forgot to >> type the remaining letters. :) > Hmm yeah I should probably just be consistent with the name in both > patches. Anyway, Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > >> - Wainer >> >>> + vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) >>> if args: >>> vm.add_args(*args) >>> return vm >>> @@ -193,6 +193,7 @@ class Test(avocado.Test): >>> def tearDown(self): >>> for vm in self._vms.values(): >>> vm.shutdown() >>> + self._sd = None >>> >>> def fetch_asset(self, name, >>> asset_hash=None, algorithm=None, >
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 3033b2cabe..bf54e419da 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -171,8 +171,8 @@ class Test(avocado.Test): self.cancel("No QEMU binary defined or found in the build tree") def _new_vm(self, *args): - sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_") - vm = QEMUMachine(self.qemu_bin, sock_dir=sd) + self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") + vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) if args: vm.add_args(*args) return vm @@ -193,6 +193,7 @@ class Test(avocado.Test): def tearDown(self): for vm in self._vms.values(): vm.shutdown() + self._sd = None def fetch_asset(self, name, asset_hash=None, algorithm=None,
Previously we were leaving temporary directories behind. While the QEMUMachine does make efforts to clean up after itself the directory belongs to the calling function. We use TemporaryDirectory to wrap this although we explicitly clear the reference in tearDown() as it doesn't get cleaned up otherwise. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/acceptance/avocado_qemu/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)