| Message ID | 20210211220146.2525771-4-crosa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Python / Acceptance Tests: improve logging | expand |
On 2/11/21 7:01 PM, Cleber Rosa wrote: > The QEMUMachine uses a base temporary directory for all temporary > needs. By setting it to the Avocado's workdir, it's possible to > keep the temporary files during debugging sessions much more > easily by setting the "--keep-tmp" command line option. > > Reference: https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.workdir > Reference: https://avocado-framework.readthedocs.io/en/85.0/config/index.html#run-keep-tmp > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) The changes look good to me, so: Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> But I got confused, the patch's subject states "use the job work directory" while the documentation of `workdir` says it is a "writable directory that exists during the entire test execution (...)". In the end is it a job or test work directory? - Wainer > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index bf54e419da..b7ab836455 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -172,7 +172,8 @@ class Test(avocado.Test): > > def _new_vm(self, *args): > self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") > - vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) > + vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir, > + sock_dir=self._sd.name) > if args: > vm.add_args(*args) > return vm
On 2/11/21 5:01 PM, Cleber Rosa wrote: > The QEMUMachine uses a base temporary directory for all temporary > needs. By setting it to the Avocado's workdir, it's possible to > keep the temporary files during debugging sessions much more > easily by setting the "--keep-tmp" command line option. > Makes sense, kinda-sorta. Is this a band-aid? QEMUMachine has this gem in _post_shutdown(): if self._temp_dir is not None: shutil.rmtree(self._temp_dir) self._temp_dir = None we probably do want a way to adjust the deletion policy that QEMUMachine has. Kevin and I discussed this (extremely briefly) in January. One note is that the QEMU logs are read into memory when the process closes, so iotests et al have a chance to show you those logs on failure cases. Not relevant here for your purposes. Meanwhile, we could also change the behavior of QEMUMachine, and create a temp dir deletion policy tunable: (1) Always delete (2) Never delete (3) Delete on success (keep on failure) (4) Delete on success and anticipated failures. (About #4: QEMUMachine has a condition where it will not report AbornalShutdown if .kill() is called and the retcode is observed to be -SIGKILL. We treat this as a kind of success.) These avocado tests could then just use a "never delete" policy, use the avocado runner's working dir, and then never worry about the cleanup. iotests could do something similar with a temporary directory established by the top-level runner that we could modify the behavior of with --keep-files[ <on-failure|always>] or similar. It might be time to just make this less stupidly annoying for all users of the library once and for all. > Reference: https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.workdir > Reference: https://avocado-framework.readthedocs.io/en/85.0/config/index.html#run-keep-tmp > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > tests/acceptance/avocado_qemu/__init__.py | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index bf54e419da..b7ab836455 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -172,7 +172,8 @@ class Test(avocado.Test): > > def _new_vm(self, *args): > self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") > - vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) > + vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir, > + sock_dir=self._sd.name) > if args: > vm.add_args(*args) > return vm > But, you know, absent all that extra work, sure: Reviewed-by: John Snow <jsnow@redhat.com>
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index bf54e419da..b7ab836455 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -172,7 +172,8 @@ class Test(avocado.Test): def _new_vm(self, *args): self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") - vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name) + vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir, + sock_dir=self._sd.name) if args: vm.add_args(*args) return vm
The QEMUMachine uses a base temporary directory for all temporary needs. By setting it to the Avocado's workdir, it's possible to keep the temporary files during debugging sessions much more easily by setting the "--keep-tmp" command line option. Reference: https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.workdir Reference: https://avocado-framework.readthedocs.io/en/85.0/config/index.html#run-keep-tmp Signed-off-by: Cleber Rosa <crosa@redhat.com> --- tests/acceptance/avocado_qemu/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)