On Mon, Dec 02, 2024 at 09:57:27AM +0100, Thomas Huth wrote:
> On 29/11/2024 18.31, Daniel P. Berrangé wrote:
> > This ensures consistency of behaviour across all the tests, and requires
> > that we provide gitlab bug links when marking a test to be skipped due
> > to unreliability.
> >
> > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> > ---
> > diff --git a/tests/functional/test_arm_aspeed.py
> > b/tests/functional/test_arm_aspeed.py
> > index 068740a813..db872ff05e 100755
> > --- a/tests/functional/test_arm_aspeed.py
> > +++ b/tests/functional/test_arm_aspeed.py
> > @@ -13,7 +13,9 @@
> > from qemu_test import (LinuxKernelTest, Asset,
> > exec_command_and_wait_for_pattern,
> > - interrupt_interactive_console_until_pattern,
> > has_cmd)
> > + interrupt_interactive_console_until_pattern,
> > + skipIfMissingCommands,
> > +)
>
> In the other files, you placed the final ")" at the end of the previous line
> instead?
>
> > diff --git a/tests/functional/test_m68k_nextcube.py
> > b/tests/functional/test_m68k_nextcube.py
> > index 0124622c40..82d3d335d0 100755
> > --- a/tests/functional/test_m68k_nextcube.py
> > +++ b/tests/functional/test_m68k_nextcube.py
> > @@ -10,16 +10,9 @@
> > import os
> > import time
> > -from qemu_test import QemuSystemTest, Asset
> > -from unittest import skipUnless
> > -
> > +from qemu_test import QemuSystemTest, Asset, skipIfMissingImports
> > from qemu_test.tesseract import tesseract_available, tesseract_ocr
> > -
> > -PIL_AVAILABLE = True
> > -try:
> > - from PIL import Image
> > -except ImportError:
> > - PIL_AVAILABLE = False
> > +from unittest import skipUnless
>
> I think you could also replace the other skipUnless() in this file nowadays:
> The version check here was only useful in the days when most distros still
> shipped Tesseract v3, but these days are gone, we don't support any of those
> distros anymore. So I think it should be fine to use skipIfMissingCommands
> here now instead.
Ah good, if we can drop that special case its nice.
>
> Anyway, I'm also fine if we keep it for now (we still can adjust it later),
> so with at least the ")" nit fixed:
>
> Reviewed-by: Thomas Huth <th...@redhat.com>
>
> > class NextCubeMachine(QemuSystemTest):
> > @@ -43,12 +36,13 @@ def check_bootrom_framebuffer(self, screenshot_path):
> > self.vm.cmd('human-monitor-command',
> > command_line='screendump %s' % screenshot_path)
> > - @skipUnless(PIL_AVAILABLE, 'Python PIL not installed')
> > + @skipIfMissingImports("PIL")
> > def test_bootrom_framebuffer_size(self):
> > self.set_machine('next-cube')
> > screenshot_path = os.path.join(self.workdir, "dump.ppm")
> > self.check_bootrom_framebuffer(screenshot_path)
> > + from PIL import Image
> > width, height = Image.open(screenshot_path).size
> > self.assertEqual(width, 1120)
> > self.assertEqual(height, 832)
> ...
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
