Message ID | 20201102144245.2134077-4-philmd@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | tests/acceptance: Only run tests tagged 'gating-ci' on GitLab CI | expand |
On 11/2/20 3:42 PM, Philippe Mathieu-Daudé wrote: > Prefer skipping incomplete tests with the "@skip" keyword, > rather than commenting the code. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@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 And I forgot: -- >8 -- @@ -14,6 +14,7 @@ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) from qemu.machine import QEMUMachine from avocado_qemu import Test +from avocado import skip # Virtio Device IDs: VIRTIO_NET = 1 --- > @@ -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") > + 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) > > def check_modern_only(self, qemu_devtype, virtio_devid): > """Check if a modern-only virtio device type behaves as expected""" >
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é <philmd@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' ? > + 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. Thomas
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é <philmd@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?
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é <philmd@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
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") + 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) def check_modern_only(self, qemu_devtype, virtio_devid): """Check if a modern-only virtio device type behaves as expected"""
Prefer skipping incomplete tests with the "@skip" keyword, rather than commenting the code. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- tests/acceptance/virtio_version.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)