From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 03C96C2D0C2 for ; Mon, 30 Dec 2019 18:17:30 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B4C53206DB for ; Mon, 30 Dec 2019 18:17:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DNks5X5J" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B4C53206DB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:35522 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ilzbc-0002v7-Uf for qemu-devel@archiver.kernel.org; Mon, 30 Dec 2019 13:17:28 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:33228) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ilzat-0002U0-Uz for qemu-devel@nongnu.org; Mon, 30 Dec 2019 13:16:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ilzas-0002S7-2j for qemu-devel@nongnu.org; Mon, 30 Dec 2019 13:16:43 -0500 Received: from mail-qt1-x842.google.com ([2607:f8b0:4864:20::842]:40695) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ilzar-0002Rl-Ti for qemu-devel@nongnu.org; Mon, 30 Dec 2019 13:16:42 -0500 Received: by mail-qt1-x842.google.com with SMTP id e6so30159026qtq.7 for ; Mon, 30 Dec 2019 10:16:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=dGRcDE+48+A3OatIVUGFJsHNqPfsUw/jJbt1IOEvK3c=; b=DNks5X5JKaf/GaBlswPKb5nw+HUiw6xhbYatNf1opfecZ8rKA8aWMu/YxikY2qztJK FWcW7JY1tI1SO8+FO7n7Ce0yYol4M5ffnvidv0azSxqHlEYWKbupF+6DGvzDyYEw5If5 lCJQtFOaazfXmFWm7tPMs/1dxYpmLIsDiEFFtNMhkJ1kn/2iwmqqCI9ZTJR9qukEpkm+ 2a9HDj2acNlJsNXv4BYLIVJ03MKHed3/wx9RgmXI2xvKbJdZp43JtJr4P7ZEe+p57X+0 HkI31FJFif+D1pDc+g3eGjMtnGCF39G1EWgNX9JkWSdhr7xLa9+uIgypdPPjUTXpnoxS CQOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=dGRcDE+48+A3OatIVUGFJsHNqPfsUw/jJbt1IOEvK3c=; b=CF5OSkyATptUsLmAw2OlfHGG61gINpbdS9Ajwp+PekrAiHReXLEA0fLG7V8peTeNmw 6eDNTiVXNAKf5Z9S19V0Q0+JfRNvMju9IGov6Iu7pqNbtWjBY4F4JWmtPrlrUYvy5Dzo +Zmfe6DVv9kQAk5hTQf9JNb7x9BPdRCokl96CTBHIHCUNR0BDYjZODHRo8F+MsxkDMpG XIGck8hCo/4J/DQComynEZGFfjdg2MlwxmeisoLFQi8K96r1Zja3VUt4UJ/ZfNZtOcvi HZg5br96+eXrM9ZzHlfxGoXVr7LMPRMMjMBQQyOQahMjSMQ3xrUdlqAfizfBvoFkQWGs kmAw== X-Gm-Message-State: APjAAAVfQMf9iqZLMzMtA6Mhvn2W/StHkUvFqFf0oDP+SlCAQFFS7zGQ Y/KcVIWDkR7Qr35DeYRzyoic8q6XF2QpOBSIhDw= X-Google-Smtp-Source: APXvYqxRuRaIdModR2wb+pluOKw/YuWkDdI63HqpTi1dBzvJPnsBaMj8JOlCti50n7MzXaDWLIsJNF04Kg07+fSonvY= X-Received: by 2002:ac8:1769:: with SMTP id u38mr48691733qtk.160.1577729800937; Mon, 30 Dec 2019 10:16:40 -0800 (PST) MIME-Version: 1.0 References: <20191229215158.5788-1-mrolnik@gmail.com> <20191229215158.5788-21-mrolnik@gmail.com> In-Reply-To: From: Michael Rolnik Date: Mon, 30 Dec 2019 20:15:59 +0200 Message-ID: Subject: Re: [PATCH v40 20/21] target/avr: Add Avocado test To: Wainer dos Santos Moschetta Content-Type: multipart/alternative; boundary="00000000000056244c059aefd843" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::842 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Thomas Huth , Joaquin de Andres , Richard Henderson , QEMU Developers , Pavel Dovgalyuk , Igor Mammedov , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Aleksandar Markovic Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000056244c059aefd843 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Wainer. thanks for reviewing. 1. when `self.vm.shutdown()` is not called `self.vm.get_log()` returns `ERROR: argument of type 'NoneType' is not iterable` 2. I will remove the unnecessary imports Thanks, Michael Rolnik On Mon, Dec 30, 2019 at 7:37 PM Wainer dos Santos Moschetta < wainersm@redhat.com> wrote: > Hi Michael, > > On 12/29/19 7:51 PM, Michael Rolnik wrote: > > The test is based on > > https://github.com/seharris/qemu-avr-tests/tree/master/free-rtos/Demo > > demo which. If working correctly, prints 'ABCDEFGHIJKLMNOPQRSTUVWX' out= . > > it also demostrates that timer and IRQ are working > > > > Signed-off-by: Michael Rolnik > > Reviewed-by: Philippe Mathieu-Daud=C3=A9 > > Tested-by: Philippe Mathieu-Daud=C3=A9 > > Acked-by: Thomas Huth > > --- > > tests/acceptance/machine_avr6.py | 58 +++++++++++++++++++++++++++++++= + > > 1 file changed, 58 insertions(+) > > create mode 100644 tests/acceptance/machine_avr6.py > > > > diff --git a/tests/acceptance/machine_avr6.py > b/tests/acceptance/machine_avr6.py > > new file mode 100644 > > index 0000000000..7a7d8afc29 > > --- /dev/null > > +++ b/tests/acceptance/machine_avr6.py > > @@ -0,0 +1,58 @@ > > +# > > +# QEMU AVR > > +# > > +# Copyright (c) 2019 Michael Rolnik > > +# > > +# This program is free software: you can redistribute it and/or modify > > +# it under the terms of the GNU General Public License as published by > > +# the Free Software Foundation, either version 2 of the License, or > > +# (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see = . > > +# > > + > > +import logging > > +import time > > +import distutils.spawn > > + > > +from avocado import skipUnless > > +from avocado_qemu import Test > > +from avocado.utils import process > > Please remove unused imports: logging, distutils.spawn, skipUnless and > process. > > > + > > +class AVR6Machine(Test): > > It helps others reading this if you document the test purpose here. > Besides it makes the pylinter happier. ;) > > > + timeout =3D 5 > > + > > + def test_freertos(self): > > + """ > > + :avocado: tags=3Darch:avr > > + :avocado: tags=3Dmachine:sample > > + """ > > + """ > > + > https://github.com/seharris/qemu-avr-tests/raw/master/free-rtos/Demo/AVR_= ATMega2560_GCC/demo.elf > > + constantly prints out > 'ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX' > > + """ > > + rom_url =3D 'https://github.com/seharris/qemu-avr-tests' > > + rom_sha1=3D '36c3e67b8755dcf37e06af6730ef5d477b8ed16d' > > + rom_url +=3D '/raw/' > > + rom_url +=3D rom_sha1 > > + rom_url +=3D '/free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf' > > + rom_hash =3D '7eb521f511ca8f2622e0a3c5e8dd686efbb911d4' > > + rom_path =3D self.fetch_asset(rom_url, asset_hash=3Drom_hash) > > + > > + self.vm.set_machine('sample') > > + self.vm.add_args('-bios', rom_path) > > + self.vm.add_args('-nographic') > > + self.vm.launch() > > + > > + time.sleep(2) > > + self.vm.shutdown() > > Do you really need to shutdown the VM here? Because it will be shut down > later on avocado_qemu.Test.tearDown() anyway. > > > + > > + match =3D 'ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX' > > + > > + self.assertIn(match, self.vm.get_log()) > > It is a matter of taste, but I would simply do: > > self.assertIn('ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX', > > self.vm.get_log()) > > Thanks for writing this acceptance test! > > - Wainer > > --=20 Best Regards, Michael Rolnik --00000000000056244c059aefd843 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Wainer.

