* [PATCH] selftest/wic: extending test coverage for WIC script options
@ 2016-12-13 15:53 Jair Gonzalez
2016-12-13 20:17 ` Ed Bartosh
0 siblings, 1 reply; 4+ messages in thread
From: Jair Gonzalez @ 2016-12-13 15:53 UTC (permalink / raw)
To: openembedded-core
The previous WIC script selftest didn't cover all of its command
line options. The following test cases were added to complete
covering them:
1552 Test wic --version
1553 Test wic help create
1554 Test wic help list
1555 Test wic list images
1556 Test wic list source-plugins
1557 Test wic listed images help
1558 Test wic debug, skip-build-check and build_rootfs
1559 Test image vars directory selection
1562 Test alternate output directory
In addition, the following test cases were assigned an ID number on
Testopia:
1560 Test creation of systemd-bootdisk image
1561 Test creation of sdimage-bootpart image
Finally, part of the test methods were rearranged to group them by
functionality, and some cleanup was made to improve the code's
compliance with PEP8 style guide.
Fixes [YOCTO 10594]
Signed-off-by: Jair Gonzalez <jair.de.jesus.gonzalez.plascencia@linux.intel.com>
---
meta/lib/oeqa/selftest/wic.py | 246 +++++++++++++++++++++++++++++-------------
1 file changed, 174 insertions(+), 72 deletions(-)
diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
index e652fad..46bfb94 100644
--- a/meta/lib/oeqa/selftest/wic.py
+++ b/meta/lib/oeqa/selftest/wic.py
@@ -37,13 +37,13 @@ class Wic(oeSelfTest):
"""Wic test class."""
resultdir = "/var/tmp/wic/build/"
+ alternate_resultdir = "/var/tmp/wic/build/alt/"
image_is_ready = False
def setUpLocal(self):
"""This code is executed before each test method."""
self.write_config('IMAGE_FSTYPES += " hddimg"\n'
- 'MACHINE_FEATURES_append = " efi"\n'
- 'WKS_FILE = "wic-image-minimal"\n')
+ 'MACHINE_FEATURES_append = " efi"\n')
# Do this here instead of in setUpClass as the base setUp does some
# clean up which can result in the native tools built earlier in
@@ -56,10 +56,16 @@ class Wic(oeSelfTest):
rmtree(self.resultdir, ignore_errors=True)
+ @testcase(1552)
+ def test_version(self):
+ """Test wic --version"""
+ self.assertEqual(0, runCmd('wic --version').status)
+
@testcase(1208)
def test_help(self):
- """Test wic --help"""
+ """Test wic --help and wic -h"""
self.assertEqual(0, runCmd('wic --help').status)
+ self.assertEqual(0, runCmd('wic -h').status)
@testcase(1209)
def test_createhelp(self):
@@ -71,19 +77,74 @@ class Wic(oeSelfTest):
"""Test wic list --help"""
self.assertEqual(0, runCmd('wic list --help').status)
+ @testcase(1553)
+ def test_help_create(self):
+ """Test wic help create"""
+ self.assertEqual(0, runCmd('wic help create').status)
+
+ @testcase(1554)
+ def test_help_list(self):
+ """Test wic help list"""
+ self.assertEqual(0, runCmd('wic help list').status)
+
+ @testcase(1215)
+ def test_help_overview(self):
+ """Test wic help overview"""
+ self.assertEqual(0, runCmd('wic help overview').status)
+
+ @testcase(1216)
+ def test_help_plugins(self):
+ """Test wic help plugins"""
+ self.assertEqual(0, runCmd('wic help plugins').status)
+
+ @testcase(1217)
+ def test_help_kickstart(self):
+ """Test wic help kickstart"""
+ self.assertEqual(0, runCmd('wic help kickstart').status)
+
+ @testcase(1555)
+ def test_list_images(self):
+ """Test wic list images"""
+ self.assertEqual(0, runCmd('wic list images').status)
+
+ @testcase(1556)
+ def test_list_source_plugins(self):
+ """Test wic list source-plugins"""
+ self.assertEqual(0, runCmd('wic list source-plugins').status)
+
+ @testcase(1557)
+ def test_listed_images_help(self):
+ """Test wic listed images help"""
+ output = runCmd('wic list images').output
+ imageDetails = [line.split() for line in output.split('\n')]
+ imageList = [row[0] for row in imageDetails]
+ for image in imageList:
+ self.assertEqual(0, runCmd('wic list %s help' % image).status)
+
+ @testcase(1213)
+ def test_unsupported_subcommand(self):
+ """Test unsupported subcommand"""
+ self.assertEqual(1, runCmd('wic unsupported',
+ ignore_status=True).status)
+
+ @testcase(1214)
+ def test_no_command(self):
+ """Test wic without command"""
+ self.assertEqual(1, runCmd('wic', ignore_status=True).status)
+
@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)
+ "--image-name=core-image-minimal").status)
self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
@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, 'core-image-minimal'))
+ 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 "
@@ -96,113 +157,110 @@ class Wic(oeSelfTest):
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)
+ "--image-name core-image-minimal "
+ ).status)
self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
- @testcase(1213)
- def test_unsupported_subcommand(self):
- """Test unsupported subcommand"""
- self.assertEqual(1, runCmd('wic unsupported',
- ignore_status=True).status)
-
- @testcase(1214)
- def test_no_command(self):
- """Test wic without command"""
- self.assertEqual(1, runCmd('wic', ignore_status=True).status)
-
- @testcase(1215)
- def test_help_overview(self):
- """Test wic help overview"""
- self.assertEqual(0, runCmd('wic help overview').status)
-
- @testcase(1216)
- def test_help_plugins(self):
- """Test wic help plugins"""
- self.assertEqual(0, runCmd('wic help plugins').status)
-
- @testcase(1217)
- def test_help_kickstart(self):
- """Test wic help kickstart"""
- self.assertEqual(0, runCmd('wic help kickstart').status)
-
@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 "
+ "-e core-image-minimal "
"-c gzip").status)
- self.assertEqual(1, len(glob(self.resultdir + \
- "directdisk-*.direct.gz")))
+ self.assertEqual(1, len(glob(self.resultdir +
+ "directdisk-*.direct.gz")))
@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 "
+ "--image-name=core-image-minimal "
"-c bzip2").status)
- self.assertEqual(1, len(glob(self.resultdir + \
- "directdisk-*.direct.bz2")))
+ self.assertEqual(1, len(glob(self.resultdir +
+ "directdisk-*.direct.bz2")))
@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")))
+ "--image-name=core-image-minimal "
+ "--compress-with=xz").status)
+ self.assertEqual(1, len(glob(self.resultdir +
+ "directdisk-*.direct.xz")))
@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 "
+ "--image-name=core-image-minimal "
"-c wrong", ignore_status=True).status)
+ @testcase(1558)
+ def test_debug_skip_build_check_and_build_rootfs(self):
+ """Test wic debug, skip-build-check and build_rootfs"""
+ self.assertEqual(0, runCmd("wic create directdisk "
+ "--image-name=core-image-minimal "
+ "-D -s -f").status)
+ self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
+ self.assertEqual(0, runCmd("wic create directdisk "
+ "--image-name=core-image-minimal "
+ "--debug "
+ "--skip-build-check "
+ "--build-rootfs").status)
+ self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
+
@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 "
+ "--image-name=core-image-minimal "
"--rootfs rootfs1=core-image-minimal "
- "--rootfs rootfs2=core-image-minimal" \
+ "--rootfs rootfs2=core-image-minimal"
% wks).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 = 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"
status = runCmd("wic create %(wks)s "
- "-b %(staging_datadir)s "
- "-k %(deploy_dir_image)s "
- "-n %(staging_dir_native)s "
+ "--bootimg-dir=%(staging_datadir)s "
+ "--kernel-dir=%(deploy_dir_image)s "
+ "--native-sysroot=%(staging_dir_native)s "
"--rootfs-dir rootfs1=%(image_rootfs)s "
- "--rootfs-dir rootfs2=%(image_rootfs)s" \
+ "--rootfs-dir rootfs2=%(image_rootfs)s"
% bbvars).status
self.assertEqual(0, status)
- self.assertEqual(1, len(glob(self.resultdir + \
+ self.assertEqual(1, len(glob(self.resultdir +
"%(wks)s-*.direct" % bbvars)))
@testcase(1346)
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)
- self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.direct")))
- self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.iso")))
+ "--image-name core-image-minimal"
+ ).status)
+ self.assertEqual(1, len(glob(self.resultdir +
+ "HYBRID_ISO_IMG-*.direct")))
+ self.assertEqual(1, len(glob(self.resultdir +
+ "HYBRID_ISO_IMG-*.iso")))
+
+ def __get_image_env_path(self, image):
+ """Generate and obtain the path to <image>.env"""
+ 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')
+ return imgdatadir
@testcase(1347)
def test_image_env(self):
- """Test generation of <image>.env files."""
+ """Test generation of <image>.env files"""
image = 'core-image-minimal'
- 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')
+ imgdatadir = self.__get_image_env_path(image)
basename = get_bb_var('IMAGE_BASENAME', image)
self.assertEqual(basename, image)
@@ -220,6 +278,21 @@ class Wic(oeSelfTest):
self.assertTrue(var in content, "%s is not in .env file" % var)
self.assertTrue(content[var])
+ @testcase(1559)
+ def test_image_vars_dir(self):
+ """Test image vars directory selection"""
+ image = 'core-image-minimal'
+ imgenvdir = self.__get_image_env_path(image)
+
+ self.assertEqual(0, runCmd("wic create directdisk "
+ "--image-name=%s "
+ "-v %s"
+ % (image, imgenvdir)).status)
+ self.assertEqual(0, runCmd("wic create directdisk "
+ "--image-name=%s "
+ "--vars %s"
+ % (image, imgenvdir)).status)
+
@testcase(1351)
def test_wic_image_type(self):
"""Test building wic images by bitbake"""
@@ -239,7 +312,7 @@ class Wic(oeSelfTest):
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" \
+ self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
% image).status)
self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
@@ -247,7 +320,8 @@ class Wic(oeSelfTest):
def test_mkgummidisk(self):
"""Test creation of mkgummidisk image"""
image = "mkgummidisk"
- self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
+ self.assertEqual(0, runCmd("wic create %s --image-name "
+ "core-image-minimal"
% image).status)
self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
@@ -255,7 +329,7 @@ class Wic(oeSelfTest):
def test_mkefidisk(self):
"""Test creation of mkefidisk image"""
image = "mkefidisk"
- self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
+ self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
% image).status)
self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
@@ -263,11 +337,11 @@ class Wic(oeSelfTest):
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" \
+ self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
% image).status)
self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
- @testcase(1422)
+ @testcase(1424)
def test_qemu(self):
"""Test wic-image-minimal under qemu"""
self.assertEqual(0, bitbake('wic-image-minimal').status)
@@ -275,28 +349,56 @@ class Wic(oeSelfTest):
with runqemu('wic-image-minimal', 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))
+ self.assertEqual(1, status, 'Failed to run command "%s": %s'
+ % (command, output))
self.assertEqual(output, '/dev/root /\r\n/dev/vda3 /mnt')
+ @testcase(1496)
def test_bmap(self):
"""Test generation of .bmap file"""
image = "directdisk"
- status = runCmd("wic create %s -e core-image-minimal --bmap" % image).status
+ status = runCmd("wic create %s -e core-image-minimal -m"
+ % image).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)))
+ status = runCmd("wic create %s -e core-image-minimal --bmap"
+ % image).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)))
+ self.assertEqual(1, len(glob(self.resultdir + "%s-*direct.bmap"
+ % image)))
+ @testcase(1560)
def test_systemd_bootdisk(self):
"""Test creation of systemd-bootdisk image"""
image = "systemd-bootdisk"
- self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
+ self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
% image).status)
self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
+ @testcase(1561)
def test_sdimage_bootpart(self):
"""Test creation of sdimage-bootpart image"""
image = "sdimage-bootpart"
self.write_config('IMAGE_BOOT_FILES = "bzImage"\n')
- self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
+ self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
% image).status)
self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
+
+ @testcase(1562)
+ def test_alternate_output_dir(self):
+ """Test alternate output directory"""
+ self.assertEqual(0, runCmd("wic create directdisk "
+ "-e core-image-minimal "
+ "-o %s"
+ % self.alternate_resultdir).status)
+ self.assertEqual(1, len(glob(self.alternate_resultdir +
+ "build/directdisk-*.direct")))
+ self.assertEqual(0, runCmd("wic create mkefidisk -e "
+ "core-image-minimal "
+ "--outdir %s"
+ % self.alternate_resultdir).status)
+ self.assertEqual(1, len(glob(self.alternate_resultdir +
+ "build/mkefidisk-*direct")))
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] selftest/wic: extending test coverage for WIC script options
2016-12-13 15:53 [PATCH] selftest/wic: extending test coverage for WIC script options Jair Gonzalez
@ 2016-12-13 20:17 ` Ed Bartosh
2016-12-14 0:24 ` Jair Gonzalez
0 siblings, 1 reply; 4+ messages in thread
From: Ed Bartosh @ 2016-12-13 20:17 UTC (permalink / raw)
To: Jair Gonzalez; +Cc: openembedded-core
Hi Jair,
Thank you for the patch! My comments are below.
On Tue, Dec 13, 2016 at 09:53:27AM -0600, Jair Gonzalez wrote:
> The previous WIC script selftest didn't cover all of its command
> line options. The following test cases were added to complete
> covering them:
>
> 1552 Test wic --version
> 1553 Test wic help create
> 1554 Test wic help list
> 1555 Test wic list images
> 1556 Test wic list source-plugins
> 1557 Test wic listed images help
> 1558 Test wic debug, skip-build-check and build_rootfs
> 1559 Test image vars directory selection
> 1562 Test alternate output directory
> In addition, the following test cases were assigned an ID number on
> Testopia:
>
> 1560 Test creation of systemd-bootdisk image
> 1561 Test creation of sdimage-bootpart image
>
> Finally, part of the test methods were rearranged to group them by
> functionality, and some cleanup was made to improve the code's
> compliance with PEP8 style guide.
I'd suggest to split this patch to at least 3 patches:
- new testcases (fix for YOCTO 10594)
- assigning id numbers
- removing WKS_FILE = "wic-image-minimal" from config
- code cleanup
> Fixes [YOCTO 10594]
>
> Signed-off-by: Jair Gonzalez <jair.de.jesus.gonzalez.plascencia@linux.intel.com>
> ---
> meta/lib/oeqa/selftest/wic.py | 246 +++++++++++++++++++++++++++++-------------
> 1 file changed, 174 insertions(+), 72 deletions(-)
>
> diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
> index e652fad..46bfb94 100644
> --- a/meta/lib/oeqa/selftest/wic.py
> +++ b/meta/lib/oeqa/selftest/wic.py
> @@ -37,13 +37,13 @@ class Wic(oeSelfTest):
> """Wic test class."""
>
> resultdir = "/var/tmp/wic/build/"
> + alternate_resultdir = "/var/tmp/wic/build/alt/"
> image_is_ready = False
>
> def setUpLocal(self):
> """This code is executed before each test method."""
> self.write_config('IMAGE_FSTYPES += " hddimg"\n'
> - 'MACHINE_FEATURES_append = " efi"\n'
> - 'WKS_FILE = "wic-image-minimal"\n')
I like the change, but it should be also in a separate patch.
> + 'MACHINE_FEATURES_append = " efi"\n')
>
> # Do this here instead of in setUpClass as the base setUp does some
> # clean up which can result in the native tools built earlier in
> @@ -56,10 +56,16 @@ class Wic(oeSelfTest):
>
> rmtree(self.resultdir, ignore_errors=True)
>
> + @testcase(1552)
> + def test_version(self):
> + """Test wic --version"""
> + self.assertEqual(0, runCmd('wic --version').status)
> +
> @testcase(1208)
> def test_help(self):
> - """Test wic --help"""
> + """Test wic --help and wic -h"""
> self.assertEqual(0, runCmd('wic --help').status)
> + self.assertEqual(0, runCmd('wic -h').status)
> @testcase(1209)
> def test_createhelp(self):
> @@ -71,19 +77,74 @@ class Wic(oeSelfTest):
> """Test wic list --help"""
> self.assertEqual(0, runCmd('wic list --help').status)
>
> + @testcase(1553)
> + def test_help_create(self):
> + """Test wic help create"""
> + self.assertEqual(0, runCmd('wic help create').status)
> +
> + @testcase(1554)
> + def test_help_list(self):
> + """Test wic help list"""
> + self.assertEqual(0, runCmd('wic help list').status)
> +
> + @testcase(1215)
> + def test_help_overview(self):
> + """Test wic help overview"""
> + self.assertEqual(0, runCmd('wic help overview').status)
> +
> + @testcase(1216)
> + def test_help_plugins(self):
> + """Test wic help plugins"""
> + self.assertEqual(0, runCmd('wic help plugins').status)
> +
> + @testcase(1217)
> + def test_help_kickstart(self):
> + """Test wic help kickstart"""
> + self.assertEqual(0, runCmd('wic help kickstart').status)
> +
> + @testcase(1555)
> + def test_list_images(self):
> + """Test wic list images"""
> + self.assertEqual(0, runCmd('wic list images').status)
> +
> + @testcase(1556)
> + def test_list_source_plugins(self):
> + """Test wic list source-plugins"""
> + self.assertEqual(0, runCmd('wic list source-plugins').status)
> +
> + @testcase(1557)
> + def test_listed_images_help(self):
> + """Test wic listed images help"""
> + output = runCmd('wic list images').output
> + imageDetails = [line.split() for line in output.split('\n')]
> + imageList = [row[0] for row in imageDetails]
How about replacing two last lines with this?
imagelist = [line.split()[0] for line in output.split('\n')]
> + for image in imageList:
> + self.assertEqual(0, runCmd('wic list %s help' % image).status)
> +
> + @testcase(1213)
> + def test_unsupported_subcommand(self):
> + """Test unsupported subcommand"""
> + self.assertEqual(1, runCmd('wic unsupported',
> + ignore_status=True).status)
> +
> + @testcase(1214)
> + def test_no_command(self):
> + """Test wic without command"""
> + self.assertEqual(1, runCmd('wic', ignore_status=True).status)
> +
> @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)
> + "--image-name=core-image-minimal").status)
Is '=' mandatory here?
> self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
>
> @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, 'core-image-minimal'))
> + 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 "
> @@ -96,113 +157,110 @@ class Wic(oeSelfTest):
> 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)
> + "--image-name core-image-minimal "
> + ).status)
What does this fix?
> self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
>
> - @testcase(1213)
> - def test_unsupported_subcommand(self):
> - """Test unsupported subcommand"""
> - self.assertEqual(1, runCmd('wic unsupported',
> - ignore_status=True).status)
> -
> - @testcase(1214)
> - def test_no_command(self):
> - """Test wic without command"""
> - self.assertEqual(1, runCmd('wic', ignore_status=True).status)
> -
> - @testcase(1215)
> - def test_help_overview(self):
> - """Test wic help overview"""
> - self.assertEqual(0, runCmd('wic help overview').status)
> -
> - @testcase(1216)
> - def test_help_plugins(self):
> - """Test wic help plugins"""
> - self.assertEqual(0, runCmd('wic help plugins').status)
> -
> - @testcase(1217)
> - def test_help_kickstart(self):
> - """Test wic help kickstart"""
> - self.assertEqual(0, runCmd('wic help kickstart').status)
> -
> @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 "
> + "-e core-image-minimal "
--image-name is more readable than -e from my point of view.
> "-c gzip").status)
> - self.assertEqual(1, len(glob(self.resultdir + \
> - "directdisk-*.direct.gz")))
> + self.assertEqual(1, len(glob(self.resultdir +
> + "directdisk-*.direct.gz")))
>
> @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 "
> + "--image-name=core-image-minimal "
> "-c bzip2").status)
> - self.assertEqual(1, len(glob(self.resultdir + \
> - "directdisk-*.direct.bz2")))
> + self.assertEqual(1, len(glob(self.resultdir +
> + "directdisk-*.direct.bz2")))
>
> @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")))
> + "--image-name=core-image-minimal "
> + "--compress-with=xz").status)
> + self.assertEqual(1, len(glob(self.resultdir +
> + "directdisk-*.direct.xz")))
>
> @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 "
> + "--image-name=core-image-minimal "
> "-c wrong", ignore_status=True).status)
>
> + @testcase(1558)
> + def test_debug_skip_build_check_and_build_rootfs(self):
> + """Test wic debug, skip-build-check and build_rootfs"""
> + self.assertEqual(0, runCmd("wic create directdisk "
> + "--image-name=core-image-minimal "
> + "-D -s -f").status)
> + self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
> + self.assertEqual(0, runCmd("wic create directdisk "
> + "--image-name=core-image-minimal "
> + "--debug "
> + "--skip-build-check "
> + "--build-rootfs").status)
> + self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
> +
I'd split this to two test cases as they're testing two different
options.
> @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 "
> + "--image-name=core-image-minimal "
> "--rootfs rootfs1=core-image-minimal "
> - "--rootfs rootfs2=core-image-minimal" \
> + "--rootfs rootfs2=core-image-minimal"
> % wks).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 = 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"
> status = runCmd("wic create %(wks)s "
> - "-b %(staging_datadir)s "
> - "-k %(deploy_dir_image)s "
> - "-n %(staging_dir_native)s "
> + "--bootimg-dir=%(staging_datadir)s "
> + "--kernel-dir=%(deploy_dir_image)s "
> + "--native-sysroot=%(staging_dir_native)s "
> "--rootfs-dir rootfs1=%(image_rootfs)s "
> - "--rootfs-dir rootfs2=%(image_rootfs)s" \
> + "--rootfs-dir rootfs2=%(image_rootfs)s"
> % bbvars).status
> self.assertEqual(0, status)
> - self.assertEqual(1, len(glob(self.resultdir + \
> + self.assertEqual(1, len(glob(self.resultdir +
> "%(wks)s-*.direct" % bbvars)))
>
> @testcase(1346)
> 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)
> - self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.direct")))
> - self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.iso")))
> + "--image-name core-image-minimal"
> + ).status)
This is less readable. Is this only to fit the line into 80 chars?
If so, let's not do it. Lines up to 100 chars long are more readable
than this I believe.
> + self.assertEqual(1, len(glob(self.resultdir +
> + "HYBRID_ISO_IMG-*.direct")))
> + self.assertEqual(1, len(glob(self.resultdir +
> + "HYBRID_ISO_IMG-*.iso")))
> +
The same thing. Full lines from previous code are more readable.
> + def __get_image_env_path(self, image):
Do you really need this to be mangled? one underscore should be enough.
> + """Generate and obtain the path to <image>.env"""
> + 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')
> + return imgdatadir
Can we cache results here? This would speed up testing I guess.
Something like this should be ok:
if image not in self.wicenv_cache:
self.assertEqual(0, bitbake('%s -c do_rootfs_wicenv' %
image).status)
stdir = get_bb_var('STAGING_DIR_TARGET', image)
self.wicenv_cache[image] = os.path.join(stdir, 'imgdata')
return self.wicenv_cache[image]
>
> @testcase(1347)
> def test_image_env(self):
> - """Test generation of <image>.env files."""
> + """Test generation of <image>.env files"""
> image = 'core-image-minimal'
> - 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')
> + imgdatadir = self.__get_image_env_path(image)
>
> basename = get_bb_var('IMAGE_BASENAME', image)
> self.assertEqual(basename, image)
> @@ -220,6 +278,21 @@ class Wic(oeSelfTest):
> self.assertTrue(var in content, "%s is not in .env file" % var)
> self.assertTrue(content[var])
>
> + @testcase(1559)
> + def test_image_vars_dir(self):
> + """Test image vars directory selection"""
> + image = 'core-image-minimal'
> + imgenvdir = self.__get_image_env_path(image)
> +
> + self.assertEqual(0, runCmd("wic create directdisk "
> + "--image-name=%s "
> + "-v %s"
> + % (image, imgenvdir)).status)
> + self.assertEqual(0, runCmd("wic create directdisk "
> + "--image-name=%s "
> + "--vars %s"
> + % (image, imgenvdir)).status)
> +
Do we really want to test short and long variant of options?
If so, we should do it for all options.
> @testcase(1351)
> def test_wic_image_type(self):
> """Test building wic images by bitbake"""
> @@ -239,7 +312,7 @@ class Wic(oeSelfTest):
> 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" \
> + self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
> % image).status)
> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>
> @@ -247,7 +320,8 @@ class Wic(oeSelfTest):
> def test_mkgummidisk(self):
> """Test creation of mkgummidisk image"""
> image = "mkgummidisk"
> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
> + self.assertEqual(0, runCmd("wic create %s --image-name "
> + "core-image-minimal"
> % image).status)
> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
I agree, this doesn't look good. How about dropping 'image' variable in
this and similar test cases?
cmd = "wic create mkgummidisk --image-name core-image-minimal"
self.assertEqual(0, runCmd(cmd).status)
self.assertEqual(1, len(glob(self.resultdir + "mkgummidisk-*direct")))
> @@ -255,7 +329,7 @@ class Wic(oeSelfTest):
> def test_mkefidisk(self):
> """Test creation of mkefidisk image"""
> image = "mkefidisk"
> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
> + self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
> % image).status)
> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>
> @@ -263,11 +337,11 @@ class Wic(oeSelfTest):
> 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" \
> + self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
> % image).status)
> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>
> - @testcase(1422)
> + @testcase(1424)
> def test_qemu(self):
> """Test wic-image-minimal under qemu"""
> self.assertEqual(0, bitbake('wic-image-minimal').status)
> @@ -275,28 +349,56 @@ class Wic(oeSelfTest):
> with runqemu('wic-image-minimal', 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))
> + self.assertEqual(1, status, 'Failed to run command "%s": %s'
> + % (command, output))
less readable
> self.assertEqual(output, '/dev/root /\r\n/dev/vda3 /mnt')
>
> + @testcase(1496)
> def test_bmap(self):
> """Test generation of .bmap file"""
> image = "directdisk"
> - status = runCmd("wic create %s -e core-image-minimal --bmap" % image).status
> + status = runCmd("wic create %s -e core-image-minimal -m"
> + % image).status
less readable. --bmap is better to use here than -m.
> + 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)))
> + status = runCmd("wic create %s -e core-image-minimal --bmap"
> + % image).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)))
> + self.assertEqual(1, len(glob(self.resultdir + "%s-*direct.bmap"
> + % image)))
>
> + @testcase(1560)
> def test_systemd_bootdisk(self):
> """Test creation of systemd-bootdisk image"""
> image = "systemd-bootdisk"
> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
> + self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
> % image).status)
> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
>
> + @testcase(1561)
> def test_sdimage_bootpart(self):
> """Test creation of sdimage-bootpart image"""
> image = "sdimage-bootpart"
> self.write_config('IMAGE_BOOT_FILES = "bzImage"\n')
> - self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
> + self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
> % image).status)
the same thing - it doesn't worth it to please PEP8 if it reduces code
readability.
> self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
> +
> + @testcase(1562)
> + def test_alternate_output_dir(self):
> + """Test alternate output directory"""
> + self.assertEqual(0, runCmd("wic create directdisk "
> + "-e core-image-minimal "
> + "-o %s"
> + % self.alternate_resultdir).status)
> + self.assertEqual(1, len(glob(self.alternate_resultdir +
> + "build/directdisk-*.direct")))
> + self.assertEqual(0, runCmd("wic create mkefidisk -e "
> + "core-image-minimal "
> + "--outdir %s"
> + % self.alternate_resultdir).status)
> + self.assertEqual(1, len(glob(self.alternate_resultdir +
> + "build/mkefidisk-*direct")))
Would one test be enough?
BTW, did you measure how long does the test run before and after your
changes? We should be careful here as this test runs on ab and makes
people wait longer for results.
--
Regards,
Ed
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selftest/wic: extending test coverage for WIC script options
2016-12-13 20:17 ` Ed Bartosh
@ 2016-12-14 0:24 ` Jair Gonzalez
2016-12-14 11:44 ` Ed Bartosh
0 siblings, 1 reply; 4+ messages in thread
From: Jair Gonzalez @ 2016-12-14 0:24 UTC (permalink / raw)
To: ed.bartosh; +Cc: openembedded-core
Hi Ed,
Thank you for your response and suggestions. Below are my comments.
> -----Original Message-----
> From: Ed Bartosh [mailto:ed.bartosh@linux.intel.com]
> Sent: Tuesday, December 13, 2016 2:17 PM
> To: Jair Gonzalez <jair.de.jesus.gonzalez.plascencia@linux.intel.com>
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] selftest/wic: extending test coverage for
WIC
> script options
>
> Hi Jair,
>
> Thank you for the patch! My comments are below.
>
> On Tue, Dec 13, 2016 at 09:53:27AM -0600, Jair Gonzalez wrote:
> > The previous WIC script selftest didn't cover all of its command line
> > options. The following test cases were added to complete covering
> > them:
> >
> > 1552 Test wic --version
> > 1553 Test wic help create
> > 1554 Test wic help list
> > 1555 Test wic list images
> > 1556 Test wic list source-plugins
> > 1557 Test wic listed images help
> > 1558 Test wic debug, skip-build-check and build_rootfs
> > 1559 Test image vars directory selection
> > 1562 Test alternate output directory
>
> > In addition, the following test cases were assigned an ID number on
> > Testopia:
> >
> > 1560 Test creation of systemd-bootdisk image
> > 1561 Test creation of sdimage-bootpart image
> >
> > Finally, part of the test methods were rearranged to group them by
> > functionality, and some cleanup was made to improve the code's
> > compliance with PEP8 style guide.
>
> I'd suggest to split this patch to at least 3 patches:
> - new testcases (fix for YOCTO 10594)
> - assigning id numbers
> - removing WKS_FILE = "wic-image-minimal" from config
> - code cleanup
Agreed. I'll split it and submit the new patches.
>
> > Fixes [YOCTO 10594]
> >
> > Signed-off-by: Jair Gonzalez
> > <jair.de.jesus.gonzalez.plascencia@linux.intel.com>
> > ---
> > meta/lib/oeqa/selftest/wic.py | 246
> > +++++++++++++++++++++++++++++-------------
> > 1 file changed, 174 insertions(+), 72 deletions(-)
> >
> > diff --git a/meta/lib/oeqa/selftest/wic.py
> > b/meta/lib/oeqa/selftest/wic.py index e652fad..46bfb94 100644
> > --- a/meta/lib/oeqa/selftest/wic.py
> > +++ b/meta/lib/oeqa/selftest/wic.py
> > @@ -37,13 +37,13 @@ class Wic(oeSelfTest):
> > """Wic test class."""
> >
> > resultdir = "/var/tmp/wic/build/"
> > + alternate_resultdir = "/var/tmp/wic/build/alt/"
> > image_is_ready = False
> >
> > def setUpLocal(self):
> > """This code is executed before each test method."""
> > self.write_config('IMAGE_FSTYPES += " hddimg"\n'
> > - 'MACHINE_FEATURES_append = " efi"\n'
> > - 'WKS_FILE = "wic-image-minimal"\n')
> I like the change, but it should be also in a separate patch.
Agreed.
>
> > + 'MACHINE_FEATURES_append = " efi"\n')
> >
> > # Do this here instead of in setUpClass as the base setUp does
some
> > # clean up which can result in the native tools built earlier
> > in @@ -56,10 +56,16 @@ class Wic(oeSelfTest):
> >
> > rmtree(self.resultdir, ignore_errors=True)
> >
> > + @testcase(1552)
> > + def test_version(self):
> > + """Test wic --version"""
> > + self.assertEqual(0, runCmd('wic --version').status)
> > +
> > @testcase(1208)
> > def test_help(self):
> > - """Test wic --help"""
> > + """Test wic --help and wic -h"""
> > self.assertEqual(0, runCmd('wic --help').status)
> > + self.assertEqual(0, runCmd('wic -h').status)
> > @testcase(1209)
> > def test_createhelp(self):
> > @@ -71,19 +77,74 @@ class Wic(oeSelfTest):
> > """Test wic list --help"""
> > self.assertEqual(0, runCmd('wic list --help').status)
> >
> > + @testcase(1553)
> > + def test_help_create(self):
> > + """Test wic help create"""
> > + self.assertEqual(0, runCmd('wic help create').status)
> > +
> > + @testcase(1554)
> > + def test_help_list(self):
> > + """Test wic help list"""
> > + self.assertEqual(0, runCmd('wic help list').status)
> > +
> > + @testcase(1215)
> > + def test_help_overview(self):
> > + """Test wic help overview"""
> > + self.assertEqual(0, runCmd('wic help overview').status)
> > +
> > + @testcase(1216)
> > + def test_help_plugins(self):
> > + """Test wic help plugins"""
> > + self.assertEqual(0, runCmd('wic help plugins').status)
> > +
> > + @testcase(1217)
> > + def test_help_kickstart(self):
> > + """Test wic help kickstart"""
> > + self.assertEqual(0, runCmd('wic help kickstart').status)
> > +
> > + @testcase(1555)
> > + def test_list_images(self):
> > + """Test wic list images"""
> > + self.assertEqual(0, runCmd('wic list images').status)
> > +
> > + @testcase(1556)
> > + def test_list_source_plugins(self):
> > + """Test wic list source-plugins"""
> > + self.assertEqual(0, runCmd('wic list source-plugins').status)
> > +
> > + @testcase(1557)
> > + def test_listed_images_help(self):
> > + """Test wic listed images help"""
> > + output = runCmd('wic list images').output
> > + imageDetails = [line.split() for line in output.split('\n')]
> > + imageList = [row[0] for row in imageDetails]
> How about replacing two last lines with this?
> imagelist = [line.split()[0] for line in output.split('\n')]
I agree. What about this?
imagelist = [line.split()[0] for line in output.splitlines()]
>
> > + for image in imageList:
> > + self.assertEqual(0, runCmd('wic list %s help' %
> > + image).status)
> > +
> > + @testcase(1213)
> > + def test_unsupported_subcommand(self):
> > + """Test unsupported subcommand"""
> > + self.assertEqual(1, runCmd('wic unsupported',
> > + ignore_status=True).status)
> > +
> > + @testcase(1214)
> > + def test_no_command(self):
> > + """Test wic without command"""
> > + self.assertEqual(1, runCmd('wic', ignore_status=True).status)
> > +
> > @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)
> > +
> > + "--image-name=core-image-minimal").status)
> Is '=' mandatory here?
On wic's help it appears as mandatory, but on practice, it can be used both
ways. I decided to use both ways along the module to test both usages and
increase coverage, but not to dedicate specific test cases to each
combination.
>
> > self.assertEqual(1, len(glob(self.resultdir +
> > "directdisk-*.direct")))
> >
> > @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,
'core-image-minimal'))
> > + 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 "
> > @@ -96,113 +157,110 @@ class Wic(oeSelfTest):
> > 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)
> > + "--image-name core-image-minimal "
> > + ).status)
> What does this fix?
It's to conform to PEP8 with equal or less than 79 chars per line.
>
> > self.assertEqual(1, len(glob(self.resultdir +
> > "directdisk-*.direct")))
> >
> > - @testcase(1213)
> > - def test_unsupported_subcommand(self):
> > - """Test unsupported subcommand"""
> > - self.assertEqual(1, runCmd('wic unsupported',
> > - ignore_status=True).status)
> > -
> > - @testcase(1214)
> > - def test_no_command(self):
> > - """Test wic without command"""
> > - self.assertEqual(1, runCmd('wic', ignore_status=True).status)
> > -
> > - @testcase(1215)
> > - def test_help_overview(self):
> > - """Test wic help overview"""
> > - self.assertEqual(0, runCmd('wic help overview').status)
> > -
> > - @testcase(1216)
> > - def test_help_plugins(self):
> > - """Test wic help plugins"""
> > - self.assertEqual(0, runCmd('wic help plugins').status)
> > -
> > - @testcase(1217)
> > - def test_help_kickstart(self):
> > - """Test wic help kickstart"""
> > - self.assertEqual(0, runCmd('wic help kickstart').status)
> > -
> > @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 "
> > + "-e core-image-minimal "
> --image-name is more readable than -e from my point of view.
Similarly to the '=' to define long option names' arguments, I used both
forms of each option along the module to increase coverage.
>
> > "-c gzip").status)
> > - self.assertEqual(1, len(glob(self.resultdir + \
> > - "directdisk-*.direct.gz")))
> > + self.assertEqual(1, len(glob(self.resultdir +
> > + "directdisk-*.direct.gz")))
> >
> > @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 "
> > + "--image-name=core-image-minimal "
> > "-c bzip2").status)
> > - self.assertEqual(1, len(glob(self.resultdir + \
> > - "directdisk-*.direct.bz2")))
> > + self.assertEqual(1, len(glob(self.resultdir +
> > + "directdisk-*.direct.bz2")))
> >
> > @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")))
> > + "--image-name=core-image-minimal "
> > + "--compress-with=xz").status)
> > + self.assertEqual(1, len(glob(self.resultdir +
> > + "directdisk-*.direct.xz")))
> >
> > @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 "
> > + "--image-name=core-image-minimal "
> > "-c wrong",
> > ignore_status=True).status)
> >
> > + @testcase(1558)
> > + def test_debug_skip_build_check_and_build_rootfs(self):
> > + """Test wic debug, skip-build-check and build_rootfs"""
> > + self.assertEqual(0, runCmd("wic create directdisk "
> > + "--image-name=core-image-minimal "
> > + "-D -s -f").status)
> > + self.assertEqual(1, len(glob(self.resultdir +
"directdisk-*.direct")))
> > + self.assertEqual(0, runCmd("wic create directdisk "
> > + "--image-name=core-image-minimal "
> > + "--debug "
> > + "--skip-build-check "
> > + "--build-rootfs").status)
> > + self.assertEqual(1, len(glob(self.resultdir +
> > + "directdisk-*.direct")))
> > +
> I'd split this to two test cases as they're testing two different options.
Actually, those are three different options (with their short and long
versions). I did this to not add too many test cases, but as you mention,
probably it's better to separate them by option to make it clearer.
>
> > @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 "
> > + "--image-name=core-image-minimal "
> > "--rootfs rootfs1=core-image-minimal
"
> > - "--rootfs
rootfs2=core-image-minimal" \
> > + "--rootfs
rootfs2=core-image-minimal"
> > % wks).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 = 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"
> > status = runCmd("wic create %(wks)s "
> > - "-b %(staging_datadir)s "
> > - "-k %(deploy_dir_image)s "
> > - "-n %(staging_dir_native)s "
> > + "--bootimg-dir=%(staging_datadir)s "
> > + "--kernel-dir=%(deploy_dir_image)s "
> > + "--native-sysroot=%(staging_dir_native)s "
> > "--rootfs-dir rootfs1=%(image_rootfs)s "
> > - "--rootfs-dir rootfs2=%(image_rootfs)s" \
> > + "--rootfs-dir rootfs2=%(image_rootfs)s"
> > % bbvars).status
> > self.assertEqual(0, status)
> > - self.assertEqual(1, len(glob(self.resultdir + \
> > + self.assertEqual(1, len(glob(self.resultdir +
> > "%(wks)s-*.direct" % bbvars)))
> >
> > @testcase(1346)
> > 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)
> > - self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-
> *.direct")))
> > - self.assertEqual(1, len(glob(self.resultdir +
"HYBRID_ISO_IMG-*.iso")))
> > + "--image-name core-image-minimal"
> > + ).status)
> This is less readable. Is this only to fit the line into 80 chars?
> If so, let's not do it. Lines up to 100 chars long are more readable than
this I
> believe.
I changed it to conform to PEP8 and increase readability on editors adjusted
to 80 chars. However, if you consider it's better to leave it on 100 chars,
I could work within that.
>
> > + self.assertEqual(1, len(glob(self.resultdir +
> > + "HYBRID_ISO_IMG-*.direct")))
> > + self.assertEqual(1, len(glob(self.resultdir +
> > + "HYBRID_ISO_IMG-*.iso")))
> > +
> The same thing. Full lines from previous code are more readable.
>
> > + def __get_image_env_path(self, image):
> Do you really need this to be mangled? one underscore should be enough.
Agreed, I'll change it.
>
> > + """Generate and obtain the path to <image>.env"""
> > + 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')
> > + return imgdatadir
> Can we cache results here? This would speed up testing I guess.
> Something like this should be ok:
>
> if image not in self.wicenv_cache:
> self.assertEqual(0, bitbake('%s -c do_rootfs_wicenv' %
> image).status)
> stdir = get_bb_var('STAGING_DIR_TARGET', image)
> self.wicenv_cache[image] = os.path.join(stdir, 'imgdata')
>
> return self.wicenv_cache[image]
Agreed, thanks. I'll try your suggestion.
>
> >
> > @testcase(1347)
> > def test_image_env(self):
> > - """Test generation of <image>.env files."""
> > + """Test generation of <image>.env files"""
> > image = 'core-image-minimal'
> > - 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')
> > + imgdatadir = self.__get_image_env_path(image)
> >
> > basename = get_bb_var('IMAGE_BASENAME', image)
> > self.assertEqual(basename, image) @@ -220,6 +278,21 @@ class
> > Wic(oeSelfTest):
> > self.assertTrue(var in content, "%s is not in .env
file" % var)
> > self.assertTrue(content[var])
> >
> > + @testcase(1559)
> > + def test_image_vars_dir(self):
> > + """Test image vars directory selection"""
> > + image = 'core-image-minimal'
> > + imgenvdir = self.__get_image_env_path(image)
> > +
> > + self.assertEqual(0, runCmd("wic create directdisk "
> > + "--image-name=%s "
> > + "-v %s"
> > + % (image, imgenvdir)).status)
> > + self.assertEqual(0, runCmd("wic create directdisk "
> > + "--image-name=%s "
> > + "--vars %s"
> > + % (image, imgenvdir)).status)
> > +
> Do we really want to test short and long variant of options?
> If so, we should do it for all options.
Within the module, all short and long variant of options are tested. Not all
combinations of long variants with '=' and without it are tested, though.
>
> > @testcase(1351)
> > def test_wic_image_type(self):
> > """Test building wic images by bitbake"""
> > @@ -239,7 +312,7 @@ class Wic(oeSelfTest):
> > 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" \
> > + self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal"
> > % image).status)
> > self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" %
> > image)))
> >
> > @@ -247,7 +320,8 @@ class Wic(oeSelfTest):
> > def test_mkgummidisk(self):
> > """Test creation of mkgummidisk image"""
> > image = "mkgummidisk"
> > - self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal" \
> > + self.assertEqual(0, runCmd("wic create %s --image-name "
> > + "core-image-minimal"
> > % image).status)
> > self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" %
> > image)))
>
> I agree, this doesn't look good. How about dropping 'image' variable in
this
> and similar test cases?
>
> cmd = "wic create mkgummidisk --image-name core-image-minimal"
> self.assertEqual(0, runCmd(cmd).status)
>
> self.assertEqual(1, len(glob(self.resultdir + "mkgummidisk-*direct")))
>
Agreed.
> > @@ -255,7 +329,7 @@ class Wic(oeSelfTest):
> > def test_mkefidisk(self):
> > """Test creation of mkefidisk image"""
> > image = "mkefidisk"
> > - self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal" \
> > + self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal"
> > % image).status)
> > self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" %
> > image)))
> >
> > @@ -263,11 +337,11 @@ class Wic(oeSelfTest):
> > 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" \
> > + self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal"
> > % image).status)
> > self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" %
> > image)))
> >
> > - @testcase(1422)
> > + @testcase(1424)
> > def test_qemu(self):
> > """Test wic-image-minimal under qemu"""
> > self.assertEqual(0, bitbake('wic-image-minimal').status)
> > @@ -275,28 +349,56 @@ class Wic(oeSelfTest):
> > with runqemu('wic-image-minimal', 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))
> > + self.assertEqual(1, status, 'Failed to run command "%s":
%s'
> > + % (command, output))
> less readable
Same, PEP8, but I may change it to 100 chars instead.
>
> > self.assertEqual(output, '/dev/root /\r\n/dev/vda3 /mnt')
> >
> > + @testcase(1496)
> > def test_bmap(self):
> > """Test generation of .bmap file"""
> > image = "directdisk"
> > - status = runCmd("wic create %s -e core-image-minimal --bmap" %
> image).status
> > + status = runCmd("wic create %s -e core-image-minimal -m"
> > + % image).status
> less readable. --bmap is better to use here than -m.
Same, PEP8, but I may change it to 100 chars instead. Using -m and --bmap in
different test cases to increase coverage.
>
> > + 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)))
> > + status = runCmd("wic create %s -e core-image-minimal --bmap"
> > + % image).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)))
> > + self.assertEqual(1, len(glob(self.resultdir + "%s-*direct.bmap"
> > + % image)))
> >
> > + @testcase(1560)
> > def test_systemd_bootdisk(self):
> > """Test creation of systemd-bootdisk image"""
> > image = "systemd-bootdisk"
> > - self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal" \
> > + self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal"
> > % image).status)
> > self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" %
> > image)))
> >
> > + @testcase(1561)
> > def test_sdimage_bootpart(self):
> > """Test creation of sdimage-bootpart image"""
> > image = "sdimage-bootpart"
> > self.write_config('IMAGE_BOOT_FILES = "bzImage"\n')
> > - self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal" \
> > + self.assertEqual(0, runCmd("wic create %s -e
core-image-minimal"
> > % image).status)
> the same thing - it doesn't worth it to please PEP8 if it reduces code
> readability.
Same, PEP8, but I may change it to 100 chars instead.
>
> > self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" %
> > image)))
> > +
> > + @testcase(1562)
> > + def test_alternate_output_dir(self):
> > + """Test alternate output directory"""
> > + self.assertEqual(0, runCmd("wic create directdisk "
> > + "-e core-image-minimal "
> > + "-o %s"
> > + % self.alternate_resultdir).status)
> > + self.assertEqual(1, len(glob(self.alternate_resultdir +
> > + "build/directdisk-*.direct")))
> > + self.assertEqual(0, runCmd("wic create mkefidisk -e "
> > + "core-image-minimal "
> > + "--outdir %s"
> > + % self.alternate_resultdir).status)
> > + self.assertEqual(1, len(glob(self.alternate_resultdir +
> > + "build/mkefidisk-*direct")))
> Would one test be enough?
I'm using both output option variants to increase coverage.
>
> BTW, did you measure how long does the test run before and after your
> changes? We should be careful here as this test runs on ab and makes
people
> wait longer for results.
Agreed. I didn't do it, but I'll measure it and take it into account for my
next update.
>
> --
> Regards,
> Ed
Regards,
Jair
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selftest/wic: extending test coverage for WIC script options
2016-12-14 0:24 ` Jair Gonzalez
@ 2016-12-14 11:44 ` Ed Bartosh
0 siblings, 0 replies; 4+ messages in thread
From: Ed Bartosh @ 2016-12-14 11:44 UTC (permalink / raw)
To: Jair Gonzalez; +Cc: openembedded-core
On Tue, Dec 13, 2016 at 06:24:57PM -0600, Jair Gonzalez wrote:
> Hi Ed,
>
> Thank you for your response and suggestions. Below are my comments.
>
> > > + @testcase(1557)
> > > + def test_listed_images_help(self):
> > > + """Test wic listed images help"""
> > > + output = runCmd('wic list images').output
> > > + imageDetails = [line.split() for line in output.split('\n')]
> > > + imageList = [row[0] for row in imageDetails]
> > How about replacing two last lines with this?
> > imagelist = [line.split()[0] for line in output.split('\n')]
> I agree. What about this?
> imagelist = [line.split()[0] for line in output.splitlines()]
This is event better, thanks!
> > > +
> > > + "--image-name=core-image-minimal").status)
> > Is '=' mandatory here?
> On wic's help it appears as mandatory, but on practice, it can be used both
> ways. I decided to use both ways along the module to test both usages and
> increase coverage, but not to dedicate specific test cases to each
> combination.
Makes sense to me.
> > > def test_compress_gzip(self):
> > > """Test compressing an image with gzip"""
> > > self.assertEqual(0, runCmd("wic create directdisk "
> > > - "--image-name core-image-minimal "
> > > + "-e core-image-minimal "
> > --image-name is more readable than -e from my point of view.
> Similarly to the '=' to define long option names' arguments, I used both
> forms of each option along the module to increase coverage.
OK
> > > + def test_debug_skip_build_check_and_build_rootfs(self):
> > > + """Test wic debug, skip-build-check and build_rootfs"""
> > > + self.assertEqual(0, runCmd("wic create directdisk "
> > > + "--image-name=core-image-minimal "
> > > + "-D -s -f").status)
> > > + self.assertEqual(1, len(glob(self.resultdir +
> "directdisk-*.direct")))
> > > + self.assertEqual(0, runCmd("wic create directdisk "
> > > + "--image-name=core-image-minimal "
> > > + "--debug "
> > > + "--skip-build-check "
> > > + "--build-rootfs").status)
> > > + self.assertEqual(1, len(glob(self.resultdir +
> > > + "directdisk-*.direct")))
> > > +
> > I'd split this to two test cases as they're testing two different options.
> Actually, those are three different options (with their short and long
> versions). I did this to not add too many test cases, but as you mention,
> probably it's better to separate them by option to make it clearer.
Agreed.
> core-image-minimal").status)
> > > - self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-
> > *.direct")))
> > > - self.assertEqual(1, len(glob(self.resultdir +
> "HYBRID_ISO_IMG-*.iso")))
> > > + "--image-name core-image-minimal"
> > > + ).status)
> > This is less readable. Is this only to fit the line into 80 chars?
> > If so, let's not do it. Lines up to 100 chars long are more readable than
> this I
> > believe.
> I changed it to conform to PEP8 and increase readability on editors adjusted
> to 80 chars. However, if you consider it's better to leave it on 100 chars,
> I could work within that.
Let's agree on max 100 chars/line if it improves readability. Otherwise
80 is ok.
> > > + self.assertEqual(0, runCmd("wic create directdisk "
> > > + "--image-name=%s "
> > > + "--vars %s"
> > > + % (image, imgenvdir)).status)
> > > +
> > Do we really want to test short and long variant of options?
> > If so, we should do it for all options.
> Within the module, all short and long variant of options are tested. Not all
> combinations of long variants with '=' and without it are tested, though.
OK, makes sense to me.
> > > + @testcase(1562)
> > > + def test_alternate_output_dir(self):
> > > + """Test alternate output directory"""
> > > + self.assertEqual(0, runCmd("wic create directdisk "
> > > + "-e core-image-minimal "
> > > + "-o %s"
> > > + % self.alternate_resultdir).status)
> > > + self.assertEqual(1, len(glob(self.alternate_resultdir +
> > > + "build/directdisk-*.direct")))
> > > + self.assertEqual(0, runCmd("wic create mkefidisk -e "
> > > + "core-image-minimal "
> > > + "--outdir %s"
> > > + % self.alternate_resultdir).status)
> > > + self.assertEqual(1, len(glob(self.alternate_resultdir +
> > > + "build/mkefidisk-*direct")))
> > Would one test be enough?
> I'm using both output option variants to increase coverage.
yep, understood.
> > BTW, did you measure how long does the test run before and after your
> > changes? We should be careful here as this test runs on ab and makes
> people
> > wait longer for results.
> Agreed. I didn't do it, but I'll measure it and take it into account for my
> next update.
Great! It would be interesting to look at the numbers.
--
Regards,
Ed
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-14 11:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-13 15:53 [PATCH] selftest/wic: extending test coverage for WIC script options Jair Gonzalez
2016-12-13 20:17 ` Ed Bartosh
2016-12-14 0:24 ` Jair Gonzalez
2016-12-14 11:44 ` Ed Bartosh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox