diff mbox series

[v2] tests/acceptance: Test case for detecting -object crashes

Message ID 20201009202905.1845802-1-ehabkost@redhat.com
State Superseded
Headers show
Series [v2] tests/acceptance: Test case for detecting -object crashes | expand

Commit Message

Eduardo Habkost Oct. 9, 2020, 8:29 p.m. UTC 
Add a simple test case that will run QEMU directly (without QMP)
just to check for crashes when using `-object`.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* "Running command:" log message instead of "Command:" (Cleber)
* Use universal_newlines=True instead of encoding='utf-8' (Cleber)
* Rename devices() to get_devices() (Cleber)
* Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)
* Reword test_crash() docstring (Cleber)
* Reorder imports
---
 tests/acceptance/object_option.py | 53 +++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 tests/acceptance/object_option.py

Comments

Philippe Mathieu-Daudé Oct. 10, 2020, 7:54 a.m. UTC | #1 
On 10/9/20 10:29 PM, Eduardo Habkost wrote:
> Add a simple test case that will run QEMU directly (without QMP)

> just to check for crashes when using `-object`.

> 

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

> ---

> Changes v1 -> v2:

> * "Running command:" log message instead of "Command:" (Cleber)

> * Use universal_newlines=True instead of encoding='utf-8' (Cleber)

> * Rename devices() to get_devices() (Cleber)

> * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)

> * Reword test_crash() docstring (Cleber)

> * Reorder imports


Assuming:
Based-on: <20201008202713.1416823-1-ehabkost@redhat.com>

I get:

  (1/2) tests/acceptance/object_option.py:ObjectOption.test_help: 
qemu-system-avr: No machine specified, and there is no default
Use -machine help to list supported machines
FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) 
(0.19 s)
  (2/2) tests/acceptance/object_option.py:ObjectOption.test_crash: 
qemu-system-avr: No machine specified, and there is no default
Use -machine help to list supported machines
FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) 
(0.18 s)

> ---

>   tests/acceptance/object_option.py | 53 +++++++++++++++++++++++++++++++

>   1 file changed, 53 insertions(+)

>   create mode 100644 tests/acceptance/object_option.py

> 

> diff --git a/tests/acceptance/object_option.py b/tests/acceptance/object_option.py

> new file mode 100644

> index 0000000000..511c03a36f

> --- /dev/null

> +++ b/tests/acceptance/object_option.py

> @@ -0,0 +1,53 @@

> +# Copyright (c) 2020 Red Hat, Inc.

> +#

> +# Author:

> +#  Eduardo Habkost <ehabkost@redhat.com>

> +#

> +# 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 shlex

> +import subprocess

> +

> +import avocado

> +import avocado_qemu

> +

> +

> +class ObjectOption(avocado_qemu.Test):

> +    """Check if ``-object`` option behaves as expected"""

> +

> +    def run(self, cmd, *args, **kwargs):

> +        cmdstr = ' '.join(shlex.quote(c) for c in cmd)

> +        self.log.info("Running command: %s", cmdstr)

> +        return subprocess.run(cmd, universal_newlines=True, *args, **kwargs)

> +

> +    def get_devices(self):

> +        out = self.run([self.qemu_bin, '-object', 'help'],

> +                       check=True, stdout=subprocess.PIPE).stdout

> +        lines = out.split('\n')

> +        return [l.strip() for l in lines[1:] if l.strip()]

> +

> +    @avocado.fail_on(subprocess.CalledProcessError)

> +    def test_help(self):

> +        """Check if ``-object ...,help`` behaves as expected"""

> +        for device in self.get_devices():

> +            self.run([self.qemu_bin, '-object', '%s,help' % (device)],

> +                     check=True,

> +                     stdout=subprocess.DEVNULL)

> +

> +    @avocado.fail_on(subprocess.CalledProcessError)

> +    def test_crash(self):

> +        """Check that QEMU doesn't crash when using ``-object ...``"""

> +        for device in self.get_devices():

> +            r = self.run([self.qemu_bin, '-object',

> +                                '%s,id=obj0' % (device),

> +                                '-monitor', 'stdio'],

> +                         input='quit\n',

> +                         stdout=subprocess.DEVNULL,

> +                         stderr=subprocess.PIPE)

> +            if r.returncode not in (0, 1):

> +                self.log.warn("QEMU stderr: %s", r.stderr)

> +                self.log.warn("QEMU exit code: %d", r.returncode)

> +                if r.returncode < 0:

> +                    self.fail("QEMU crashed")

> +                else:

> +                    self.fail("Unexpected exit code")

>
Cleber Rosa Oct. 12, 2020, 3:18 a.m. UTC | #2 
On Sat, Oct 10, 2020 at 09:54:16AM +0200, Philippe Mathieu-Daudé wrote:
> On 10/9/20 10:29 PM, Eduardo Habkost wrote:
> > Add a simple test case that will run QEMU directly (without QMP)
> > just to check for crashes when using `-object`.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * "Running command:" log message instead of "Command:" (Cleber)
> > * Use universal_newlines=True instead of encoding='utf-8' (Cleber)
> > * Rename devices() to get_devices() (Cleber)
> > * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)
> > * Reword test_crash() docstring (Cleber)
> > * Reorder imports
> 
> Assuming:
> Based-on: <20201008202713.1416823-1-ehabkost@redhat.com>
> 
> I get:
> 
>  (1/2) tests/acceptance/object_option.py:ObjectOption.test_help:
> qemu-system-avr: No machine specified, and there is no default
> Use -machine help to list supported machines
> FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.19
> s)
>  (2/2) tests/acceptance/object_option.py:ObjectOption.test_crash:
> qemu-system-avr: No machine specified, and there is no default
> Use -machine help to list supported machines
> FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.18
> s)
>

Did you influence (test parameter?) the QEMU binary to be used?

Thanks,
- Cleber.
Eduardo Habkost Oct. 13, 2020, 5:46 p.m. UTC | #3 
On Sun, Oct 11, 2020 at 11:18:59PM -0400, Cleber Rosa wrote:
> On Sat, Oct 10, 2020 at 09:54:16AM +0200, Philippe Mathieu-Daudé wrote:
> > On 10/9/20 10:29 PM, Eduardo Habkost wrote:
> > > Add a simple test case that will run QEMU directly (without QMP)
> > > just to check for crashes when using `-object`.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > Changes v1 -> v2:
> > > * "Running command:" log message instead of "Command:" (Cleber)
> > > * Use universal_newlines=True instead of encoding='utf-8' (Cleber)
> > > * Rename devices() to get_devices() (Cleber)
> > > * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)
> > > * Reword test_crash() docstring (Cleber)
> > > * Reorder imports
> > 
> > Assuming:
> > Based-on: <20201008202713.1416823-1-ehabkost@redhat.com>
> > 
> > I get:
> > 
> >  (1/2) tests/acceptance/object_option.py:ObjectOption.test_help:
> > qemu-system-avr: No machine specified, and there is no default
> > Use -machine help to list supported machines
> > FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.19
> > s)
> >  (2/2) tests/acceptance/object_option.py:ObjectOption.test_crash:
> > qemu-system-avr: No machine specified, and there is no default
> > Use -machine help to list supported machines
> > FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.18
> > s)
> >
> 
> Did you influence (test parameter?) the QEMU binary to be used?

I'm assuming this was triggered by "make check-acceptance".  I
will change the test case to use '-machine none', which should
work on all architectures.
Cleber Rosa Oct. 13, 2020, 6:01 p.m. UTC | #4 
On Tue, Oct 13, 2020 at 01:46:11PM -0400, Eduardo Habkost wrote:
> On Sun, Oct 11, 2020 at 11:18:59PM -0400, Cleber Rosa wrote:

> > On Sat, Oct 10, 2020 at 09:54:16AM +0200, Philippe Mathieu-Daudé wrote:

> > > On 10/9/20 10:29 PM, Eduardo Habkost wrote:

> > > > Add a simple test case that will run QEMU directly (without QMP)

> > > > just to check for crashes when using `-object`.

> > > > 

> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

> > > > ---

> > > > Changes v1 -> v2:

> > > > * "Running command:" log message instead of "Command:" (Cleber)

> > > > * Use universal_newlines=True instead of encoding='utf-8' (Cleber)

> > > > * Rename devices() to get_devices() (Cleber)

> > > > * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)

> > > > * Reword test_crash() docstring (Cleber)

> > > > * Reorder imports

> > > 

> > > Assuming:

> > > Based-on: <20201008202713.1416823-1-ehabkost@redhat.com>

> > > 

> > > I get:

> > > 

