| Message ID | 20210927163116.1998349-2-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests/acceptance: Add bFLT loader linux-user test | expand |
On Mon, Sep 27, 2021 at 1:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > The Avocado Test::fetch_asset() is handy to download artifacts > before running tests. The current class is named Test but only > tests system emulation. As we want to test user emulation, > refactor the common code as QemuBaseTest. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > tests/acceptance/avocado_qemu/__init__.py | 72 +++++++++++++---------- > 1 file changed, 41 insertions(+), 31 deletions(-) > > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index 2c4fef3e149..8fcbed74849 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -175,7 +175,7 @@ def exec_command_and_wait_for_pattern(test, command, > """ > _console_interaction(test, success_message, failure_message, command + '\r') > > -class Test(avocado.Test): > +class QemuBaseTest(avocado.Test): > def _get_unique_tag_val(self, tag_name): > """ > Gets a tag value, if unique for a key > @@ -185,6 +185,46 @@ def _get_unique_tag_val(self, tag_name): > return vals.pop() > return None > > + def setUp(self): > + self.arch = self.params.get('arch', > + default=self._get_unique_tag_val('arch')) > + > + self.cpu = self.params.get('cpu', > + default=self._get_unique_tag_val('cpu')) > + > + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) > + self.qemu_bin = self.params.get('qemu_bin', > + default=default_qemu_bin) > + if self.qemu_bin is None: > + self.cancel("No QEMU binary defined or found in the build tree") > + > + def fetch_asset(self, name, > + asset_hash=None, algorithm=None, > + locations=None, expire=None, > + find_only=False, cancel_on_missing=True): > + return super(QemuBaseTest, self).fetch_asset(name, It is preferable to use the PEP3135 (https://www.python.org/dev/peps/pep-3135/) when calling `super` as linter are complaining about it: return super().fetch_asset(name, And after reading through the patch I noticed it was a method move, so, feel free to take the suggestion or ignore it for now. > + asset_hash=asset_hash, > + algorithm=algorithm, > + locations=locations, > + expire=expire, > + find_only=find_only, > + cancel_on_missing=cancel_on_missing) > + > + > +class Test(QemuBaseTest): > + """Facilitates system emulation tests. > + > + TODO: Rename this class as `QemuSystemTest`. > + """ > + > + def setUp(self): > + self._vms = {} > + > + super(Test, self).setUp() Same from previous comment: super().setUp() > + > + self.machine = self.params.get('machine', > + default=self._get_unique_tag_val('machine')) > + > def require_accelerator(self, accelerator): > """ > Requires an accelerator to be available for the test to continue > @@ -207,24 +247,6 @@ def require_accelerator(self, accelerator): > self.cancel("%s accelerator does not seem to be " > "available" % accelerator) > > - def setUp(self): > - self._vms = {} > - > - self.arch = self.params.get('arch', > - default=self._get_unique_tag_val('arch')) > - > - self.cpu = self.params.get('cpu', > - default=self._get_unique_tag_val('cpu')) > - > - self.machine = self.params.get('machine', > - default=self._get_unique_tag_val('machine')) > - > - default_qemu_bin = pick_default_qemu_bin(arch=self.arch) > - self.qemu_bin = self.params.get('qemu_bin', > - default=default_qemu_bin) > - if self.qemu_bin is None: > - self.cancel("No QEMU binary defined or found in the build tree") > - > def _new_vm(self, name, *args): > self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") > vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir, > @@ -277,18 +299,6 @@ def tearDown(self): > vm.shutdown() > self._sd = None > > - def fetch_asset(self, name, > - asset_hash=None, algorithm=None, > - locations=None, expire=None, > - find_only=False, cancel_on_missing=True): > - return super(Test, self).fetch_asset(name, > - asset_hash=asset_hash, > - algorithm=algorithm, > - locations=locations, > - expire=expire, > - find_only=find_only, > - cancel_on_missing=cancel_on_missing) > - > > class LinuxSSHMixIn: > """Contains utility methods for interacting with a guest via SSH.""" > -- > 2.31.1 > Except for one (or two, if you consider the first) small comment, looks good to me, so Reviewed-by: Willian Rampazzo <willianr@redhat.com>
On 11/1/21 19:01, Willian Rampazzo wrote: > On Mon, Sep 27, 2021 at 1:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> The Avocado Test::fetch_asset() is handy to download artifacts >> before running tests. The current class is named Test but only >> tests system emulation. As we want to test user emulation, >> refactor the common code as QemuBaseTest. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> tests/acceptance/avocado_qemu/__init__.py | 72 +++++++++++++---------- >> 1 file changed, 41 insertions(+), 31 deletions(-) >> >> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >> index 2c4fef3e149..8fcbed74849 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -175,7 +175,7 @@ def exec_command_and_wait_for_pattern(test, command, >> """ >> _console_interaction(test, success_message, failure_message, command + '\r') >> >> -class Test(avocado.Test): >> +class QemuBaseTest(avocado.Test): >> def _get_unique_tag_val(self, tag_name): >> """ >> Gets a tag value, if unique for a key >> @@ -185,6 +185,46 @@ def _get_unique_tag_val(self, tag_name): >> return vals.pop() >> return None >> >> + def setUp(self): >> + self.arch = self.params.get('arch', >> + default=self._get_unique_tag_val('arch')) >> + >> + self.cpu = self.params.get('cpu', >> + default=self._get_unique_tag_val('cpu')) >> + >> + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) >> + self.qemu_bin = self.params.get('qemu_bin', >> + default=default_qemu_bin) >> + if self.qemu_bin is None: >> + self.cancel("No QEMU binary defined or found in the build tree") >> + >> + def fetch_asset(self, name, >> + asset_hash=None, algorithm=None, >> + locations=None, expire=None, >> + find_only=False, cancel_on_missing=True): >> + return super(QemuBaseTest, self).fetch_asset(name, > > It is preferable to use the PEP3135 > (https://www.python.org/dev/peps/pep-3135/) when calling `super` as > linter are complaining about it: > > return super().fetch_asset(name, > > And after reading through the patch I noticed it was a method move, > so, feel free to take the suggestion or ignore it for now. This series was sent before commit 14f02d8a9ec ("Merge 'integration-testing-20210927' into staging") :/ I'll modify, thanks.
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 2c4fef3e149..8fcbed74849 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -175,7 +175,7 @@ def exec_command_and_wait_for_pattern(test, command, """ _console_interaction(test, success_message, failure_message, command + '\r') -class Test(avocado.Test): +class QemuBaseTest(avocado.Test): def _get_unique_tag_val(self, tag_name): """ Gets a tag value, if unique for a key @@ -185,6 +185,46 @@ def _get_unique_tag_val(self, tag_name): return vals.pop() return None + def setUp(self): + self.arch = self.params.get('arch', + default=self._get_unique_tag_val('arch')) + + self.cpu = self.params.get('cpu', + default=self._get_unique_tag_val('cpu')) + + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) + self.qemu_bin = self.params.get('qemu_bin', + default=default_qemu_bin) + if self.qemu_bin is None: + self.cancel("No QEMU binary defined or found in the build tree") + + def fetch_asset(self, name, + asset_hash=None, algorithm=None, + locations=None, expire=None, + find_only=False, cancel_on_missing=True): + return super(QemuBaseTest, self).fetch_asset(name, + asset_hash=asset_hash, + algorithm=algorithm, + locations=locations, + expire=expire, + find_only=find_only, + cancel_on_missing=cancel_on_missing) + + +class Test(QemuBaseTest): + """Facilitates system emulation tests. + + TODO: Rename this class as `QemuSystemTest`. + """ + + def setUp(self): + self._vms = {} + + super(Test, self).setUp() + + self.machine = self.params.get('machine', + default=self._get_unique_tag_val('machine')) + def require_accelerator(self, accelerator): """ Requires an accelerator to be available for the test to continue @@ -207,24 +247,6 @@ def require_accelerator(self, accelerator): self.cancel("%s accelerator does not seem to be " "available" % accelerator) - def setUp(self): - self._vms = {} - - self.arch = self.params.get('arch', - default=self._get_unique_tag_val('arch')) - - self.cpu = self.params.get('cpu', - default=self._get_unique_tag_val('cpu')) - - self.machine = self.params.get('machine', - default=self._get_unique_tag_val('machine')) - - default_qemu_bin = pick_default_qemu_bin(arch=self.arch) - self.qemu_bin = self.params.get('qemu_bin', - default=default_qemu_bin) - if self.qemu_bin is None: - self.cancel("No QEMU binary defined or found in the build tree") - def _new_vm(self, name, *args): self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_") vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir, @@ -277,18 +299,6 @@ def tearDown(self): vm.shutdown() self._sd = None - def fetch_asset(self, name, - asset_hash=None, algorithm=None, - locations=None, expire=None, - find_only=False, cancel_on_missing=True): - return super(Test, self).fetch_asset(name, - asset_hash=asset_hash, - algorithm=algorithm, - locations=locations, - expire=expire, - find_only=find_only, - cancel_on_missing=cancel_on_missing) - class LinuxSSHMixIn: """Contains utility methods for interacting with a guest via SSH."""
The Avocado Test::fetch_asset() is handy to download artifacts before running tests. The current class is named Test but only tests system emulation. As we want to test user emulation, refactor the common code as QemuBaseTest. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- tests/acceptance/avocado_qemu/__init__.py | 72 +++++++++++++---------- 1 file changed, 41 insertions(+), 31 deletions(-)