[PATCH 0/3] Fix bitbake-selftest failures

[PATCH 0/3] Fix bitbake-selftest failures

[Mark Hatle]

The original purpose of this work is to fix the Yocto Project bug 12928.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=12928

During that work it was noticed that parse.py was running chdir without
restoring the original path.  Also the layerindexlib cooker test was
using __file__ without fully sanitizing it, so due to the prior issue
it could fail to load the test-case data.

The patch set is availabe on bitbake-contrib on the mgh/12928 branch.

Mark Hatle (3):
  lib/bb/tests/parse.py: Test case was changing chdir
  lib/layerindexlib/tests/cooker.py: Fix topdir to use an absolute
    (real) path
  lib/layerindexlib/tests/restapi.py: Fix test cases when
    BB_SKIP_NETTETS=yes

 lib/bb/tests/parse.py              |   4 +
 lib/layerindexlib/tests/cooker.py  |   2 +-
 lib/layerindexlib/tests/restapi.py | 164 ++++++++++++++++++++-----------------
 3 files changed, 92 insertions(+), 78 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] lib/bb/tests/parse.py: Test case was changing chdir

[Mark Hatle]

The test case was changing the current directory, but was never restoring it
to the original location.  This causes occasional failures in later test cases.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/bb/tests/parse.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/bb/tests/parse.py b/lib/bb/tests/parse.py
index 8f16ba4..1bc4740 100644
--- a/lib/bb/tests/parse.py
+++ b/lib/bb/tests/parse.py
@@ -44,9 +44,13 @@ C = "3"
 """
 
     def setUp(self):
+        self.origdir = os.getcwd()
         self.d = bb.data.init()
         bb.parse.siggen = bb.siggen.init(self.d)
 
+    def tearDown(self):
+        os.chdir(self.origdir)
+
     def parsehelper(self, content, suffix = ".bb"):
 
         f = tempfile.NamedTemporaryFile(suffix = suffix)
-- 
1.8.3.1

[PATCH 2/3] lib/layerindexlib/tests/cooker.py: Fix topdir to use an absolute (real) path

[Mark Hatle]

The test case needs to access test case files.  Different versions of python
may return absolute or relative locations in __file__.  Use the same approach
as other test cases in determining the location of the test files.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/layerindexlib/tests/cooker.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/layerindexlib/tests/cooker.py b/lib/layerindexlib/tests/cooker.py
index 9ce6e8c..46d006b 100644
--- a/lib/layerindexlib/tests/cooker.py
+++ b/lib/layerindexlib/tests/cooker.py
@@ -32,7 +32,7 @@ class LayerIndexCookerTest(LayersTest):
         # configure the test data.  But we can emulate the basics of the layer.conf
         # files, so that is what we will do.
 
-        new_topdir = os.path.join(os.path.dirname(__file__), "testdata")
+        new_topdir = topdir = os.path.join(os.path.dirname(os.path.realpath(__file__)), "testdata")
         new_bbpath = os.path.join(new_topdir, "build")
 
         self.d.setVar('TOPDIR', new_topdir)
-- 
1.8.3.1

[PATCH 3/3] lib/layerindexlib/tests/restapi.py: Fix test cases when BB_SKIP_NETTETS=yes

[Mark Hatle]

Change the way the network related tests are skipped, based on how the
fetch tests are handled.  We introduce a 'skipIfNoNetwork()' function that
will report skipped tests through the standard unit test reporting system.

[YOCTO #12928]

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 lib/layerindexlib/tests/restapi.py | 164 ++++++++++++++++++++-----------------
 1 file changed, 87 insertions(+), 77 deletions(-)

diff --git a/lib/layerindexlib/tests/restapi.py b/lib/layerindexlib/tests/restapi.py
index bfaac43..5876695 100644
--- a/lib/layerindexlib/tests/restapi.py
+++ b/lib/layerindexlib/tests/restapi.py
@@ -23,113 +23,122 @@ from layerindexlib.tests.common import LayersTest
 
 import logging
 
-class LayerIndexWebRestApiTest(LayersTest):
-
+def skipIfNoNetwork():
     if os.environ.get("BB_SKIP_NETTESTS") == "yes":
-        print("Unset BB_SKIP_NETTESTS to run network tests")
-    else:
-        def setUp(self):
-            LayersTest.setUp(self)
-            self.layerindex = layerindexlib.LayerIndex(self.d)
-            self.layerindex.load_layerindex('http://layers.openembedded.org/layerindex/api/;branch=sumo', load=['layerDependencies'])
+        return unittest.skip("Network tests being skipped")
+    return lambda f: f
 
-        def test_layerindex_is_empty(self):
-            self.assertFalse(self.layerindex.is_empty(), msg="Layerindex is empty")
+class LayerIndexWebRestApiTest(LayersTest):
 
-        def test_layerindex_store_file(self):
-            self.layerindex.store_layerindex('file://%s/file.json' % self.tempdir, self.layerindex.indexes[0])
+    @skipIfNoNetwork()
+    def setUp(self):
+        self.assertFalse(os.environ.get("BB_SKIP_NETTESTS") == "yes", msg="BB_SKIP_NETTESTS set, but we tried to test anyway")
+        LayersTest.setUp(self)
+        self.layerindex = layerindexlib.LayerIndex(self.d)
+        self.layerindex.load_layerindex('http://layers.openembedded.org/layerindex/api/;branch=sumo', load=['layerDependencies'])
 
-            self.assertTrue(os.path.isfile('%s/file.json' % self.tempdir), msg="Temporary file was not created by store_layerindex")
+    @skipIfNoNetwork()
+    def test_layerindex_is_empty(self):
+        self.assertFalse(self.layerindex.is_empty(), msg="Layerindex is empty")
 
-            reload = layerindexlib.LayerIndex(self.d)
-            reload.load_layerindex('file://%s/file.json' % self.tempdir)
+    @skipIfNoNetwork()
+    def test_layerindex_store_file(self):
+        self.layerindex.store_layerindex('file://%s/file.json' % self.tempdir, self.layerindex.indexes[0])
 
-            self.assertFalse(reload.is_empty(), msg="Layerindex is empty")
+        self.assertTrue(os.path.isfile('%s/file.json' % self.tempdir), msg="Temporary file was not created by store_layerindex")
 
-            # Calculate layerItems in original index that should NOT be in reload
-            layerItemNames = []
-            for itemId in self.layerindex.indexes[0].layerItems:
-                layerItemNames.append(self.layerindex.indexes[0].layerItems[itemId].name)
+        reload = layerindexlib.LayerIndex(self.d)
+        reload.load_layerindex('file://%s/file.json' % self.tempdir)
 
-            for layerBranchId in self.layerindex.indexes[0].layerBranches:
-                layerItemNames.remove(self.layerindex.indexes[0].layerBranches[layerBranchId].layer.name)
+        self.assertFalse(reload.is_empty(), msg="Layerindex is empty")
 
-            for itemId in reload.indexes[0].layerItems:
-                self.assertFalse(reload.indexes[0].layerItems[itemId].name in layerItemNames, msg="Item reloaded when it shouldn't have been")
+        # Calculate layerItems in original index that should NOT be in reload
+        layerItemNames = []
+        for itemId in self.layerindex.indexes[0].layerItems:
+            layerItemNames.append(self.layerindex.indexes[0].layerItems[itemId].name)
 
-            # Compare the original to what we wrote...
-            for type in self.layerindex.indexes[0]._index:
-                if type == 'apilinks' or \
-                   type == 'layerItems' or \
-                   type in self.layerindex.indexes[0].config['local']:
-                    continue
-                for id in getattr(self.layerindex.indexes[0], type):
-                    self.logger.debug(1, "type %s" % (type))
+        for layerBranchId in self.layerindex.indexes[0].layerBranches:
+            layerItemNames.remove(self.layerindex.indexes[0].layerBranches[layerBranchId].layer.name)
 
-                    self.assertTrue(id in getattr(reload.indexes[0], type), msg="Id number not in reloaded index")
+        for itemId in reload.indexes[0].layerItems:
+            self.assertFalse(reload.indexes[0].layerItems[itemId].name in layerItemNames, msg="Item reloaded when it shouldn't have been")
 
-                    self.logger.debug(1, "%s ? %s" % (getattr(self.layerindex.indexes[0], type)[id], getattr(reload.indexes[0], type)[id]))
+        # Compare the original to what we wrote...
+        for type in self.layerindex.indexes[0]._index:
+            if type == 'apilinks' or \
+               type == 'layerItems' or \
+               type in self.layerindex.indexes[0].config['local']:
+                continue
+            for id in getattr(self.layerindex.indexes[0], type):
+                self.logger.debug(1, "type %s" % (type))
 
-                    self.assertEqual(getattr(self.layerindex.indexes[0], type)[id], getattr(reload.indexes[0], type)[id], msg="Reloaded contents different")
+                self.assertTrue(id in getattr(reload.indexes[0], type), msg="Id number not in reloaded index")
 
-        def test_layerindex_store_split(self):
-            self.layerindex.store_layerindex('file://%s' % self.tempdir, self.layerindex.indexes[0])
+                self.logger.debug(1, "%s ? %s" % (getattr(self.layerindex.indexes[0], type)[id], getattr(reload.indexes[0], type)[id]))
 
-            reload = layerindexlib.LayerIndex(self.d)
-            reload.load_layerindex('file://%s' % self.tempdir)
+                self.assertEqual(getattr(self.layerindex.indexes[0], type)[id], getattr(reload.indexes[0], type)[id], msg="Reloaded contents different")
 
-            self.assertFalse(reload.is_empty(), msg="Layer index is empty")
+    @skipIfNoNetwork()
+    def test_layerindex_store_split(self):
+        self.layerindex.store_layerindex('file://%s' % self.tempdir, self.layerindex.indexes[0])
 
-            for type in self.layerindex.indexes[0]._index:
-                if type == 'apilinks' or \
-                   type == 'layerItems' or \
-                   type in self.layerindex.indexes[0].config['local']:
-                    continue
-                for id in getattr(self.layerindex.indexes[0] ,type):
-                    self.logger.debug(1, "type %s" % (type))
+        reload = layerindexlib.LayerIndex(self.d)
+        reload.load_layerindex('file://%s' % self.tempdir)
 
-                    self.assertTrue(id in getattr(reload.indexes[0], type), msg="Id number missing from reloaded data")
+        self.assertFalse(reload.is_empty(), msg="Layer index is empty")
 
-                    self.logger.debug(1, "%s ? %s" % (getattr(self.layerindex.indexes[0] ,type)[id], getattr(reload.indexes[0], type)[id]))
+        for type in self.layerindex.indexes[0]._index:
+            if type == 'apilinks' or \
+               type == 'layerItems' or \
+               type in self.layerindex.indexes[0].config['local']:
+                continue
+            for id in getattr(self.layerindex.indexes[0] ,type):
+                self.logger.debug(1, "type %s" % (type))
 
-                    self.assertEqual(getattr(self.layerindex.indexes[0] ,type)[id], getattr(reload.indexes[0], type)[id], msg="reloaded data does not match original")
+                self.assertTrue(id in getattr(reload.indexes[0], type), msg="Id number missing from reloaded data")
 
-        def test_dependency_resolution(self):
-            # Verify depth first searching...
-            (dependencies, invalidnames) = self.layerindex.find_dependencies(names=['meta-python'])
+                self.logger.debug(1, "%s ? %s" % (getattr(self.layerindex.indexes[0] ,type)[id], getattr(reload.indexes[0], type)[id]))
 
-            first = True
-            for deplayerbranch in dependencies:
-                layerBranch = dependencies[deplayerbranch][0]
-                layerDeps = dependencies[deplayerbranch][1:]
+                self.assertEqual(getattr(self.layerindex.indexes[0] ,type)[id], getattr(reload.indexes[0], type)[id], msg="reloaded data does not match original")
 
-                if not first:
-                    continue
+    @skipIfNoNetwork()
+    def test_dependency_resolution(self):
+        # Verify depth first searching...
+        (dependencies, invalidnames) = self.layerindex.find_dependencies(names=['meta-python'])
 
-                first = False
+        first = True
+        for deplayerbranch in dependencies:
+            layerBranch = dependencies[deplayerbranch][0]
+            layerDeps = dependencies[deplayerbranch][1:]
 
-                # Top of the deps should be openembedded-core, since everything depends on it.
-                self.assertEqual(layerBranch.layer.name, "openembedded-core", msg='OpenEmbedded-Core is no the first dependency')
+            if not first:
+                continue
 
-                # meta-python should cause an openembedded-core dependency, if not assert!
-                for dep in layerDeps:
-                    if dep.layer.name == 'meta-python':
-                        break
-                else:
-                    self.logger.debug(1, "meta-python was not found")
-                    self.assetTrue(False)
+            first = False
 
-                # Only check the first element...
-                break
+            # Top of the deps should be openembedded-core, since everything depends on it.
+            self.assertEqual(layerBranch.layer.name, "openembedded-core", msg='OpenEmbedded-Core is no the first dependency')
+
+            # meta-python should cause an openembedded-core dependency, if not assert!
+            for dep in layerDeps:
+                if dep.layer.name == 'meta-python':
+                    break
             else:
-                # Empty list, this is bad.
-                self.logger.debug(1, "Empty list of dependencies")
-                self.assertIsNotNone(first, msg="Empty list of dependencies")
+                self.logger.debug(1, "meta-python was not found")
+                self.assetTrue(False)
+
+            # Only check the first element...
+            break
+        else:
+            # Empty list, this is bad.
+            self.logger.debug(1, "Empty list of dependencies")
+            self.assertIsNotNone(first, msg="Empty list of dependencies")
 
-                # Last dep should be the requested item
-                layerBranch = dependencies[deplayerbranch][0]
-                self.assertEqual(layerBranch.layer.name, "meta-python", msg="Last dependency not meta-python")
+            # Last dep should be the requested item
+            layerBranch = dependencies[deplayerbranch][0]
+            self.assertEqual(layerBranch.layer.name, "meta-python", msg="Last dependency not meta-python")
 
+    @skipIfNoNetwork()
     def test_find_collection(self):
         def _check(collection, expected):
             self.logger.debug(1, "Looking for collection %s..." % collection)
@@ -149,6 +158,7 @@ class LayerIndexWebRestApiTest(LayersTest):
         for collection,result in tests:
             _check(collection, result)
 
+    @skipIfNoNetwork()
     def test_find_layerbranch(self):
         def _check(name, expected):
             self.logger.debug(1, "Looking for layerbranch %s..." % name)
-- 
1.8.3.1

Re: [PATCH 2/3] lib/layerindexlib/tests/cooker.py: Fix topdir to use an absolute (real) path

[Mark Hatle]

On 9/25/18 10:07 AM, Mark Hatle wrote:
> The test case needs to access test case files.  Different versions of python
> may return absolute or relative locations in __file__.  Use the same approach
> as other test cases in determining the location of the test files.
> 
> Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
> ---
>  lib/layerindexlib/tests/cooker.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/layerindexlib/tests/cooker.py b/lib/layerindexlib/tests/cooker.py
> index 9ce6e8c..46d006b 100644
> --- a/lib/layerindexlib/tests/cooker.py
> +++ b/lib/layerindexlib/tests/cooker.py
> @@ -32,7 +32,7 @@ class LayerIndexCookerTest(LayersTest):
>          # configure the test data.  But we can emulate the basics of the layer.conf
>          # files, so that is what we will do.
>  
> -        new_topdir = os.path.join(os.path.dirname(__file__), "testdata")
> +        new_topdir = topdir = os.path.join(os.path.dirname(os.path.realpath(__file__)), "testdata")

Ugh, copy paste failure.  I've sent a new 2/3 to correct the above line.
(Should not have the = topdir part!)

--Mark

>          new_bbpath = os.path.join(new_topdir, "build")
>  
>          self.d.setVar('TOPDIR', new_topdir)
>