[PATCH 1/3] oeqa/selftest: clean up selftest.inc in teardown

[PATCH 1/3] oeqa/selftest: clean up selftest.inc in teardown

[leonardo.sandoval.gonzalez]

From: Ross Burton <ross.burton@intel.com>

Test cases may want to do call bitbake in setUpClass() but at that point the
previous selftest.inc is still present which could change the build
configuration and result in any built artifacts being removed in the next
bitbake invocation as part of the sysroot clean up.

Resolve this by cleaning selftest.inc in the tearDown, the clean in setUp should
be considered a safety net.

[YOCTO #8466]

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/lib/oeqa/selftest/base.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oeqa/selftest/base.py b/meta/lib/oeqa/selftest/base.py
index b2faa66..9bddc23 100644
--- a/meta/lib/oeqa/selftest/base.py
+++ b/meta/lib/oeqa/selftest/base.py
@@ -31,7 +31,7 @@ class oeSelfTest(unittest.TestCase):
         self.testinc_bblayers_path = os.path.join(self.builddir, "conf/bblayers.inc")
         self.testlayer_path = oeSelfTest.testlayer_path
         self._extra_tear_down_commands = []
-        self._track_for_cleanup = []
+        self._track_for_cleanup = [self.testinc_path]
         super(oeSelfTest, self).__init__(methodName)
 
     def setUp(self):
-- 
1.8.4.5

[PATCH 3/3] oeqa/utils/ftools: Checks before appending/reading files

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Before trying to append/read a file, check if file exists.

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/meta/lib/oeqa/utils/ftools.py b/meta/lib/oeqa/utils/ftools.py
index 64ebe3d..70a55b8 100644
--- a/meta/lib/oeqa/utils/ftools.py
+++ b/meta/lib/oeqa/utils/ftools.py
@@ -8,20 +8,24 @@ def write_file(path, data):
 
 def append_file(path, data):
     wdata = data.rstrip() + "\n"
-    with open(path, "a") as f:
+    if os.path.isfile(path):
+        with open(path, "a") as f:
             f.write(wdata)
 
 def read_file(path):
     data = None
-    with open(path) as f:
-        data = f.read()
+    if os.path.isfile(path):
+        with open(path) as f:
+            data = f.read()
     return data
 
 def remove_from_file(path, data):
-    lines = read_file(path).splitlines()
-    rmdata = data.strip().splitlines()
-    for l in rmdata:
-        for c in range(0, lines.count(l)):
-            i = lines.index(l)
-            del(lines[i])
-    write_file(path, "\n".join(lines))
+    rawdata = read_file(path)
+    if rawdata:
+        lines = rawdata.splitlines()
+        rmdata = data.strip().splitlines()
+        for l in rmdata:
+            for c in range(0, lines.count(l)):
+                i = lines.index(l)
+                del(lines[i])
+        write_file(path, "\n".join(lines))
-- 
1.8.4.5

Re: [PATCH 3/3] oeqa/utils/ftools: Checks before appending/reading files

[Paul Eggleton]

Hi Leo,

On Monday 19 October 2015 05:24:44 leonardo.sandoval.gonzalez@linux.intel.com 
wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Before trying to append/read a file, check if file exists.
> 
> Signed-off-by: Leonardo Sandoval
> <leonardo.sandoval.gonzalez@linux.intel.com> ---
>  meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/meta/lib/oeqa/utils/ftools.py b/meta/lib/oeqa/utils/ftools.py
> index 64ebe3d..70a55b8 100644
> --- a/meta/lib/oeqa/utils/ftools.py
> +++ b/meta/lib/oeqa/utils/ftools.py
> @@ -8,20 +8,24 @@ def write_file(path, data):
> 
>  def append_file(path, data):
>      wdata = data.rstrip() + "\n"
> -    with open(path, "a") as f:
> +    if os.path.isfile(path):
> +        with open(path, "a") as f:
>              f.write(wdata)

Hang on - opening a nonexistent file with mode 'a' is perfectly fine, it'll just 
get created - why do we need this check?


>  def read_file(path):
>      data = None
> -    with open(path) as f:
> -        data = f.read()
> +    if os.path.isfile(path):
> +        with open(path) as f:
> +            data = f.read()
>      return data
> 
>  def remove_from_file(path, data):
> -    lines = read_file(path).splitlines()
> -    rmdata = data.strip().splitlines()
> -    for l in rmdata:
> -        for c in range(0, lines.count(l)):
> -            i = lines.index(l)
> -            del(lines[i])
> -    write_file(path, "\n".join(lines))
> +    rawdata = read_file(path)
> +    if rawdata:
> +        lines = rawdata.splitlines()
> +        rmdata = data.strip().splitlines()
> +        for l in rmdata:
> +            for c in range(0, lines.count(l)):
> +                i = lines.index(l)
> +                del(lines[i])
> +        write_file(path, "\n".join(lines))

Checking a file exists before opening it isn't good practice. It's much better 
to try opening it and if the open fails with IOError of errno.ENOENT then 
ignore it (or rather, if e.errno  != errno.ENOENT then re-raise the 
exception).

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 3/3] oeqa/utils/ftools: Checks before appending/reading files

