On 04/11/2020 12.27, Philippe Mathieu-Daudé wrote:
> On 11/4/20 12:13 PM, Thomas Huth wrote:
>> On 02/11/2020 15.42, Philippe Mathieu-Daudé wrote:
>>> Prefer skipping incomplete tests with the "@skip" keyword,
>>> rather than commenting the code.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>>> ---
>>> tests/acceptance/virtio_version.py | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/acceptance/virtio_version.py
>>> b/tests/acceptance/virtio_version.py
>>> index 33593c29dd0..187bbfa1f42 100644
>>> --- a/tests/acceptance/virtio_version.py
>>> +++ b/tests/acceptance/virtio_version.py
>>> @@ -140,17 +140,20 @@ def check_all_variants(self, qemu_devtype,
>>> virtio_devid):
>>> self.assertIn('conventional-pci-device', trans_ifaces)
>>> self.assertNotIn('pci-express-device', trans_ifaces)
>>>
>>> + @skip("virtio-blk requires 'driver' parameter")
>>
>> Shouldn't that be 'drive' instead of 'driver' ?
>
> No clue, this is the previous commented line.
>
>>
>>> + def test_conventional_devs_driver(self):
>>> + self.check_all_variants('virtio-blk-pci', VIRTIO_BLOCK)
>>> +
>>> + @skip("virtio-9p requires 'fsdev' parameter")
>>> + def test_conventional_devs_fsdev(self):
>>> + self.check_all_variants('virtio-9p-pci', VIRTIO_9P)
>>>
>>> def test_conventional_devs(self):
>>> self.check_all_variants('virtio-net-pci', VIRTIO_NET)
>>> - # virtio-blk requires 'driver' parameter
>>> - #self.check_all_variants('virtio-blk-pci', VIRTIO_BLOCK)
>>> self.check_all_variants('virtio-serial-pci', VIRTIO_CONSOLE)
>>> self.check_all_variants('virtio-rng-pci', VIRTIO_RNG)
>>> self.check_all_variants('virtio-balloon-pci', VIRTIO_BALLOON)
>>> self.check_all_variants('virtio-scsi-pci', VIRTIO_SCSI)
>>> - # virtio-9p requires 'fsdev' parameter
>>> - #self.check_all_variants('virtio-9p-pci', VIRTIO_9P)
>>
>> I think I'd prefer to keep the stuff commented ... otherwise it will show up
>> in the logs, giving the impression that you could run the tests somehow if
>> you just provided the right environment, which is just not possible right
>> now.
>
> Well, we usually don't commit commented code like that.
> If it is committed, it is important, then it has to show up in
> the log. If you don't want it logged, why not remove it then?
Then I'd rather remove it. Or leave a comment saying "we cannot exercise
virtio-9p-pci and virtio-blk-pci here (yet) since they need additional
parameters to be set".
Thomas
