Re: [PATCH v2 4/4] tests/functional/test_vmstate: Test whether the checker script works as expected

[Thomas Huth <thuth@redhat.com>]

On 22/05/2025 16.40, Peter Xu wrote:
> On Thu, May 22, 2025 at 03:37:56PM +0200, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> We've got two vmstate dump files in the repository which are meant
>> for verifying whether the vmstate-static-checker.py works as expected.
>> Since running this manually is a cumbersome job, let's add an automated
>> test for this instead that runs the script with the two dump files
>> and checks for the expected output.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/functional/test_vmstate.py | 37 ++++++++++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/tests/functional/test_vmstate.py b/tests/functional/test_vmstate.py
>> index cc988987481..19a62e8a17e 100755
>> --- a/tests/functional/test_vmstate.py
>> +++ b/tests/functional/test_vmstate.py
>> @@ -9,6 +9,22 @@
>>   from qemu_test import QemuSystemTest
>>   
>>   
>> +expected_output='''Warning: checking incompatible machine types: "pc-i440fx-2.1", "pc-i440fx-2.2"
>> +Section "fw_cfg" does not exist in dest
>> +Section "fusbh200-ehci-usb" version error: 2 > 1
>> +Section "fusbh200-ehci-usb", Description "ehci-core": expected field "usbsts", got "usbsts_pending"; skipping rest
>> +Section "pci-serial-4x" Description "pci-serial-multi": Entry "Fields" missing
>> +Section "intel-hda-generic", Description "intel-hda", Field "pci": missing description
>> +Section "cfi.pflash01": Entry "Description" missing
>> +Section "megasas", Description "PCIDevice": expected field "irq_state", while dest has no further fields
>> +Section "PIIX3-xen" Description "PIIX3": minimum version error: 1 < 2
>> +Section "PIIX3-xen" Description "PIIX3": Entry "Subsections" missing
>> +Section "tpci200": Description "tpci200" missing, got "tpci2002" instead; skipping
>> +Section "sun-fdtwo" Description "fdc": version error: 2 > 1
>> +Section "sun-fdtwo", Description "fdrive": Subsection "fdrive/media_rate" not found
>> +Section "usb-kbd" Description "usb-kbd" Field "kbd.keycodes" size mismatch: 4 , 2
>> +'''
>> +
>>   class VmStateTest(QemuSystemTest):
>>   
>>       def run_vmstate_checker(self, src_json, dst_json):
>> @@ -19,6 +35,27 @@ def run_vmstate_checker(self, src_json, dst_json):
>>                                 stderr=subprocess.STDOUT,
>>                                 text=True)
>>   
>> +    def test_checker(self):
>> +        """
>> +        Test whether the checker script correctly detects the changes
>> +        between dump1.json and dump2.json.
>> +        """
>> +        if self.arch != 'x86_64':
>> +            self.skipTest('for x86 only')
>> +        src_json = self.data_file('..', 'data', 'vmstate-static-checker',
>> +                                  'dump1.json')
>> +        dst_json = self.data_file('..', 'data', 'vmstate-static-checker',
>> +                                  'dump2.json')
>> +        self.log.info(f'Comparing {src_json} with {dst_json}')
>> +        cp = self.run_vmstate_checker(src_json, dst_json)
>> +        if cp.returncode != 13:
>> +            self.fail('Unexpected return code of vmstate-static-checker: ' +
>> +                      cp.returncode)
>> +        if cp.stdout != expected_output:
>> +            self.log.info('vmstate-static-checker output:\n' + cp.stdout)
>> +            self.log.info('expected output:\n' + expected_output)
>> +            self.fail('Unexpected vmstate-static-checker output!')
>> +
> 
> Would this run for every make check and every CI run?

Yes.

> Since the script almost never change, I wonder whether this test should be
> triggered only if manually.

We could disable it by default with a @skip... decorator, but I think it's 
not really worth the effort: The runtime of test_checker() is about 100 ms, 
that's really nothing compared to the hunders of seconds that some other 
tests take.

  Thomas