qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR
@ 2019-07-01 15:34 Philippe Mathieu-Daudé
  2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 1/2] " Philippe Mathieu-Daudé
  2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 2/2] .travis.yml: Let the avocado job run the NeXTcube tests Philippe Mathieu-Daudé
  0 siblings, 2 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 15:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Eduardo Habkost, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Gerd Hoffmann, Cleber Rosa,
	Philippe Mathieu-Daudé

Hi,

I was looking at Thomas' last series [*] where he adds the
NeXTcube machine, thinking about enforcing a new rule "new
machines must have tests". Then I realized the UART is not
yet implemented, so our current sample tests are not helpful.

Since the framebuffer is working, I gave a try at dumping the
screen content via the HMP 'screendump' command, then parsing
the screenshot with an OCR tool.

The default ROM dump the bootlog to a console. Using the old
good tesseract tool we can recover some useful words to be
sure the guest is sane, its framebuffer is definitively working.

This test takes less than 6s on Travis-CI:
https://travis-ci.org/philmd/qemu/builds/552174983#L1836

   AVOCADO tests/acceptance
 (3/9) /home/travis/build/philmd/qemu/tests/acceptance/machine_m68k_nextcube.py:NextCubeMachine.test_bootrom_framebuffer:  PASS (5.69 s)

Since v1: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06514.html
- use the English dictionary (Thomas)
- support tesseract v3 and v4 (much better results with v4, but not
  all distros provide it)
- add a test of the framebuffer width/height

Regards,

Phil.

Based-on: 20190628181536.13729-1-huth@tuxfamily.org
[*] "m68k: Add basic support for the NeXTcube machine"
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06393.html

Philippe Mathieu-Daudé (2):
  tests/acceptance: Add test of NeXTcube framebuffer using OCR
  .travis.yml: Let the avocado job run the NeXTcube tests

 .travis.yml                               |   7 +-
 tests/acceptance/machine_m68k_nextcube.py | 102 ++++++++++++++++++++++
 2 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 tests/acceptance/machine_m68k_nextcube.py

-- 
2.19.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH v2 1/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR
  2019-07-01 15:34 [Qemu-devel] [PATCH v2 0/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR Philippe Mathieu-Daudé
@ 2019-07-01 15:34 ` Philippe Mathieu-Daudé
  2019-07-01 20:09   ` Cleber Rosa
  2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 2/2] .travis.yml: Let the avocado job run the NeXTcube tests Philippe Mathieu-Daudé
  1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 15:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Eduardo Habkost, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Gerd Hoffmann, Cleber Rosa,
	Philippe Mathieu-Daudé

Add a test of the NeXTcube framebuffer using the Tesseract OCR
engine on a screenshot of the framebuffer device.

The test is very quick:

  $ avocado --show=app,ocr run tests/acceptance/machine_m68k_nextcube.py
  JOB ID     : f7d3c27976047036dc568183baf64c04863d9985
  JOB LOG    : ~/avocado/job-results/job-2019-06-29T16.18-f7d3c27/job.log
  (1/1) tests/acceptance/machine_m68k_nextcube.py:NextCubeMachine.test_bootrom_framebuffer: |ocr:
  ue r pun Honl'flx ; 5‘ 55‘
  avg ncaaaaa 25 MHZ, memary jag m
  Backplane slat «a
  Ethernet address a a r a r3 2
  Memgry sackets aea canflqured far 16MB Darlly page made stMs but have 16MB page made stMs )nstalled
  Memgry sackets a and 1 canflqured far 16MB Darlly page made stMs but have 16MB page made stMs )nstalled
  [...]
  Yestlnq the rpu, 5::
  system test raneg Errar egge 51
  Egg: cammand
  Default pggc devlce nut fauna
  NEXY>I
  PASS (3.59 s)
  RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
  JOB TIME   : 3.97 s