> > >  (1/2) tests/acceptance/object_option.py:ObjectOption.test_help:

> > > qemu-system-avr: No machine specified, and there is no default

> > > Use -machine help to list supported machines

> > > FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.19

> > > s)

> > >  (2/2) tests/acceptance/object_option.py:ObjectOption.test_crash:

> > > qemu-system-avr: No machine specified, and there is no default

> > > Use -machine help to list supported machines

> > > FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.18

> > > s)

> > >

> > 

> > Did you influence (test parameter?) the QEMU binary to be used?

> 

> I'm assuming this was triggered by "make check-acceptance".  I


The output shows test 1/2 and 2/2, so I don't think it was triggered
by "make check-acceptance".

> will change the test case to use '-machine none', which should

> work on all architectures.

>


+1, sounds good!
- Cleber.

> -- 

> Eduardo
Philippe Mathieu-Daudé Oct. 26, 2020, 8:16 a.m. UTC | #5 
On 10/13/20 8:01 PM, Cleber Rosa wrote:
> On Tue, Oct 13, 2020 at 01:46:11PM -0400, Eduardo Habkost wrote:
>> On Sun, Oct 11, 2020 at 11:18:59PM -0400, Cleber Rosa wrote:
>>> On Sat, Oct 10, 2020 at 09:54:16AM +0200, Philippe Mathieu-Daudé wrote:
>>>> On 10/9/20 10:29 PM, Eduardo Habkost wrote:
>>>>> Add a simple test case that will run QEMU directly (without QMP)
>>>>> just to check for crashes when using `-object`.
>>>>>
>>>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>>>> ---
>>>>> Changes v1 -> v2:
>>>>> * "Running command:" log message instead of "Command:" (Cleber)
>>>>> * Use universal_newlines=True instead of encoding='utf-8' (Cleber)
>>>>> * Rename devices() to get_devices() (Cleber)
>>>>> * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)
>>>>> * Reword test_crash() docstring (Cleber)
>>>>> * Reorder imports
>>>>
>>>> Assuming:
>>>> Based-on: <20201008202713.1416823-1-ehabkost@redhat.com>
>>>>
>>>> I get:
>>>>
>>>>   (1/2) tests/acceptance/object_option.py:ObjectOption.test_help:
>>>> qemu-system-avr: No machine specified, and there is no default
>>>> Use -machine help to list supported machines
>>>> FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.19
>>>> s)
>>>>   (2/2) tests/acceptance/object_option.py:ObjectOption.test_crash:
>>>> qemu-system-avr: No machine specified, and there is no default
>>>> Use -machine help to list supported machines
>>>> FAIL: CalledProcessError(1, ['./qemu-system-avr', '-object', 'help']) (0.18
>>>> s)
>>>>
>>>
>>> Did you influence (test parameter?) the QEMU binary to be used?
>>
>> I'm assuming this was triggered by "make check-acceptance".  I
> 
> The output shows test 1/2 and 2/2, so I don't think it was triggered
> by "make check-acceptance".

I tested it in a build dir configured with:

   ../configure --target-list=avr-softmmu

I will test it again during the week.

> 
>> will change the test case to use '-machine none', which should
>> work on all architectures.
>>
> 
> +1, sounds good!
> - Cleber.
> 
>> -- 
>> Eduardo
>
Philippe Mathieu-Daudé Oct. 26, 2020, 8:17 a.m. UTC | #6 
Hi Eduardo, Thomas,

On 10/9/20 10:29 PM, Eduardo Habkost wrote:
> Add a simple test case that will run QEMU directly (without QMP)

> just to check for crashes when using `-object`.

> 

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

> ---

> Changes v1 -> v2:

> * "Running command:" log message instead of "Command:" (Cleber)

> * Use universal_newlines=True instead of encoding='utf-8' (Cleber)

> * Rename devices() to get_devices() (Cleber)

> * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)

> * Reword test_crash() docstring (Cleber)

> * Reorder imports

> ---

>   tests/acceptance/object_option.py | 53 +++++++++++++++++++++++++++++++

>   1 file changed, 53 insertions(+)

>   create mode 100644 tests/acceptance/object_option.py

> 

> diff --git a/tests/acceptance/object_option.py b/tests/acceptance/object_option.py

> new file mode 100644

> index 0000000000..511c03a36f

> --- /dev/null

