Hi Paul,
Thank you very much for your feedback.
A stylistic thing perhaps, but instead of ... {}".format(str) why not just use
... %s" % str ?
-- I don't have strong feelings about using str.format ..I could use "%s %str"
instead if that's the style. I like format because I don't have to cast to
string the passed in parameter.
Hmm, so I thought the plan we talked about here was to use QemuRunner. As I
said earlier, if QemuRunner isn't doing what we need here (e.g. isn't allowing
us to use pexpect) then we should fix it so that it does or at least make it
optional. Even with these changes we are still left with a complete
implementation of running QEMU here in the test, which we want to avoid.
-- I am aware that we should use QemuRunner but at the moment it isn't doing
what we need here.
Since these testcases might interfere with other qemu instances running in
parallel I thought it is a good compromise the way I implemented in this patch.
When we will manage to update QemuRunner, these testcase can be easily
updated too.
These changes aren't described in the commit message - please either add
mention of them or if they aren't actually related to resolving the QEMU
running issue, put them in a separate patch.
-- I updated the assert message to also include the output. Is it really
necessary to submit another patch with this minor change?
Thanks,
--Daniel
-----Original Message-----
From: Paul Eggleton [mailto:[email protected]]
Sent: Tuesday, July 14, 2015 12:30 PM
To: Istrate, Daniel AlexandruX
Cc: [email protected]
Subject: Re: [OE-core] [PATCH] oeqa/selftest: [YOCTO #7976] Updated
imagefeature testcases.
Hi Daniel,
Comments in-line below.
On Tuesday 14 July 2015 11:48:09 Daniel Istrate wrote:
> Updated testcases that use qemu to not interfere with other qemu
> instances that might run in parallel.
The [YOCTO #....] reference should go here, not in the shortlog.
> Signed-off-by: Daniel Istrate <[email protected]>
> ---
> meta/lib/oeqa/selftest/imagefeatures.py | 153
> ++++++++++++-------------------- 1 file changed, 58 insertions(+), 95
> deletions(-)
>
> diff --git a/meta/lib/oeqa/selftest/imagefeatures.py
> b/meta/lib/oeqa/selftest/imagefeatures.py index e0e424d..030969e
> 100644
> --- a/meta/lib/oeqa/selftest/imagefeatures.py
> +++ b/meta/lib/oeqa/selftest/imagefeatures.py
> @@ -2,13 +2,20 @@ from oeqa.selftest.base import oeSelfTest from
> oeqa.utils.commands import runCmd, bitbake, get_bb_var from
> oeqa.utils.decorators import testcase import pexpect -from os.path
> import expanduser, isfile
> +from os.path import isfile
> from os import system
> import glob
> +import time
>
>
> class ImageFeatures(oeSelfTest):
>
> + test_user = 'tester'
> + root_user = 'root'
> + prompt = r'qemux86:\S+[$#]\s+'
> + ssh_cmd = "ssh {} -l {} -o UserKnownHostsFile=/dev/null -o
> StrictHostKeyChecking=no"
> + get_ip_patt = r'\s+ip=(?P<qemu_ip>(\d+.){3}\d+)::'
> +
> @testcase(1107)
> def test_non_root_user_can_connect_via_ssh_without_password(self):
> """
> @@ -20,69 +27,49 @@ class ImageFeatures(oeSelfTest):
> AutomatedBy: Daniel Istrate <[email protected]>
> """
>
> - test_user = 'tester'
> - root_user = 'root'
> - prompt = r'qemux86:\S+[$#]\s+'
> - tap_inf_ip = '192.168.7.2'
> -
> features = 'EXTRA_IMAGE_FEATURES += "ssh-server-openssh
> empty-root-password"\n' features += 'INHERIT += "extrausers"\n'
> - features += 'EXTRA_USERS_PARAMS = "useradd -p \'\' {}; usermod -s
> /bin/sh {};"'.format(test_user, test_user) + features +=
> 'EXTRA_USERS_PARAMS = "useradd -p \'\' {}; usermod -s /bin/sh
> {};"'.format(self.test_user, self.test_user)
>
> # Append 'features' to local.conf
> self.append_config(features)
>
> # Build a core-image-minimal
> ret = bitbake('core-image-minimal')
> - self.assertEqual(0, ret.status, 'Failed to build a
> core-image-minimal')
> -
> - rm_ssh_keys_cmd = 'ssh-keygen -f "{}/.ssh/known_hosts" -R
> {}'.format(expanduser('~'), tap_inf_ip)
> - # Delete the ssh keys for 192.168.7.2 (qemu)
> - ret = runCmd(rm_ssh_keys_cmd)
> - self.assertEqual(0, ret.status, 'Failed to delete ssh keys for qemu
> host.')
> + self.assertEqual(0, ret.status, 'Failed to build a
> core-image-minimal. Reason: {}'.format(ret.output))
A stylistic thing perhaps, but instead of ... {}".format(str) why not just use
... %s" % str ?
> # Boot qemu image
> - proc_qemu = pexpect.spawn('runqemu qemux86 nographic')
> + proc_qemu = pexpect.spawn('runqemu qemux86')
Hmm, so I thought the plan we talked about here was to use QemuRunner. As I
said earlier, if QemuRunner isn't doing what we need here (e.g. isn't allowing
us to use pexpect) then we should fix it so that it does or at least make it
optional. Even with these changes we are still left with a complete
implementation of running QEMU here in the test, which we want to avoid.
...
> @testcase(1101)
> def test_efi_gummiboot_images_can_be_build(self):
> @@ -311,7 +274,7 @@ class Gummiboot(oeSelfTest):
> # Create efi/gummiboot installation images
> wic_create_cmd = 'wic create mkgummidisk -e core-image-minimal'
> ret = runCmd(wic_create_cmd)
> - self.assertEqual(0, ret.status, 'Failed to create efi/gummiboot
> installation images.')
> + self.assertEqual(0, ret.status, 'Failed to
> create efi/gummiboot installation images. Reason: {}'.format(ret.output))
These changes aren't described in the commit message - please either add
mention of them or if they aren't actually related to resolving the QEMU
running issue, put them in a separate patch.
Thanks,
Paul
--
Paul Eggleton
Intel Open Source Technology Centre
--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core
