| Message ID | 20241211172648.2893097-6-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tests/functional: various improvements wrt assets/scratch files | expand |
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é <berrange@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. 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
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é <berrange@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
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') def test_bootrom_framebuffer_ocr_with_tesseract(self): self.set_machine('next-cube') screenshot_path = os.path.join(self.workdir, "dump.ppm")
Platforms we target have new enough tesseract that it suffices to merely check if the binary exists. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/functional/qemu_test/tesseract.py | 12 +----------- tests/functional/test_m68k_nextcube.py | 8 +++----- 2 files changed, 4 insertions(+), 16 deletions(-)