> +++ b/tests/acceptance/object_option.py

> @@ -0,0 +1,53 @@

> +# Copyright (c) 2020 Red Hat, Inc.

> +#

> +# Author:

> +#  Eduardo Habkost <ehabkost@redhat.com>

> +#

> +# 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 shlex

> +import subprocess

> +

> +import avocado

> +import avocado_qemu

> +

> +

> +class ObjectOption(avocado_qemu.Test):

> +    """Check if ``-object`` option behaves as expected"""

> +

> +    def run(self, cmd, *args, **kwargs):

> +        cmdstr = ' '.join(shlex.quote(c) for c in cmd)

> +        self.log.info("Running command: %s", cmdstr)

> +        return subprocess.run(cmd, universal_newlines=True, *args, **kwargs)

> +

> +    def get_devices(self):

> +        out = self.run([self.qemu_bin, '-object', 'help'],

> +                       check=True, stdout=subprocess.PIPE).stdout

> +        lines = out.split('\n')

> +        return [l.strip() for l in lines[1:] if l.strip()]

> +

> +    @avocado.fail_on(subprocess.CalledProcessError)

> +    def test_help(self):

> +        """Check if ``-object ...,help`` behaves as expected"""

> +        for device in self.get_devices():

> +            self.run([self.qemu_bin, '-object', '%s,help' % (device)],

> +                     check=True,

> +                     stdout=subprocess.DEVNULL)

> +

> +    @avocado.fail_on(subprocess.CalledProcessError)

> +    def test_crash(self):

> +        """Check that QEMU doesn't crash when using ``-object ...``"""

> +        for device in self.get_devices():

> +            r = self.run([self.qemu_bin, '-object',

> +                                '%s,id=obj0' % (device),

> +                                '-monitor', 'stdio'],

> +                         input='quit\n',

> +                         stdout=subprocess.DEVNULL,

> +                         stderr=subprocess.PIPE)

> +            if r.returncode not in (0, 1):

> +                self.log.warn("QEMU stderr: %s", r.stderr)

> +                self.log.warn("QEMU exit code: %d", r.returncode)

> +                if r.returncode < 0:

> +                    self.fail("QEMU crashed")

> +                else:

> +                    self.fail("Unexpected exit code")

> 


Eduardo, what is the "acceptance" (functional) part of this test?

Thomas, could this be written using the QTest framework instead?

Thanks,

Phil.
Eduardo Habkost Oct. 26, 2020, 12:07 p.m. UTC | #7 
On Mon, Oct 26, 2020 at 09:17:57AM +0100, Philippe Mathieu-Daudé wrote:
> Hi Eduardo, Thomas,

> 

> On 10/9/20 10:29 PM, Eduardo Habkost wrote:

> > Add a simple test case that will run QEMU directly (without QMP)

> > just to check for crashes when using `-object`.

> > 

> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

> > ---

> > Changes v1 -> v2:

> > * "Running command:" log message instead of "Command:" (Cleber)

> > * Use universal_newlines=True instead of encoding='utf-8' (Cleber)

> > * Rename devices() to get_devices() (Cleber)

> > * Use @avocado.fail_on(subprocess.CalledProcessError) (Cleber)

> > * Reword test_crash() docstring (Cleber)

> > * Reorder imports

> > ---

> >   tests/acceptance/object_option.py | 53 +++++++++++++++++++++++++++++++

> >   1 file changed, 53 insertions(+)

> >   create mode 100644 tests/acceptance/object_option.py

> > 

> > diff --git a/tests/acceptance/object_option.py b/tests/acceptance/object_option.py

> > new file mode 100644

> > index 0000000000..511c03a36f

> > --- /dev/null

> > +++ b/tests/acceptance/object_option.py

> > @@ -0,0 +1,53 @@

> > +# Copyright (c) 2020 Red Hat, Inc.

> > +#

> > +# Author:

> > +#  Eduardo Habkost <ehabkost@redhat.com>

> > +#

> > +# 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 shlex

> > +import subprocess

> > +

> > +import avocado

> > +import avocado_qemu

> > +

> > +

> > +class ObjectOption(avocado_qemu.Test):

> > +    """Check if ``-object`` option behaves as expected"""

> > +

> > +    def run(self, cmd, *args, **kwargs):

> > +        cmdstr = ' '.join(shlex.quote(c) for c in cmd)

