Review: Needs Fixing
Can you drop the missing Bionic/Focal ones and add them in a separate PR?
I added a few spots where I asked if we should move a unittest into a base
class and iterate skip any releases that don't support it so we don't have to
duplicate the unittest in each release class.
There are a few places I didn't mention that; some of them are class settings;
like disk_block_size = 4096, I think those are fine to duplicate though if we
could make a 4k base class and subclass the users of that; it'd be a nice
cleanup.
Diff comments:
> diff --git a/tests/vmtests/test_lvm_raid.py b/tests/vmtests/test_lvm_raid.py
> index cc1afa1..5fe7993 100644
> --- a/tests/vmtests/test_lvm_raid.py
> +++ b/tests/vmtests/test_lvm_raid.py
> @@ -47,7 +47,7 @@ class TestLvmOverRaidAbs(TestMdadmAbs, TestLvmAbs):
> return self._test_pvs(dname_to_vg)
>
>
> -class FocalTestLvmOverRaid(relbase.focal, TestLvmOverRaidAbs):
> +class XenialGATestLvmOverRaid(relbase.xenial_ga, TestLvmOverRaidAbs):
This is a strange re-ordering; was this manual?
> __test__ = True
>
>
> diff --git a/tests/vmtests/test_mdadm_bcache.py
> b/tests/vmtests/test_mdadm_bcache.py
> index 4f38e7a..ac8af3f 100644
> --- a/tests/vmtests/test_mdadm_bcache.py
> +++ b/tests/vmtests/test_mdadm_bcache.py
> @@ -333,6 +346,16 @@ class FocalTestMirrorbootPartitionsUEFI(relbase.focal,
> self.assertEqual(primary_esp, backup_esp)
>
>
> +class GroovyTestMirrorbootPartitionsUEFI(relbase.groovy,
> + TestMirrorbootPartitionsUEFIAbs):
> + __test__ = True
> +
> + def test_backup_esp_matches_primary(self):
> + primary_esp = self.load_collect_file("diska-part1-efi.out")
> + backup_esp = self.load_collect_file("diskb-part1-efi.out")
> + self.assertEqual(primary_esp, backup_esp)
I wonder if we should move this to the TestMirrorbootPartitionsUEFIAbs class
and have it SkipTest if not release in ['focal', 'groovy'];
I much prefer to have a single unittest than a copy-paste from
release to release.
> +
> +
> class TestRaid5bootAbs(TestMdadmAbs):
> # alternative config for more complex setup
> conf_file = "examples/tests/raid5boot.yaml"
> diff --git a/tests/vmtests/test_network_mtu.py
> b/tests/vmtests/test_network_mtu.py
> index 71f87ca..4bed78a 100644
> --- a/tests/vmtests/test_network_mtu.py
> +++ b/tests/vmtests/test_network_mtu.py
> @@ -191,6 +191,11 @@ class FocalTestNetworkMtu(relbase.focal,
> TestNetworkMtuAbs):
> __test__ = True
>
>
> +class GroovyTestNetworkMtu(relbase.groovy, TestNetworkMtuAbs):
> + conf_file = "examples/tests/network_mtu_networkd.yaml"
Hrm, do we not have a class for just network_mtu_networkd that multiple release
classes can subclass from?
> + __test__ = True
> +
> +
> class Centos66TestNetworkMtu(centos_relbase.centos66_xenial,
> CentosTestNetworkMtuAbs):
> __test__ = True
--
https://code.launchpad.net/~paride/curtin/+git/curtin/+merge/387865
Your team curtin developers is requested to review the proposed merge of
~paride/curtin:vmtests-add-groovy into curtin:master.
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
