[OE-core] [PATCH] oeqa/selftest: [YOCTO #7976] Updated imagefeature testcases.

[Istrate, Daniel AlexandruX]

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:paul.eggleton at linux.intel.com] 
Sent: Tuesday, July 14, 2015 12:30 PM
To: Istrate, Daniel AlexandruX
Cc: openembedded-core at lists.openembedded.org
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 <daniel.alexandrux.istrate at intel.com>
> ---
>  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 <daniel.alexandrux.istrate at intel.com>
>          """
> 
> -        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