On Tue, Nov 15, 2022, 10:24 PM Ani Sinha <a...@anisinha.ca> wrote: > On Wed, Nov 16, 2022 at 2:58 AM John Snow <js...@redhat.com> wrote: > > > > Instead of using a hardcoded timeout, just rely on Avocado's built-in > > test case timeout. This helps avoid timeout issues on machines where 60 > > seconds is not sufficient. > > > > Signed-off-by: John Snow <js...@redhat.com> > > Reviewed-by: Ani Sinha <a...@anisinha.ca> >
Alex's critique is valid, though: the way vm.wait() works is to immediately terminate the serial console connection as it prepares for the VM to shut down. I forgot about this. (For historical reasons, it does this to avoid deadlocks when the pipe fills.) I think we definitely do want to make sure we watch the console *while* we wait for it to shut down, which is not a feature QEMUMachine really offers right now in a meaningful way. I need to make some more drastic changes to machine.py, but in the meantime I can revise this patch to do something a bit smarter so we get console logging while we wait. This is a use case worth supporting. (Thanks for writing new and interesting tests!) > > --- > > tests/avocado/acpi-bits.py | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/tests/avocado/acpi-bits.py b/tests/avocado/acpi-bits.py > > index 8745a58a766..ac13e22dc93 100644 > > --- a/tests/avocado/acpi-bits.py > > +++ b/tests/avocado/acpi-bits.py > > @@ -385,12 +385,6 @@ def test_acpi_smbios_bits(self): > > self._vm.launch() > > # biosbits has been configured to run all the specified test > suites > > # in batch mode and then automatically initiate a vm shutdown. > > - # sleep for maximum of one minute > > - max_sleep_time = time.monotonic() + 60 > > - while self._vm.is_running() and time.monotonic() < > max_sleep_time: > > - time.sleep(1) > > - > > - self.assertFalse(time.monotonic() > max_sleep_time, > > - 'The VM seems to have failed to shutdown in > time') > > - > > + # Rely on avocado's unit test timeout. > > + self._vm.wait(timeout=None) > > I think this is fine. This just waits until the VM is shutdown on its > own and relies on the avocado framework to timeout if it doesn't. We > do not need to look into the console. The test issues a shutdown from > the VM itself once its done with the batch operations. Still, if it fails, we want to see the output, right? It's very frustrating if it doesn't, especially in an automated pipeline. > > self.parse_log() > > -- > > 2.37.3 > > > >