> > +        self.log.info("Running command: %s", cmdstr)

> > +        return subprocess.run(cmd, universal_newlines=True, *args, **kwargs)

> > +

> > +    def get_devices(self):

> > +        out = self.run([self.qemu_bin, '-object', 'help'],

> > +                       check=True, stdout=subprocess.PIPE).stdout

> > +        lines = out.split('\n')

> > +        return [l.strip() for l in lines[1:] if l.strip()]

> > +

> > +    @avocado.fail_on(subprocess.CalledProcessError)

> > +    def test_help(self):

> > +        """Check if ``-object ...,help`` behaves as expected"""

> > +        for device in self.get_devices():

> > +            self.run([self.qemu_bin, '-object', '%s,help' % (device)],

> > +                     check=True,

> > +                     stdout=subprocess.DEVNULL)

> > +

> > +    @avocado.fail_on(subprocess.CalledProcessError)

> > +    def test_crash(self):

> > +        """Check that QEMU doesn't crash when using ``-object ...``"""

> > +        for device in self.get_devices():

> > +            r = self.run([self.qemu_bin, '-object',

> > +                                '%s,id=obj0' % (device),

> > +                                '-monitor', 'stdio'],

> > +                         input='quit\n',

> > +                         stdout=subprocess.DEVNULL,

> > +                         stderr=subprocess.PIPE)

> > +            if r.returncode not in (0, 1):

> > +                self.log.warn("QEMU stderr: %s", r.stderr)

> > +                self.log.warn("QEMU exit code: %d", r.returncode)

> > +                if r.returncode < 0:

> > +                    self.fail("QEMU crashed")

> > +                else:

> > +                    self.fail("Unexpected exit code")

> > 

> 

> Eduardo, what is the "acceptance" (functional) part of this test?


It is testing an external interface (the command line), and I'm
pretty sure "not crashing when using -object with valid types" is
a functional requirement.

> 

> Thomas, could this be written using the QTest framework instead?


I'm sure it can, but why would we?

I don't think rewriting test cases in C is a good use of our
time.

-- 
Eduardo
diff mbox series

Patch

diff --git a/tests/acceptance/object_option.py b/tests/acceptance/object_option.py
new file mode 100644
index 0000000000..511c03a36f
--- /dev/null
+++ b/tests/acceptance/object_option.py
@@ -0,0 +1,53 @@ 
+# Copyright (c) 2020 Red Hat, Inc.
+#
+# Author:
+#  Eduardo Habkost <ehabkost@redhat.com>
+#
+# 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 shlex
+import subprocess
+
+import avocado
+import avocado_qemu
+
+
+class ObjectOption(avocado_qemu.Test):
+    """Check if ``-object`` option behaves as expected"""
+
+    def run(self, cmd, *args, **kwargs):
+        cmdstr = ' '.join(shlex.quote(c) for c in cmd)
+        self.log.info("Running command: %s", cmdstr)
+        return subprocess.run(cmd, universal_newlines=True, *args, **kwargs)
+
+    def get_devices(self):
+        out = self.run([self.qemu_bin, '-object', 'help'],
+                       check=True, stdout=subprocess.PIPE).stdout
+        lines = out.split('\n')
+        return [l.strip() for l in lines[1:] if l.strip()]
+
+    @avocado.fail_on(subprocess.CalledProcessError)
+    def test_help(self):
+        """Check if ``-object ...,help`` behaves as expected"""
+        for device in self.get_devices():
+            self.run([self.qemu_bin, '-object', '%s,help' % (device)],
+                     check=True,
+                     stdout=subprocess.DEVNULL)
+
+    @avocado.fail_on(subprocess.CalledProcessError)
+    def test_crash(self):
+        """Check that QEMU doesn't crash when using ``-object ...``"""
+        for device in self.get_devices():
+            r = self.run([self.qemu_bin, '-object',
+                                '%s,id=obj0' % (device),
+                                '-monitor', 'stdio'],
+                         input='quit\n',
+                         stdout=subprocess.DEVNULL,
+                         stderr=subprocess.PIPE)
+            if r.returncode not in (0, 1):
+                self.log.warn("QEMU stderr: %s", r.stderr)
+                self.log.warn("QEMU exit code: %d", r.returncode)
+                if r.returncode < 0:
+                    self.fail("QEMU crashed")
+                else:
+                    self.fail("Unexpected exit code")