Re: [PATCH] oeqa/runtime: Added wifi tests.

["Burton, Ross" <ross.burton@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3374 bytes --]

On 20 November 2015 at 11:42, Lucian Musat <george.l.musat@intel.com> wrote:

> +    def check_wifi_ip(self):
> +        time.sleep(3)
>

Why a sleep here?  Why none of the other tests?


> +        # Check ip address by ifconfig command
> +        wifi_interface = "nothing"
> +        (status, wifi_interface) = self.target.run("ifconfig | grep
> '^wlp\|^wlan' | awk '{print $1}'")
>

No need to grep and awk, just use a pattern match in awk.


> +    def test_check_wifi_capable(self):
> +        (status, output) = self.target.run('cp /proc/config.gz .; gunzip
> config.gz; cat config | grep CONFIG_IWLWIFI=')
>

"gunzip -c" will write to stdout, so this can be replaced with gunzip -c
/proc/config.gz | grep -q CONFIG_IWLWIFI=.


> --- /dev/null
> +++ b/meta/lib/oeqa/utils/helper.py
> @@ -0,0 +1,62 @@
> +#[PROTEXCAT]
> +#\License: ALL RIGHTS RESERVED
> +#\Author: Wang, Jing <jing.j.wang@intel.com>
>

What's [PROTEXCAT] mean?  That license isn't right for something that's
meant to be in oe-core (typically, MIT).


> +def shell_cmd(cmd):
> +    """Execute shell command till it return"""
> +    cmd_proc = subprocess.Popen(cmd, shell=True)
> +    return cmd_proc.wait() if cmd_proc else -1
>

This is just a bad reimplementation of subprocess.call().  Use call().


> +def shell_cmd_timeout(cmd, timeout=0):
> +    """Execute shell command till timeout"""
> +    cmd_proc = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE)
> +    if not cmd_proc:
> +        return -1, ''
>

How is subproccess.Popen() expected to return something that evaluates to
false?  This can't happen.


> +    t_timeout, tick = timeout, 2
> +    ret, output = None, ''
> +    while True:
> +        time.sleep(tick)
> +        output = cmd_proc.communicate()[0]
> +        ret = cmd_proc.poll()
> +        if ret is not None:
> +            break
> +
> +        if t_timeout > 0:
> +            t_timeout -= tick
> +
> +        if t_timeout <= 0:
> +            # timeout, kill command
> +            cmd_proc.kill()
> +            ret = -99999
> +            break
> +    return ret, output
>

sshcontrol has a neater way of doing this.  I'm wondering why we don't just
install expect on the target instead of assuming the host has expect and
doing ssh-in-expect.

+def collect_pnp_log(casename, logname, log):
> +    """collect the result log for pnp part"""
> +    curpath = os.getcwd()
> +    if not os.path.exists(casename):
> +        os.makedirs(casename)
> +
> +    logpath = os.path.join(curpath, casename, logname)
> +    logtime = time.strftime("%Y-%m-%d %H:%M:%S")
> +    with open(logpath, "a") as text_file:
> +        text_file.write("%s %s:%s\n" %(logtime, casename, log))
>
> Nothing uses this.


> +def get_files_dir():
> +    """Get directory of supporting files"""
> +    pkgarch = oeRuntimeTest.tc.d.getVar('MACHINE', True)
> +    deploydir = oeRuntimeTest.tc.d.getVar('DEPLOY_DIR', True)
> +    return os.path.join(deploydir, "files", "target", pkgarch)
>

Or this.


> +def get_native_dir():
> +    """Get directory of native files"""
> +    arch = oeRuntimeTest.tc.d.getVar('BUILD_ARCH', True)
> +    deploydir = oeRuntimeTest.tc.d.getVar('DEPLOY_DIR', True)
> +    return os.path.join(deploydir, "files", "native", arch)
>

Or this.

Ross

[-- Attachment #2: Type: text/html, Size: 5647 bytes --]