Skip to contentSkip to conversation

oeqa/selftest: clean up selftest.inc in teardown

PATCH In review [email protected] 2 participants 5 messages Oct 19, 2015 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

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

From: Ross Burton <[email protected]>

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]

---
 meta/lib/oeqa/selftest/base.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/lib/oeqa/selftest/base.py +1 -1
index b2faa66..9bddc23 100644
@@ -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):
Signature · 1 line
1.8.4.5
#2 lore raw

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

In reply to a message not in the archive

From: Leonardo Sandoval <[email protected]>

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

---
 meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta-yocto-bsp/lib/oeqa/selftest/gummiboot.py +1 -1
index 00aa36f..70a5584 100644
@@ -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):
         """
Signature · 1 line
1.8.4.5
#3 lore raw

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

In reply to a message not in the archive

From: Leonardo Sandoval <[email protected]>

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

---
 meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
meta/lib/oeqa/utils/ftools.py +14 -10
index 64ebe3d..70a55b8 100644
@@ -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))
Signature · 1 line
1.8.4.5
#4 lore raw

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

Hi Leo,

Quoted 14 lines · On Monday 19 October 2015 05:24:44 [email protected]…

From: Leonardo Sandoval <[email protected]>

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

Signed-off-by: Leonardo Sandoval
<[email protected]> ---

 meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
meta/lib/oeqa/utils/ftools.py +0 -0
index 64ebe3d..70a55b8 100644

meta/lib/oeqa/utils/ftools.py def write_file(path, data): · line 13

Quoted 7 lines
meta/lib/oeqa/utils/ftools.py +2 -1
@@ -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?

Quoted 16 lines
meta/lib/oeqa/utils/ftools.py +3 -8
 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])

meta/lib/oeqa/utils/ftools.py def write_file(path, data): · line 31

Quoted 10 lines
meta/lib/oeqa/utils/ftools.py +9 -1
-    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

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#5 lore raw

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

Hi Paul,

Quoted 19 lines · Paul Eggleton

Hi Leo,

Quoted 15 lines · On Monday 19 October 2015 05:24:44 [email protected]…

From: Leonardo Sandoval <[email protected]>

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

Signed-off-by: Leonardo Sandoval
<[email protected]> ---

  meta/lib/oeqa/utils/ftools.py | 24 ++++++++++++++----------
  1 file changed, 14 insertions(+), 10 deletions(-)
meta/lib/oeqa/utils/ftools.py +0 -0
index 64ebe3d..70a55b8 100644
@@ -8,20 +8,24 @@ def write_file(path, data):
 
Quoted 9 lines

meta/lib/oeqa/utils/ftools.py def write_file(path, data): · line 13

Quoted 6 lines
meta/lib/oeqa/utils/ftools.py +2 -1
  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.

Quoted 29 lines
Quoted 18 lines
meta/lib/oeqa/utils/ftools.py +3 -8
  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])

meta/lib/oeqa/utils/ftools.py def write_file(path, data): · line 31

Quoted 10 lines
meta/lib/oeqa/utils/ftools.py +9 -1
-    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))
Quoted 4 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.

Quoted 4 lines

Cheers,
Paul