[Leonardo Sandoval]

Hi Paul,

On 10/19/2015 08:41 AM, Paul Eggleton wrote:
> Hi Leo,
>
> On Monday 19 October 2015 05:24:44 leonardo.sandoval.gonzalez@linux.intel.com
> wrote:
>> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
>>
>> Before trying to append/read a file, check if file exists.
>>
>> Signed-off-by: Leonardo Sandoval
>> <leonardo.sandoval.gonzalez@linux.intel.com> ---
>>   meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/meta/lib/oeqa/utils/ftools.py b/meta/lib/oeqa/utils/ftools.py
>> index 64ebe3d..70a55b8 100644
>> --- a/meta/lib/oeqa/utils/ftools.py
>> +++ b/meta/lib/oeqa/utils/ftools.py
>> @@ -8,20 +8,24 @@ def write_file(path, data):
>>
>>   def append_file(path, data):
>>       wdata = data.rstrip() + "\n"
>> -    with open(path, "a") as f:
>> +    if os.path.isfile(path):
>> +        with open(path, "a") as f:
>>               f.write(wdata)
>
> Hang on - opening a nonexistent file with mode 'a' is perfectly fine, it'll just
> get created - why do we need this check?

You are right, there is no need.

>
>
>>   def read_file(path):
>>       data = None
>> -    with open(path) as f:
>> -        data = f.read()
>> +    if os.path.isfile(path):
>> +        with open(path) as f:
>> +            data = f.read()
>>       return data
>>
>>   def remove_from_file(path, data):
>> -    lines = read_file(path).splitlines()
>> -    rmdata = data.strip().splitlines()
>> -    for l in rmdata:
>> -        for c in range(0, lines.count(l)):
>> -            i = lines.index(l)
>> -            del(lines[i])
>> -    write_file(path, "\n".join(lines))
>> +    rawdata = read_file(path)
>> +    if rawdata:
>> +        lines = rawdata.splitlines()
>> +        rmdata = data.strip().splitlines()
>> +        for l in rmdata:
>> +            for c in range(0, lines.count(l)):
>> +                i = lines.index(l)
>> +                del(lines[i])
>> +        write_file(path, "\n".join(lines))
>
> Checking a file exists before opening it isn't good practice. It's much better
> to try opening it and if the open fails with IOError of errno.ENOENT then
> ignore it (or rather, if e.errno  != errno.ENOENT then re-raise the
 > exception).

After applying patch into master

http://lists.openembedded.org/pipermail/openembedded-core/2015-October/111704.html

I saw this error:

Traceback (most recent call last):
   File 
"/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/selftest/base.py", 
line 110, in remove_config
     ftools.remove_from_file(self.testinc_path, data)
   File 
"/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/utils/ftools.py", 
line 21, in remove_from_file
     lines = read_file(path).splitlines()
   File 
"/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/utils/ftools.py", 
line 16, in read_file
     with open(path) as f:
IOError: [Errno 2] No such file or directory: 
'/srv/ab/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/build/conf/selftest.inc'


That is why I created the latter check. I will change it to use IOError, 
safer and cleaner. Sending V2 today.

Thanks for your comments.

>
> Cheers,
> Paul
>

[PATCH 2/3] oeqa/selftest/gummiboot: Use write instead of append_config

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Instead of appending the configuration, it is safer to create a new
config file, thus avoiding appending to a previous configuration.

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py b/meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py
index 00aa36f..70a5584 100644
--- a/meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py
+++ b/meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py
@@ -17,7 +17,7 @@ class Gummiboot(oeSelfTest):
         # Set EFI_PROVIDER = "gummiboot" and MACHINE = "genericx86-64" in conf/local.conf
         features = 'EFI_PROVIDER = "gummiboot"\n'
         features += 'MACHINE = "genericx86-64"'
-        self.append_config(features)
+        self.write_config(features)
 
     def _common_build(self):
         """
-- 
1.8.4.5