Documentation on how to install tesseract:
  https://github.com/tesseract-ocr/tesseract/wiki#installation

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v2:
- test fb sizes
- handle 2 versions of teseract
---
 tests/acceptance/machine_m68k_nextcube.py | 102 ++++++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 tests/acceptance/machine_m68k_nextcube.py

diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py
new file mode 100644
index 0000000000..f8e514a058
--- /dev/null
+++ b/tests/acceptance/machine_m68k_nextcube.py
@@ -0,0 +1,102 @@
+# Functional test that boots a VM and run OCR on the framebuffer
+#
+# Copyright (c) Philippe Mathieu-Daudé <f4bug@amsat.org>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import logging
+import time
+import distutils.spawn
+
+from avocado import skipUnless
+from avocado_qemu import Test
+from avocado.utils import process
+
+try:
+    from PIL import Image
+    pil_available = True
+except ImportError:
+    pil_available = False
+
+
+def tesseract_available(expected_version):
+    if not distutils.spawn.find_executable('tesseract'):
+        return False
+    res = process.run('tesseract --version')
+    try:
+        version = res.stdout_text.split()[1]
+    except IndexError:
+        version = res.stderr_text.split()[1]
+    return int(version.split('.')[0]) == expected_version
+
+
+class NextCubeMachine(Test):
+
+    timeout = 15
+
+    def check_bootrom_framebuffer(self, screenshot_path):
+        rom_url = ('http://www.nextcomputers.org/NeXTfiles/Software/ROM_Files/'
+                   '68040_Non-Turbo_Chipset/Rev_2.5_v66.BIN')
+        rom_hash = 'b3534796abae238a0111299fc406a9349f7fee24'
+        rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
+
+        self.vm.set_machine('next-cube')
+        self.vm.add_args('-bios', rom_path)
+        self.vm.launch()
+
+        self.log.info('VM launched, waiting for display')
+        # FIXME how to catch the 'displaysurface_create 1120x832' trace-event?
+        time.sleep(2)
+
+        self.vm.command('human-monitor-command',
+                        command_line='screendump %s' % screenshot_path)
+
+    @skipUnless(pil_available, 'Python PIL not installed')
+    def test_bootrom_framebuffer_size(self):
+        """
+        :avocado: tags=arch:m68k
+        :avocado: tags=machine:next-cube
+        :avocado: tags=device:framebuffer
+        """
+        screenshot_path = self.workdir + "dump"
+        self.check_bootrom_framebuffer(screenshot_path)
+
+        width, height = Image.open(screenshot_path).size
+        self.assertEqual(width, 1120)
+        self.assertEqual(height, 832)
+
+    @skipUnless(tesseract_available(3), 'tesseract v3 OCR tool not available')
+    def test_bootrom_framebuffer_ocr_with_tesseract_v3(self):
+        """
+        :avocado: tags=arch:m68k
+        :avocado: tags=machine:next-cube
+        :avocado: tags=device:framebuffer
+        """
+        screenshot_path = self.workdir + "dump"
+        self.check_bootrom_framebuffer(screenshot_path)
+
+        console_logger = logging.getLogger('ocr')
+        text = process.run("tesseract %s stdout" % screenshot_path).stdout_text
+        console_logger.debug(text)
+        self.assertIn('Backplane', text)
+        self.assertIn('Ethernet address', text)
+
+    @skipUnless(tesseract_available(4), 'tesseract v4 OCR tool not available')
+    def test_bootrom_framebuffer_ocr_with_tesseract_v4(self):
+        """
+        :avocado: tags=arch:m68k
+        :avocado: tags=machine:next-cube
+        :avocado: tags=device:framebuffer
+        """
+        screenshot_path = self.workdir + "dump"
+        self.check_bootrom_framebuffer(screenshot_path)
+
+        console_logger = logging.getLogger('ocr')
+        proc = process.run("tesseract --oem 1 %s stdout" % screenshot_path)
+        text = proc.stdout_text
+        console_logger.debug(text)
+        self.assertIn('Testing the FPU, SCC', text)
+        self.assertIn('System test failed. Error code 51', text)
+        self.assertIn('Boot command', text)
+        self.assertIn('Next>', text)
-- 
2.19.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH v2 2/2] .travis.yml: Let the avocado job run the NeXTcube tests
  2019-07-01 15:34 [Qemu-devel] [PATCH v2 0/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR Philippe Mathieu-Daudé
  2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 1/2] " Philippe Mathieu-Daudé
