[OE-core] [PATCH 19/20] oeqa.selftest.wic: Split configuration from code
Jose Lamego
jose.a.lamego at linux.intel.com
Tue Aug 9 15:20:12 UTC 2016
Hi Ed, thanks for your quick response.
On 08/09/2016 02:26 AM, Ed Bartosh wrote:
> Hi Jose,
>
> On Mon, Aug 08, 2016 at 09:23:07AM -0700, Jose Lamego wrote:
>> Improve oeqa-selftest capabilities and UX by placing
>> test configuration features and variables into a separate
>> configuration file.
>>
>
> Frankly I have mixed feelings about these changes. From the first look
> splitting configuration and code is a good idea. However, looking at the
> result I must say I feel like the test became less readable and harder
> to understand and maintain.
>
> May be the changes make sense for other tests, but for this one they make
> my life as a maintainer of this one less easy I'd say.
I totally understand you point. But before we drop the idea of this
change for wic, would there be a way to keep the code understandable
while leveraging on the configuration file to be able to easily try
different input/output combinations without touching the test code
(which is the basic purpose of the change)? Maybe renaming the conf
variables (see below proposals)?
If you still believe there would be no benefits from this change, it
will be better to skip it for wic.
>
>> [Yocto 9389]
>>
>> Signed-off-by: Jose Lamego <jose.a.lamego at linux.intel.com>
>> ---
>> meta/lib/oeqa/selftest/conf/wic.conf | 10 +++
>> meta/lib/oeqa/selftest/wic.py | 150 ++++++++++++++++++++---------------
>> 2 files changed, 98 insertions(+), 62 deletions(-)
>> create mode 100644 meta/lib/oeqa/selftest/conf/wic.conf
>>
>> diff --git a/meta/lib/oeqa/selftest/conf/wic.conf b/meta/lib/oeqa/selftest/conf/wic.conf
>> new file mode 100644
>> index 0000000..e2afb8d
>> --- /dev/null
>> +++ b/meta/lib/oeqa/selftest/conf/wic.conf
>> @@ -0,0 +1,10 @@
>> +[Wic]
>> +setUpClass_image1 = core-image-minimal
>> +setUpClass_image2 = minimal
> setUpClass_image2 is not used in the code.
>
>> +setUpClass_image3 = qemux86-directdisk
>> +setUpClass_image4 = directdisk
>> +setUpClass_image5 = wic-image-minimal
> setUpClass_image1 is an image produced by bitbake, but setUpClass_image3
> is a name of wic image. They're not the same. Bitbake image is an input
> for wic and wic image is an output. setUpClass_image5 is again the name
> of the bitbake target, but it differs from _image3. Naming doesn't reflect this fact.
setUpClass_image1 could be renamed to setUpClass_bitbakeImage1
setUpClass_image3 could be renamed to setClass_wicImage
setUpClass_image5 to setUpClass_bitbakeImage2
>
>
>> +setUpClass_features = IMAGE_FSTYPES += " hddimg"
>> + MACHINE_FEATURES_append = " efi"
>> +setUpClass_recipes = syslinux syslinux-native parted-native gptfdisk-native dosfstools-native mtools-native bmap-tools-native
> This list is used only once in the test. Moving it to config file makes
> it longer to understand what's in the list.
>
>> diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
>> index e550785..dc81457 100644
>> --- a/meta/lib/oeqa/selftest/wic.py
>> +++ b/meta/lib/oeqa/selftest/wic.py
>> @@ -31,6 +31,7 @@ from shutil import rmtree
>> from oeqa.selftest.base import oeSelfTest
>> from oeqa.utils.commands import runCmd, bitbake, get_bb_var, runqemu
>> from oeqa.utils.decorators import testcase
>> +from oeqa.utils.readconfig import conffile
>>
>>
>> class Wic(oeSelfTest):
>> @@ -39,6 +40,18 @@ class Wic(oeSelfTest):
>> resultdir = "/var/tmp/wic/build/"
>> image_is_ready = False
>>
>> + @classmethod
>> + def setUpClass(cls):
>> + # Get test configurations from configuration file
>> + cls.config = conffile(__file__)
>> + cls.image1 = cls.config.get('Wic', 'setUpClass_image1')
>> + cls.image2 = cls.config.get('Wic', 'setUpClass_image2')
>> + cls.image3 = cls.config.get('Wic', 'setUpClass_image3')
>> + cls.image4 = cls.config.get('Wic', 'setUpClass_image4')
>> + cls.image5 = cls.config.get('Wic', 'setUpClass_image5')
>> + cls.features = cls.config.get('Wic', 'setUpClass_features')
>> + cls.recipes = cls.config.get('Wic', 'setUpClass_recipes')
> This looks like a duplication of code and configuration and also hides real
> image names and features making test much less readable.
>
>> +
>> def setUpLocal(self):
>> """This code is executed before each test method."""
>> self.write_config('IMAGE_FSTYPES += " hddimg"\n'
>> @@ -48,9 +61,8 @@ class Wic(oeSelfTest):
>> # clean up which can result in the native tools built earlier in
>> # setUpClass being unavailable.
>> if not Wic.image_is_ready:
>> - bitbake('syslinux syslinux-native parted-native gptfdisk-native '
>> - 'dosfstools-native mtools-native bmap-tools-native')
>> - bitbake('core-image-minimal')
>> + bitbake(self.recipes)
>> + bitbake(self.image1)
> I'd really prefer to see list of dependencies and the image name right
> in the test code without having to look into configuration file.
Could this dependency list be renamed to something that reflects its
nature? something like "nativeDependencies"? So a different dependency
group could be used from the conf file.
>
> The same is true for pretty much all the changes.
>
>> Wic.image_is_ready = True
>>
>> rmtree(self.resultdir, ignore_errors=True)
>> @@ -73,30 +85,35 @@ class Wic(oeSelfTest):
>> @testcase(1211)
>> def test_build_image_name(self):
>> """Test wic create directdisk --image-name core-image-minimal"""
>> - self.assertEqual(0, runCmd("wic create directdisk "
>> - "--image-name core-image-minimal").status)
>> - self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
>> + self.assertEqual(
>> + 0, runCmd("wic create %s --image-name %s"
>> + % (self.image4, self.image1)).status)
>> + self.assertEqual(
>> + 1, len(glob(self.resultdir + "%s-*.direct" % self.image4)))
>>
>> @testcase(1212)
>> def test_build_artifacts(self):
>> """Test wic create directdisk providing all artifacts."""
>> - bbvars = dict((var.lower(), get_bb_var(var, 'core-image-minimal')) \
>> - for var in ('STAGING_DATADIR', 'DEPLOY_DIR_IMAGE',
>> - 'STAGING_DIR_NATIVE', 'IMAGE_ROOTFS'))
>> + bbvars = dict((var.lower(), get_bb_var(var, self.image1))
>> + for var in ('STAGING_DATADIR', 'DEPLOY_DIR_IMAGE',
>> + 'STAGING_DIR_NATIVE', 'IMAGE_ROOTFS'))
>> status = runCmd("wic create directdisk "
>> "-b %(staging_datadir)s "
>> "-k %(deploy_dir_image)s "
>> "-n %(staging_dir_native)s "
>> "-r %(image_rootfs)s" % bbvars).status
>> self.assertEqual(0, status)
>> - self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
>> + self.assertEqual(1, len(glob(self.resultdir + "%s-*.direct"
>> + % self.image4)))
>>
>> @testcase(1157)
>> def test_gpt_image(self):
>> - """Test creation of core-image-minimal with gpt table and UUID boot"""
>> - self.assertEqual(0, runCmd("wic create directdisk-gpt "
>> - "--image-name core-image-minimal").status)
>> - self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
>> + """Test creation of %s with gpt table and UUID boot""" % self.image1
>> + self.assertEqual(0, runCmd("wic create %s-gpt "
>> + "--image-name %s"
>> + % (self.image4, self.image1)).status)
>> + self.assertEqual(1, len(glob(self.resultdir + "%s-*.direct"
>> + % self.image4)))
>>
>> @testcase(1213)
>> def test_unsupported_subcommand(self):
>> @@ -127,55 +144,63 @@ class Wic(oeSelfTest):
>> @testcase(1264)
>> def test_compress_gzip(self):
>> """Test compressing an image with gzip"""
>> - self.assertEqual(0, runCmd("wic create directdisk "
>> - "--image-name core-image-minimal "
>> - "-c gzip").status)
>> - self.assertEqual(1, len(glob(self.resultdir + \
>> - "directdisk-*.direct.gz")))
>> + self.assertEqual(0, runCmd("wic create %s "
>> + "--image-name %s "
>> + "-c gzip"
>> + % (self.image4, self.image1)).status)
>> + self.assertEqual(1, len(glob(
>> + self.resultdir + "%s-*.direct.gz"
>> + % self.image4)))
>>
>> @testcase(1265)
>> def test_compress_bzip2(self):
>> """Test compressing an image with bzip2"""
>> - self.assertEqual(0, runCmd("wic create directdisk "
>> - "--image-name core-image-minimal "
>> - "-c bzip2").status)
>> - self.assertEqual(1, len(glob(self.resultdir + \
>> - "directdisk-*.direct.bz2")))
>> + self.assertEqual(0, runCmd("wic create %s "
>> + "--image-name %s "
>> + "-c bzip2"
>> + % (self.image4, self.image1)).status)
>> + self.assertEqual(1, len(glob(
>> + self.resultdir + "%s-*.direct.bz2"
>> + % self.image4)))
>>
>> @testcase(1266)
>> def test_compress_xz(self):
>> """Test compressing an image with xz"""
>> - self.assertEqual(0, runCmd("wic create directdisk "
>> - "--image-name core-image-minimal "
>> - "-c xz").status)
>> - self.assertEqual(1, len(glob(self.resultdir + \
>> - "directdisk-*.direct.xz")))
>> + self.assertEqual(0, runCmd("wic create %s "
>> + "--image-name %s "
>> + "-c xz"
>> + % (self.image4, self.image1)).status)
>> + self.assertEqual(1, len(glob(
>> + self.resultdir + "%s-*.direct.xz"
>> + % self.image4)))
>>
>> @testcase(1267)
>> def test_wrong_compressor(self):
>> """Test how wic breaks if wrong compressor is provided"""
>> - self.assertEqual(2, runCmd("wic create directdisk "
>> - "--image-name core-image-minimal "
>> - "-c wrong", ignore_status=True).status)
>> + self.assertEqual(2, runCmd("wic create %s "
>> + "--image-name %s "
>> + "-c wrong" % (self.image4, self.image1),
>> + ignore_status=True).status)
>>
>> @testcase(1268)
>> def test_rootfs_indirect_recipes(self):
>> """Test usage of rootfs plugin with rootfs recipes"""
>> wks = "directdisk-multi-rootfs"
>> self.assertEqual(0, runCmd("wic create %s "
>> - "--image-name core-image-minimal "
>> - "--rootfs rootfs1=core-image-minimal "
>> - "--rootfs rootfs2=core-image-minimal" \
>> - % wks).status)
>> + "--image-name %s "
>> + "--rootfs rootfs1=%s "
>> + "--rootfs rootfs2=%s"
>> + % (wks, self.image1, self.image1,
>> + self.image1)).status)
>> self.assertEqual(1, len(glob(self.resultdir + "%s*.direct" % wks)))
>>
>> @testcase(1269)
>> def test_rootfs_artifacts(self):
>> """Test usage of rootfs plugin with rootfs paths"""
>> - bbvars = dict((var.lower(), get_bb_var(var, 'core-image-minimal')) \
>> - for var in ('STAGING_DATADIR', 'DEPLOY_DIR_IMAGE',
>> - 'STAGING_DIR_NATIVE', 'IMAGE_ROOTFS'))
>> - bbvars['wks'] = "directdisk-multi-rootfs"
>> + bbvars = dict((var.lower(), get_bb_var(var, self.image1))
>> + for var in ('STAGING_DATADIR', 'DEPLOY_DIR_IMAGE',
>> + 'STAGING_DIR_NATIVE', 'IMAGE_ROOTFS'))
>> + bbvars['wks'] = "%s-multi-rootfs" % self.image4
>> status = runCmd("wic create %(wks)s "
>> "-b %(staging_datadir)s "
>> "-k %(deploy_dir_image)s "
>> @@ -191,14 +216,14 @@ class Wic(oeSelfTest):
>> def test_iso_image(self):
>> """Test creation of hybrid iso image with legacy and EFI boot"""
>> self.assertEqual(0, runCmd("wic create mkhybridiso "
>> - "--image-name core-image-minimal").status)
>> + "--image-name %s" % self.image1).status)
>> self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.direct")))
>> self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.iso")))
>>
>> @testcase(1347)
>> def test_image_env(self):
>> """Test generation of <image>.env files."""
>> - image = 'core-image-minimal'
>> + image = self.image1
>> self.assertEqual(0, bitbake('%s -c do_rootfs_wicenv' % image).status)
>> stdir = get_bb_var('STAGING_DIR_TARGET', image)
>> imgdatadir = os.path.join(stdir, 'imgdata')
>> @@ -222,11 +247,11 @@ class Wic(oeSelfTest):
>> @testcase(1351)
>> def test_wic_image_type(self):
>> """Test building wic images by bitbake"""
>> - self.assertEqual(0, bitbake('wic-image-minimal').status)
>> + self.assertEqual(0, bitbake(self.image5).status)
>>
>> deploy_dir = get_bb_var('DEPLOY_DIR_IMAGE')
>> machine = get_bb_var('MACHINE')
>> - prefix = os.path.join(deploy_dir, 'wic-image-minimal-%s.' % machine)
>> + prefix = os.path.join(deploy_dir, '%s-%s.' % (self.image5, machine))
>> # check if we have result image and manifests symlinks
>> # pointing to existing files
>> for suffix in ('wic', 'manifest'):
>> @@ -236,42 +261,42 @@ class Wic(oeSelfTest):
>>
>> @testcase(1348)
>> def test_qemux86_directdisk(self):
>> - """Test creation of qemux-86-directdisk image"""
>> - image = "qemux86-directdisk"
>> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
>> - % image).status)
>> - self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>> + """Test creation of %s image""" % self.image3
>> + self.assertEqual(0, runCmd("wic create %s -e %s"
>> + % (self.image3, self.image1)).status)
>> + self.assertEqual(1, len(glob(self.resultdir + "%s-*direct"
>> + % self.image3)))
>>
>> @testcase(1349)
>> def test_mkgummidisk(self):
>> """Test creation of mkgummidisk image"""
>> image = "mkgummidisk"
>> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
>> - % image).status)
>> + self.assertEqual(0, runCmd("wic create %s -e %s"
>> + % (image, self.image1)).status)
>> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>>
>> @testcase(1350)
>> def test_mkefidisk(self):
>> """Test creation of mkefidisk image"""
>> image = "mkefidisk"
>> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
>> - % image).status)
>> + self.assertEqual(0, runCmd("wic create %s -e %s"
>> + % (image, self.image1)).status)
>> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>>
>> @testcase(1385)
>> def test_directdisk_bootloader_config(self):
>> - """Test creation of directdisk-bootloader-config image"""
>> - image = "directdisk-bootloader-config"
>> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
>> - % image).status)
>> + """Test creation of %s-bootloader-config image""" % self.image4
>> + image = "%s-bootloader-config" % self.image4
>> + self.assertEqual(0, runCmd("wic create %s -e %s"
>> + % (image, self.image1)).status)
>> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>>
>> @testcase(1422)
>> def test_qemu(self):
>> - """Test wic-image-minimal under qemu"""
>> - self.assertEqual(0, bitbake('wic-image-minimal').status)
>> + """Test %s under qemu""" % self.image5
>> + self.assertEqual(0, bitbake(self.image5).status)
>>
>> - with runqemu('wic-image-minimal', ssh=False) as qemu:
>> + with runqemu(self.image5, ssh=False) as qemu:
>> command = "mount |grep '^/dev/' | cut -f1,3 -d ' '"
>> status, output = qemu.run_serial(command)
>> self.assertEqual(1, status, 'Failed to run command "%s": %s' % (command, output))
>> @@ -279,8 +304,9 @@ class Wic(oeSelfTest):
>>
>> def test_bmap(self):
>> """Test generation of .bmap file"""
>> - image = "directdisk"
>> - status = runCmd("wic create %s -e core-image-minimal --bmap" % image).status
>> + image = self.image4
>> + status = runCmd("wic create %s -e %s --bmap"
>> + % (image, self.image1)).status
>> self.assertEqual(0, status)
>> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct.bmap" % image)))
>> --
>> 1.8.3.1
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core at lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
--
Jose Lamego | OTC Embedded Platforms & Tools | GDC
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openembedded.org/pipermail/openembedded-core/attachments/20160809/d2aaa36a/attachment-0002.sig>
More information about the Openembedded-core
mailing list