thanks for reviewing.

1. when `self.vm.shutdown()` is not called `self.vm= .get_log()` returns `ERROR: argument of type 'NoneType' is not iter= able`
2. I will remove the unnecessary imports

Thanks,
Michael Rolnik


On Mon, Dec 30,= 2019 at 7:37 PM Wainer dos Santos Moschetta <wainersm@redhat.com> wrote:
Hi Michael,

On 12/29/19 7:51 PM, Michael Rolnik wrote:
> The test is based on
> https://github.com/seharri= s/qemu-avr-tests/tree/master/free-rtos/Demo
> demo which. If working correctly, prints 'ABCDEFGHIJKLMNOPQRSTUVWX= ' out.
> it also demostrates that timer and IRQ are working
>
> Signed-off-by: Michael Rolnik <mrolnik@gmail.com>
> Reviewed-by: Philippe Mathieu-Daud=C3=A9 <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daud=C3=A9 <philmd@redhat.com>
> Acked-by: Thomas Huth <thuth@redhat.com>
> ---
>=C2=A0 =C2=A0tests/acceptance/machine_avr6.py | 58 ++++++++++++++++++++= ++++++++++++
>=C2=A0 =C2=A01 file changed, 58 insertions(+)
>=C2=A0 =C2=A0create mode 100644 tests/acceptance/machine_avr6.py
>
> diff --git a/tests/acceptance/machine_avr6.py b/tests/acceptance/machi= ne_avr6.py
> new file mode 100644
> index 0000000000..7a7d8afc29
> --- /dev/null
> +++ b/tests/acceptance/machine_avr6.py
> @@ -0,0 +1,58 @@
> +#
> +# QEMU AVR
> +#
> +# Copyright (c) 2019 Michael Rolnik <mrolnik@gmail.com>
> +#
> +# This program is free software: you can redistribute it and/or modif= y
> +# it under the terms of the GNU General Public License as published b= y
> +# the Free Software Foundation, either version 2 of the License, or > +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.=C2=A0 See the<= br> > +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License > +# along with this program.=C2=A0 If not, see <http://www.gnu.or= g/licenses/>.
> +#
> +
> +import logging
> +import time
> +import distutils.spawn
> +
> +from avocado import skipUnless
> +from avocado_qemu import Test
> +from avocado.utils import process