@ 2019-07-01 15:34 ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 15:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Eduardo Habkost, Thomas Huth, Alex Bennée,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Gerd Hoffmann, Cleber Rosa,
	Philippe Mathieu-Daudé

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v2:
- install tesseract English package (Thomas)
- install PIL
---
 .travis.yml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index aeb9b211cd..0980f65ec5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -231,15 +231,20 @@ matrix:
 
     # Acceptance (Functional) tests
     - env:
-        - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu"
+        - CONFIG="--python=/usr/bin/python3 --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu,m68k-softmmu"
         - TEST_CMD="make check-acceptance"
       after_failure:
         - cat tests/results/latest/job.log
       addons:
         apt:
           packages:
+            - python3-pil
             - python3-pip
             - python3.5-venv
+            - tesseract-ocr
+            - tesseract-ocr-eng
+
+
     # Using newer GCC with sanitizers
     - addons:
         apt:
-- 
2.19.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR
  2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 1/2] " Philippe Mathieu-Daudé
@ 2019-07-01 20:09   ` Cleber Rosa
  2019-07-01 21:24     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 5+ messages in thread
From: Cleber Rosa @ 2019-07-01 20:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, Eduardo Habkost, Thomas Huth, Alex Bennée,
	qemu-devel, Wainer dos Santos Moschetta, Gerd Hoffmann,
	Philippe Mathieu-Daudé

On Mon, Jul 01, 2019 at 05:34:35PM +0200, Philippe Mathieu-Daudé wrote:
> Add a test of the NeXTcube framebuffer using the Tesseract OCR
> engine on a screenshot of the framebuffer device.
> 
> The test is very quick:
> 
>   $ avocado --show=app,ocr run tests/acceptance/machine_m68k_nextcube.py

Shouldn't we stick to "console" here?  I understand we're resorting to ocr
but the content, for what it's worth, should be the same as in the console
for other tests.  This allows a common expectation across tests too.

>   JOB ID     : f7d3c27976047036dc568183baf64c04863d9985
>   JOB LOG    : ~/avocado/job-results/job-2019-06-29T16.18-f7d3c27/job.log
>   (1/1) tests/acceptance/machine_m68k_nextcube.py:NextCubeMachine.test_bootrom_framebuffer: |ocr:
>   ue r pun Honl'flx ; 5‘ 55‘
>   avg ncaaaaa 25 MHZ, memary jag m
>   Backplane slat «a
>   Ethernet address a a r a r3 2
>   Memgry sackets aea canflqured far 16MB Darlly page made stMs but have 16MB page made stMs )nstalled
>   Memgry sackets a and 1 canflqured far 16MB Darlly page made stMs but have 16MB page made stMs )nstalled
>   [...]
>   Yestlnq the rpu, 5::
>   system test raneg Errar egge 51
>   Egg: cammand
>   Default pggc devlce nut fauna
>   NEXY>I
>   PASS (3.59 s)
>   RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
>   JOB TIME   : 3.97 s
> 
> Documentation on how to install tesseract:
>   https://github.com/tesseract-ocr/tesseract/wiki#installation
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v2:
> - test fb sizes
> - handle 2 versions of teseract
> ---
>  tests/acceptance/machine_m68k_nextcube.py | 102 ++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 tests/acceptance/machine_m68k_nextcube.py
> 
> diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py
> new file mode 100644
> index 0000000000..f8e514a058
> --- /dev/null
> +++ b/tests/acceptance/machine_m68k_nextcube.py
> @@ -0,0 +1,102 @@
> +# Functional test that boots a VM and run OCR on the framebuffer
> +#
> +# Copyright (c) Philippe Mathieu-Daudé <f4bug@amsat.org>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +import logging
> +import time
> +import distutils.spawn
> +
> +from avocado import skipUnless
> +from avocado_qemu import Test
> +from avocado.utils import process

Style nitpick:

from avocado_qemu import Test
from avocado import skipUnless
from avocado.utils import process

> +
> +try:
> +    from PIL import Image
> +    pil_available = True

Another style nitpick, but very minor issue is the use of ALL_CAPS
variables if they are at the module level.  So that would become

PIL_AVAILABLE = True

> +except ImportError:
> +    pil_available = False
> +
> +
> +def tesseract_available(expected_version):
> +    if not distutils.spawn.find_executable('tesseract'):

Just though of pointing out that there's a similar function in
avocado.utils.path, called find_command:

https://avocado-framework.readthedocs.io/en/68.0/api/utils/avocado.utils.html#avocado.utils.path.find_command

Feel free to pick your poison! :)

> +        return False
> +    res = process.run('tesseract --version')
> +    try:
> +        version = res.stdout_text.split()[1]
> +    except IndexError:
> +        version = res.stderr_text.split()[1]

Do some versions write this to stdout and others to stderr?

> +    return int(version.split('.')[0]) == expected_version

This can raise an exception if some other sort of output is
produced.  How about:

   import re

   match = re.match(r'tesseract\s(\d)', res)
   if match is not None:
      # now this is guaranteed to be a digit
      if int(match.groups()[0]) == expected_version:
         return True
   return False

> +
> +
> +class NextCubeMachine(Test):
> +
> +    timeout = 15
> +
> +    def check_bootrom_framebuffer(self, screenshot_path):
> +        rom_url = ('http://www.nextcomputers.org/NeXTfiles/Software/ROM_Files/'
> +                   '68040_Non-Turbo_Chipset/Rev_2.5_v66.BIN')
> +        rom_hash = 'b3534796abae238a0111299fc406a9349f7fee24'
> +        rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
> +
> +        self.vm.set_machine('next-cube')
> +        self.vm.add_args('-bios', rom_path)
> +        self.vm.launch()
> +
> +        self.log.info('VM launched, waiting for display')
> +        # FIXME how to catch the 'displaysurface_create 1120x832' trace-event?
> +        time.sleep(2)

There's avocado.utils.wait.wait_for() to *help* with waiting, but I'm
not sure about the trace-events.

> +
> +        self.vm.command('human-monitor-command',
> +                        command_line='screendump %s' % screenshot_path)
> +
> +    @skipUnless(pil_available, 'Python PIL not installed')
> +    def test_bootrom_framebuffer_size(self):
> +        """
> +        :avocado: tags=arch:m68k
> +        :avocado: tags=machine:next-cube

Here we hit the syntax limitation of the Avocado tags regex again:

https://avocado-framework.readthedocs.io/en/68.0/api/core/avocado.core.html#avocado.core.safeloader.DOCSTRING_DIRECTIVE_RE_RAW

I'll look into raising that limitation on the next release, but,
for the time being, this will need to be:

    :avocado: tags=machine:next_cube

The same applies to the other tests, of course.

> +        :avocado: tags=device:framebuffer
> +        """
> +        screenshot_path = self.workdir + "dump"

Best practice is to use os.path.join() instead.

> +        self.check_bootrom_framebuffer(screenshot_path)
> +
> +        width, height = Image.open(screenshot_path).size
> +        self.assertEqual(width, 1120)
> +        self.assertEqual(height, 832)
> +
> +    @skipUnless(tesseract_available(3), 'tesseract v3 OCR tool not available')
> +    def test_bootrom_framebuffer_ocr_with_tesseract_v3(self):
> +        """
> +        :avocado: tags=arch:m68k
> +        :avocado: tags=machine:next-cube
> +        :avocado: tags=device:framebuffer
> +        """
> +        screenshot_path = self.workdir + "dump"
> +        self.check_bootrom_framebuffer(screenshot_path)
> +
> +        console_logger = logging.getLogger('ocr')
> +        text = process.run("tesseract %s stdout" % screenshot_path).stdout_text
> +        console_logger.debug(text)
> +        self.assertIn('Backplane', text)
> +        self.assertIn('Ethernet address', text)

I haven't tried v3, but I'm curious... is this about the change in
command line syntax only?  Or do v3 and v4 are able to recognize
different characters?

- Cleber.

> +
> +    @skipUnless(tesseract_available(4), 'tesseract v4 OCR tool not available')
> +    def test_bootrom_framebuffer_ocr_with_tesseract_v4(self):
> +        """
> +        :avocado: tags=arch:m68k
> +        :avocado: tags=machine:next-cube
> +        :avocado: tags=device:framebuffer
> +        """
> +        screenshot_path = self.workdir + "dump"
> +        self.check_bootrom_framebuffer(screenshot_path)
> +
> +        console_logger = logging.getLogger('ocr')
> +        proc = process.run("tesseract --oem 1 %s stdout" % screenshot_path)
> +        text = proc.stdout_text
> +        console_logger.debug(text)
> +        self.assertIn('Testing the FPU, SCC', text)
> +        self.assertIn('System test failed. Error code 51', text)
> +        self.assertIn('Boot command', text)
> +        self.assertIn('Next>', text)
> -- 
> 2.19.1
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR
  2019-07-01 20:09   ` Cleber Rosa
