On Thu, Dec 12, 2024 at 07:57:37AM +0100, Thomas Huth wrote:
> On 11/12/2024 18.26, Daniel P. Berrangé wrote:
> > Platforms we target have new enough tesseract that it suffices to merely
> > check if the binary exists.
> >
> > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> > ---
> > tests/functional/qemu_test/tesseract.py | 12 +-----------
> > tests/functional/test_m68k_nextcube.py | 8 +++-----
> > 2 files changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/tests/functional/qemu_test/tesseract.py
> > b/tests/functional/qemu_test/tesseract.py
> > index ef1833139d..1b7818090a 100644
> > --- a/tests/functional/qemu_test/tesseract.py
> > +++ b/tests/functional/qemu_test/tesseract.py
> > @@ -7,17 +7,7 @@
> > import logging
> > -from . import has_cmd, run_cmd
> > -
> > -def tesseract_available(expected_version):
> > - (has_tesseract, _) = has_cmd('tesseract')
> > - if not has_tesseract:
> > - return False
> > - (stdout, stderr, ret) = run_cmd([ 'tesseract', '--version'])
> > - if ret:
> > - return False
> > - version = stdout.split()[1]
> > - return int(version.split('.')[0]) >= expected_version
> > +from . import run_cmd
> > def tesseract_ocr(image_path, tesseract_args=''):
> > console_logger = logging.getLogger('console')
> > diff --git a/tests/functional/test_m68k_nextcube.py
> > b/tests/functional/test_m68k_nextcube.py
> > index 0124622c40..1022e8f468 100755
> > --- a/tests/functional/test_m68k_nextcube.py
> > +++ b/tests/functional/test_m68k_nextcube.py
> > @@ -13,7 +13,8 @@
> > from qemu_test import QemuSystemTest, Asset
> > from unittest import skipUnless
> > -from qemu_test.tesseract import tesseract_available, tesseract_ocr
> > +from qemu_test import has_cmd
> > +from qemu_test.tesseract import tesseract_ocr
> > PIL_AVAILABLE = True
> > try:
> > @@ -53,10 +54,7 @@ def test_bootrom_framebuffer_size(self):
> > self.assertEqual(width, 1120)
> > self.assertEqual(height, 832)
> > - # Tesseract 4 adds a new OCR engine based on LSTM neural networks. The
> > - # new version is faster and more accurate than version 3. The drawback
> > is
> > - # that it is still alpha-level software.
> > - @skipUnless(tesseract_available(4), 'tesseract OCR tool not available')
> > + @skipUnless(*has_cmd('tesseract') 'tesseract OCR tool not available')
>
> The *has_cmd('tesseract') already provides the error message, so you've got
> to drop the 'tesseract OCR tool not available' part now, otherwise this ends
> up in an SyntaxError. You likely didn't notice since it gets replaced later
> anyway, but for bisectability, it would be good to fix it.
Opps, yes, will address it.
>
> Anyway, this is yet another good example why we should rather get rid of
> has_cmd() ... it's too error prone, I made the same or similar mistake in
> the past already, too.
>
> Thomas
>
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 :|