Please remove unused imports: logging, distutils.spawn, skipUnless and
process.

> +
> +class AVR6Machine(Test):

It helps others reading this if you document the test purpose here.
Besides it makes the pylinter happier. ;)

> +=C2=A0 =C2=A0 timeout =3D 5
> +
> +=C2=A0 =C2=A0 def test_freertos(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 :avocado: tags=3Darch:avr
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 :avocado: tags=3Dmachine:sample
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 https://github.com/seharris/qemu-avr-tests/ra= w/master/free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 constantly prints out 'ABCDEFGHIJKLMN= OPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_url =3D 'https:/= /github.com/seharris/qemu-avr-tests'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_sha1=3D '36c3e67b8755dcf37e06af67= 30ef5d477b8ed16d'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_url +=3D '/raw/'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_url +=3D rom_sha1
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_url +=3D '/free-rtos/Demo/AVR_ATM= ega2560_GCC/demo.elf'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_hash =3D '7eb521f511ca8f2622e0a3c= 5e8dd686efbb911d4'
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rom_path =3D self.fetch_asset(rom_url, as= set_hash=3Drom_hash)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.vm.set_machine('sample')
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.vm.add_args('-bios', rom_pat= h)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.vm.add_args('-nographic') > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.vm.launch()
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 time.sleep(2)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.vm.shutdown()

Do you really need to shutdown the VM here? Because it will be shut down later on avocado_qemu.Test.tearDown() anyway.

> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 match =3D 'ABCDEFGHIJKLMNOPQRSTUVWXAB= CDEFGHIJKLMNOPQRSTUVWX'
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.assertIn(match, self.vm.get_log())
It is a matter of taste, but I would simply do:

self.assertIn('ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX',
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 self.vm.get_log())

Thanks for writing this acceptance test!

- Wainer



--
Best Regards,
Michael Rolnik
--00000000000056244c059aefd843--