@ 2019-07-01 21:24     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-07-01 21:24 UTC (permalink / raw)
  To: Cleber Rosa, Philippe Mathieu-Daudé
  Cc: Fam Zheng, Eduardo Habkost, Thomas Huth, qemu-devel,
	Wainer dos Santos Moschetta, Gerd Hoffmann, Alex Bennée

On 7/1/19 10:09 PM, Cleber Rosa wrote:
> On Mon, Jul 01, 2019 at 05:34:35PM +0200, Philippe Mathieu-Daudé wrote:
>> Add a test of the NeXTcube framebuffer using the Tesseract OCR
>> engine on a screenshot of the framebuffer device.
>>
>> The test is very quick:
>>
>>   $ avocado --show=app,ocr run tests/acceptance/machine_m68k_nextcube.py
> 
> Shouldn't we stick to "console" here?  I understand we're resorting to ocr
> but the content, for what it's worth, should be the same as in the console
> for other tests.  This allows a common expectation across tests too.
> 
>>   JOB ID     : f7d3c27976047036dc568183baf64c04863d9985
>>   JOB LOG    : ~/avocado/job-results/job-2019-06-29T16.18-f7d3c27/job.log
>>   (1/1) tests/acceptance/machine_m68k_nextcube.py:NextCubeMachine.test_bootrom_framebuffer: |ocr:
>>   ue r pun Honl'flx ; 5‘ 55‘
>>   avg ncaaaaa 25 MHZ, memary jag m
>>   Backplane slat «a
>>   Ethernet address a a r a r3 2
>>   Memgry sackets aea canflqured far 16MB Darlly page made stMs but have 16MB page made stMs )nstalled
>>   Memgry sackets a and 1 canflqured far 16MB Darlly page made stMs but have 16MB page made stMs )nstalled
>>   [...]
>>   Yestlnq the rpu, 5::
>>   system test raneg Errar egge 51
>>   Egg: cammand
>>   Default pggc devlce nut fauna
>>   NEXY>I
>>   PASS (3.59 s)
>>   RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
>>   JOB TIME   : 3.97 s
>>
>> Documentation on how to install tesseract:
>>   https://github.com/tesseract-ocr/tesseract/wiki#installation
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> v2:
>> - test fb sizes
>> - handle 2 versions of teseract
>> ---
>>  tests/acceptance/machine_m68k_nextcube.py | 102 ++++++++++++++++++++++
>>  1 file changed, 102 insertions(+)
>>  create mode 100644 tests/acceptance/machine_m68k_nextcube.py
>>
>> diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py
>> new file mode 100644
>> index 0000000000..f8e514a058
>> --- /dev/null
>> +++ b/tests/acceptance/machine_m68k_nextcube.py
>> @@ -0,0 +1,102 @@
>> +# Functional test that boots a VM and run OCR on the framebuffer
>> +#
>> +# Copyright (c) Philippe Mathieu-Daudé <f4bug@amsat.org>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> +# later.  See the COPYING file in the top-level directory.
>> +
>> +import logging
>> +import time
>> +import distutils.spawn
>> +
>> +from avocado import skipUnless
>> +from avocado_qemu import Test
>> +from avocado.utils import process
> 
> Style nitpick:
> 
> from avocado_qemu import Test
> from avocado import skipUnless
> from avocado.utils import process

What is the logic here?

>> +
>> +try:
>> +    from PIL import Image
>> +    pil_available = True
> 
> Another style nitpick, but very minor issue is the use of ALL_CAPS
> variables if they are at the module level.  So that would become
> 
> PIL_AVAILABLE = True
> 
>> +except ImportError:
>> +    pil_available = False

OK.

>> +
>> +
>> +def tesseract_available(expected_version):
>> +    if not distutils.spawn.find_executable('tesseract'):
> 
> Just though of pointing out that there's a similar function in
> avocado.utils.path, called find_command:
> 
> https://avocado-framework.readthedocs.io/en/68.0/api/utils/avocado.utils.html#avocado.utils.path.find_command
> 
> Feel free to pick your poison! :)

OK.

>> +        return False
>> +    res = process.run('tesseract --version')
>> +    try:
>> +        version = res.stdout_text.split()[1]
>> +    except IndexError:
>> +        version = res.stderr_text.split()[1]
> 
> Do some versions write this to stdout and others to stderr?

Yes...

v3: stderr
v4: stdout

>> +    return int(version.split('.')[0]) == expected_version
> 
> This can raise an exception if some other sort of output is
> produced.  How about:
> 
>    import re
> 
>    match = re.match(r'tesseract\s(\d)', res)
>    if match is not None:
>       # now this is guaranteed to be a digit
>       if int(match.groups()[0]) == expected_version:
>          return True
>    return False

I wanted to avoid regex, but OK.

>> +
>> +
>> +class NextCubeMachine(Test):
>> +
>> +    timeout = 15
>> +
>> +    def check_bootrom_framebuffer(self, screenshot_path):
>> +        rom_url = ('http://www.nextcomputers.org/NeXTfiles/Software/ROM_Files/'
>> +                   '68040_Non-Turbo_Chipset/Rev_2.5_v66.BIN')
>> +        rom_hash = 'b3534796abae238a0111299fc406a9349f7fee24'
>> +        rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
>> +
>> +        self.vm.set_machine('next-cube')
>> +        self.vm.add_args('-bios', rom_path)
>> +        self.vm.launch()
>> +
>> +        self.log.info('VM launched, waiting for display')
>> +        # FIXME how to catch the 'displaysurface_create 1120x832' trace-event?
>> +        time.sleep(2)
> 
> There's avocado.utils.wait.wait_for() to *help* with waiting, but I'm
> not sure about the trace-events.

trace-events can be logged into a file, so as with chardev I'd like to
use a pipe and monitor it in parallel.

>> +
>> +        self.vm.command('human-monitor-command',
>> +                        command_line='screendump %s' % screenshot_path)
>> +
>> +    @skipUnless(pil_available, 'Python PIL not installed')
>> +    def test_bootrom_framebuffer_size(self):
>> +        """
>> +        :avocado: tags=arch:m68k
>> +        :avocado: tags=machine:next-cube
> 
> Here we hit the syntax limitation of the Avocado tags regex again:
> 
> https://avocado-framework.readthedocs.io/en/68.0/api/core/avocado.core.html#avocado.core.safeloader.DOCSTRING_DIRECTIVE_RE_RAW
> 
> I'll look into raising that limitation on the next release, but,
> for the time being, this will need to be:
> 
>     :avocado: tags=machine:next_cube
> 
> The same applies to the other tests, of course.

OK, since there are no warnings, I did not notice.

>> +        :avocado: tags=device:framebuffer
>> +        """
>> +        screenshot_path = self.workdir + "dump"
> 
> Best practice is to use os.path.join() instead.

OK.

>> +        self.check_bootrom_framebuffer(screenshot_path)
>> +
>> +        width, height = Image.open(screenshot_path).size
>> +        self.assertEqual(width, 1120)
>> +        self.assertEqual(height, 832)
>> +
>> +    @skipUnless(tesseract_available(3), 'tesseract v3 OCR tool not available')
>> +    def test_bootrom_framebuffer_ocr_with_tesseract_v3(self):
>> +        """
>> +        :avocado: tags=arch:m68k
>> +        :avocado: tags=machine:next-cube
>> +        :avocado: tags=device:framebuffer
>> +        """
>> +        screenshot_path = self.workdir + "dump"
>> +        self.check_bootrom_framebuffer(screenshot_path)
>> +
>> +        console_logger = logging.getLogger('ocr')
>> +        text = process.run("tesseract %s stdout" % screenshot_path).stdout_text
>> +        console_logger.debug(text)
>> +        self.assertIn('Backplane', text)
>> +        self.assertIn('Ethernet address', text)
> 
> I haven't tried v3, but I'm curious... is this about the change in
> command line syntax only?  Or do v3 and v4 are able to recognize
> different characters?

Yes, they use different engine.

In short:
"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."
[https://stackoverflow.com/questions/48498465/difference-between-tesseract-3-and-tesseract-4]

Thanks for your review!

> - Cleber.
> 
>> +
>> +    @skipUnless(tesseract_available(4), 'tesseract v4 OCR tool not available')
>> +    def test_bootrom_framebuffer_ocr_with_tesseract_v4(self):
>> +        """
>> +        :avocado: tags=arch:m68k
>> +        :avocado: tags=machine:next-cube
>> +        :avocado: tags=device:framebuffer
>> +        """
>> +        screenshot_path = self.workdir + "dump"
>> +        self.check_bootrom_framebuffer(screenshot_path)
>> +
>> +        console_logger = logging.getLogger('ocr')
>> +        proc = process.run("tesseract --oem 1 %s stdout" % screenshot_path)
>> +        text = proc.stdout_text
>> +        console_logger.debug(text)
>> +        self.assertIn('Testing the FPU, SCC', text)
>> +        self.assertIn('System test failed. Error code 51', text)
>> +        self.assertIn('Boot command', text)
>> +        self.assertIn('Next>', text)
>> -- 
>> 2.19.1
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-07-02  0:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-01 15:34 [Qemu-devel] [PATCH v2 0/2] tests/acceptance: Add test of NeXTcube framebuffer using OCR Philippe Mathieu-Daudé
2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 1/2] " Philippe Mathieu-Daudé
2019-07-01 20:09   ` Cleber Rosa
2019-07-01 21:24     ` Philippe Mathieu-Daudé
2019-07-01 15:34 ` [Qemu-devel] [PATCH v2 2/2] .travis.yml: Let the avocado job run the NeXTcube tests Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).