[3/3] oeqa/selftest/runtime_test: simplify postinst testing

Message ID 20171122160414.14093-3-ross.burton@intel.com
State Accepted
Commit 781a1be88f5812157a231bf5771a01bb978bfcd9
Headers show
Series
  • [1/3] oeqa/commands: don't break if get_bb_vars is passed a tuple
Related show

Commit Message

Ross Burton Nov. 22, 2017, 4:04 p.m.
Update the packages and file names to reflect the new postinst recipe.

Fix a sh syntax error in the run_serial file exists test which was hidden by a
logic problem in the status code.

Remove the older test_verify_postinst as it's effectively a subset of
test_postinst_rootfs_and_boot, and doesn't work: when booting under systemd the
strings it searches for are not output to the console, but the test still
passes.

Signed-off-by: Ross Burton <ross.burton@intel.com>

---
 meta/lib/oeqa/selftest/cases/runtime_test.py | 82 +++++++---------------------
 1 file changed, 19 insertions(+), 63 deletions(-)

-- 
2.11.0

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Comments

Joshua Lock Nov. 23, 2017, 1:34 a.m. | #1
On 23/11/2017 00:04, Ross Burton wrote:
> Update the packages and file names to reflect the new postinst recipe.

> 

> Fix a sh syntax error in the run_serial file exists test which was hidden by a

> logic problem in the status code.

> 

> Remove the older test_verify_postinst as it's effectively a subset of

> test_postinst_rootfs_and_boot, and doesn't work: when booting under systemd the

> strings it searches for are not output to the console, but the test still

> passes.

> 

> Signed-off-by: Ross Burton <ross.burton@intel.com>

> ---

>   meta/lib/oeqa/selftest/cases/runtime_test.py | 82 +++++++---------------------

>   1 file changed, 19 insertions(+), 63 deletions(-)

> 

> diff --git a/meta/lib/oeqa/selftest/cases/runtime_test.py b/meta/lib/oeqa/selftest/cases/runtime_test.py

> index 25270b7535b..1c69255b568 100644

> --- a/meta/lib/oeqa/selftest/cases/runtime_test.py

> +++ b/meta/lib/oeqa/selftest/cases/runtime_test.py

> @@ -167,55 +167,6 @@ class TestImage(OESelftestTestCase):

>   

>   class Postinst(OESelftestTestCase):

>       @OETestID(1540)


We're effectively stating that test ID 1540 and test ID 1545 are both 
covered by the same test method, right?

Perhaps we don't need to maintain both test cases in Testopia, as we'll 
only ever see both tests pass or fail?

> -    def test_verify_postinst(self):

> -        """

> -        Summary: The purpose of this test is to verify the execution order of postinst Bugzilla ID: [5319]

> -        Expected :

> -        1. Compile a minimal image.

> -        2. The compiled image will add the created layer with the recipes postinst[ abdpt]

> -        3. Run qemux86

> -        4. Validate the task execution order

> -        Author: Francisco Pedraza <francisco.j.pedraza.gonzalez@intel.com>

> -        """

> -        features = 'INHERIT += "testimage"\n'

> -        features += 'CORE_IMAGE_EXTRA_INSTALL += "postinst-at-rootfs \

> -postinst-delayed-a \

> -postinst-delayed-b \

> -postinst-delayed-d \

> -postinst-delayed-p \

> -postinst-delayed-t \

> -"\n'

> -        self.write_config(features)

> -

> -        bitbake('core-image-minimal -f ')

> -

> -        postinst_list = ['100-postinst-at-rootfs',

> -                         '101-postinst-delayed-a',

> -                         '102-postinst-delayed-b',

> -                         '103-postinst-delayed-d',

> -                         '104-postinst-delayed-p',

> -                         '105-postinst-delayed-t']

> -        path_workdir = get_bb_var('WORKDIR','core-image-minimal')

> -        workspacedir = 'testimage/qemu_boot_log'

> -        workspacedir = os.path.join(path_workdir, workspacedir)

> -        rexp = re.compile("^Running postinst .*/(?P<postinst>.*)\.\.\.$")

> -        with runqemu('core-image-minimal') as qemu:

> -            with open(workspacedir) as f:

> -                found = False

> -                idx = 0

> -                for line in f.readlines():

> -                    line = line.strip().replace("^M","")

> -                    if not line: # To avoid empty lines

> -                        continue

> -                    m = rexp.search(line)

> -                    if m:

> -                        self.assertEqual(postinst_list[idx], m.group('postinst'), "Fail")

> -                        idx = idx+1

> -                        found = True

> -                    elif found:

> -                        self.assertEqual(idx, len(postinst_list), "Not found all postinsts")

> -                        break

> -

>       @OETestID(1545)

>       def test_postinst_rootfs_and_boot(self):

>           """

> @@ -234,16 +185,22 @@ postinst-delayed-t \

>                           for initialization managers: sysvinit and systemd.

>   

>           """

> -        file_rootfs_name = "this-was-created-at-rootfstime"

> -        fileboot_name = "this-was-created-at-first-boot"

> -        rootfs_pkg = 'postinst-at-rootfs'

> -        boot_pkg = 'postinst-delayed-a'

> +

> +        import oe.path

> +

> +        vars = get_bb_vars(("IMAGE_ROOTFS", "sysconfdir"), "core-image-minimal")

> +        rootfs = vars["IMAGE_ROOTFS"]

> +        self.assertIsNotNone(rootfs)

> +        sysconfdir = vars["sysconfdir"]

> +        self.assertIsNotNone(sysconfdir)

> +        # Need to use oe.path here as sysconfdir starts with /

> +        hosttestdir = oe.path.join(rootfs, sysconfdir, "postinst-test")

> +        targettestdir = os.path.join(sysconfdir, "postinst-test")

>   

>           for init_manager in ("sysvinit", "systemd"):

>               for classes in ("package_rpm", "package_deb", "package_ipk"):

>                   with self.subTest(init_manager=init_manager, package_class=classes):

> -                    features = 'MACHINE = "qemux86"\n'

> -                    features += 'CORE_IMAGE_EXTRA_INSTALL += "%s %s "\n'% (rootfs_pkg, boot_pkg)

> +                    features = 'CORE_IMAGE_EXTRA_INSTALL = "postinst-delayed-b"\n'

>                       features += 'IMAGE_FEATURES += "package-management empty-root-password"\n'


The indentation doesn't look right here?

>                       features += 'PACKAGE_CLASSES = "%s"\n' % classes

>                       if init_manager == "systemd":

> @@ -255,13 +212,12 @@ postinst-delayed-t \

>   

>                       bitbake('core-image-minimal')

>   

> -                    file_rootfs_created = os.path.join(get_bb_var('IMAGE_ROOTFS', "core-image-minimal"),

> -                                                       file_rootfs_name)

> -                    found = os.path.isfile(file_rootfs_created)

> -                    self.assertTrue(found, "File %s was not created at rootfs time by %s" % \

> -                                    (file_rootfs_name, rootfs_pkg))

> +                    self.assertTrue(os.path.isfile(os.path.join(hosttestdir, "rootfs")),

> +                                    "rootfs state file was not created")

>   

> -                    testcommand = 'ls /etc/' + fileboot_name

>                       with runqemu('core-image-minimal') as qemu:

> -                        status, output = qemu.run_serial("-f /etc/" + fileboot_name)

> -                        self.assertEqual(status, 0, 'File %s was not created at first boot (%s)' % (fileboot_name, output))

> +                        # Make the test echo a string and search for that as

> +                        # run_serial()'s status code is useless.'

> +                        for filename in ("rootfs", "delayed-a", "delayed-b"):

> +                            status, output = qemu.run_serial("test -f %s && echo found" % os.path.join(targettestdir, filename))

> +                            self.assertEqual(output, "found", "%s was not present on boot" % filename)

> 


Why not just test the status (return code of test -f) instead of echoing 
a known value? i.e.

self.assertEqual(status, 0, "%s was not present on boot" % filename)

Joshua
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Ross Burton Nov. 23, 2017, 10:21 a.m. | #2
On 23 November 2017 at 01:34, Joshua Lock <joshua.g.lock@linux.intel.com>
wrote:

>       @OETestID(1540)

>>

>

> We're effectively stating that test ID 1540 and test ID 1545 are both

> covered by the same test method, right?

>

> Perhaps we don't need to maintain both test cases in Testopia, as we'll

> only ever see both tests pass or fail?



Yes, reviewing and sanitising testopia was my next job.

-                    features = 'MACHINE = "qemux86"\n'
>> -                    features += 'CORE_IMAGE_EXTRA_INSTALL += "%s %s

>> "\n'% (rootfs_pkg, boot_pkg)

>> +                    features = 'CORE_IMAGE_EXTRA_INSTALL =

>> "postinst-delayed-b"\n'

>>                       features += 'IMAGE_FEATURES += "package-management

>> empty-root-password"\n'

>>

>

> The indentation doesn't look right here?



That would be Atom "helpfully" doing smart intentation.  I'll fix it up in
something else, thanks.


> +                            status, output = qemu.run_serial("test -f %s

>> && echo found" % os.path.join(targettestdir, filename))

>> +                            self.assertEqual(output, "found", "%s was

>> not present on boot" % filename)

>>

>>

> Why not just test the status (return code of test -f) instead of echoing a

> known value? i.e.

>

> self.assertEqual(status, 0, "%s was not present on boot" % filename)



Because run_serial() is a bit lame and I don't trust it to correctly report
the exit status.   Rewriting it is on my hitlist, yes.

Ross
<div dir="ltr">On 23 November 2017 at 01:34, Joshua Lock <span dir="ltr">&lt;<a href="mailto:joshua.g.lock@linux.intel.com" target="_blank">joshua.g.lock@linux.intel.com</a>&gt;</span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">      @OETestID(1540)<br>
</blockquote>
<br></span>
We&#39;re effectively stating that test ID 1540 and test ID 1545 are both covered by the same test method, right?<br>
<br>
Perhaps we don&#39;t need to maintain both test cases in Testopia, as we&#39;ll only ever see both tests pass or fail?</blockquote><div><br></div><div>Yes, reviewing and sanitising testopia was my next job.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">-                    features = &#39;MACHINE = &quot;qemux86&quot;\n&#39;<br>
-                    features += &#39;CORE_IMAGE_EXTRA_INSTALL += &quot;%s %s &quot;\n&#39;% (rootfs_pkg, boot_pkg)<br>
+                    features = &#39;CORE_IMAGE_EXTRA_INSTALL = &quot;postinst-delayed-b&quot;\n&#39;<br>
                      features += &#39;IMAGE_FEATURES += &quot;package-management empty-root-password&quot;\n&#39;<br>
</blockquote>
<br></div></div>
The indentation doesn&#39;t look right here?</blockquote><div><br></div><div>That would be Atom &quot;helpfully&quot; doing smart intentation.  I&#39;ll fix it up in something else, thanks.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span class="gmail-">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">+                            status, output = qemu.run_serial(&quot;test -f %s &amp;&amp; echo found&quot; % os.path.join(targettestdir, filename))<br>
+                            self.assertEqual(output, &quot;found&quot;, &quot;%s was not present on boot&quot; % filename)<br>
<br>
</blockquote>
<br></span>
Why not just test the status (return code of test -f) instead of echoing a known value? i.e.<br>
<br>
self.assertEqual(status, 0, &quot;%s was not present on boot&quot; % filename)</blockquote><div><br></div><div>Because run_serial() is a bit lame and I don&#39;t trust it to correctly report the exit status.   Rewriting it is on my hitlist, yes.</div><div><br></div><div>Ross</div></div></div></div>
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Ross Burton Nov. 23, 2017, 12:08 p.m. | #3
On 23 November 2017 at 10:21, Burton, Ross <ross.burton@intel.com> wrote:

>

>> The indentation doesn't look right here?

>

>

> That would be Atom "helpfully" doing smart intentation.  I'll fix it up in

> something else, thanks.

>


Can't see any problems, what didn't look right?

Ross
<div dir="ltr">On 23 November 2017 at 10:21, Burton, Ross <span dir="ltr">&lt;<a href="mailto:ross.burton@intel.com" target="_blank">ross.burton@intel.com</a>&gt;</span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div class="m_8341726617125964381gmail-h5"><br></div></div>
The indentation doesn&#39;t look right here?</blockquote><div><br></div></span><div>That would be Atom &quot;helpfully&quot; doing smart intentation.  I&#39;ll fix it up in something else, thanks.</div></div></div></div></blockquote><div><br></div><div>Can&#39;t see any problems, what didn&#39;t look right?</div><div><br></div><div>Ross </div></div></div></div>
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Patch

diff --git a/meta/lib/oeqa/selftest/cases/runtime_test.py b/meta/lib/oeqa/selftest/cases/runtime_test.py
index 25270b7535b..1c69255b568 100644
--- a/meta/lib/oeqa/selftest/cases/runtime_test.py
+++ b/meta/lib/oeqa/selftest/cases/runtime_test.py
@@ -167,55 +167,6 @@  class TestImage(OESelftestTestCase):
 
 class Postinst(OESelftestTestCase):
     @OETestID(1540)
-    def test_verify_postinst(self):
-        """
-        Summary: The purpose of this test is to verify the execution order of postinst Bugzilla ID: [5319]
-        Expected :
-        1. Compile a minimal image.
-        2. The compiled image will add the created layer with the recipes postinst[ abdpt]
-        3. Run qemux86
-        4. Validate the task execution order
-        Author: Francisco Pedraza <francisco.j.pedraza.gonzalez@intel.com>
-        """
-        features = 'INHERIT += "testimage"\n'
-        features += 'CORE_IMAGE_EXTRA_INSTALL += "postinst-at-rootfs \
-postinst-delayed-a \
-postinst-delayed-b \
-postinst-delayed-d \
-postinst-delayed-p \
-postinst-delayed-t \
-"\n'
-        self.write_config(features)
-
-        bitbake('core-image-minimal -f ')
-
-        postinst_list = ['100-postinst-at-rootfs',
-                         '101-postinst-delayed-a',
-                         '102-postinst-delayed-b',
-                         '103-postinst-delayed-d',
-                         '104-postinst-delayed-p',
-                         '105-postinst-delayed-t']
-        path_workdir = get_bb_var('WORKDIR','core-image-minimal')
-        workspacedir = 'testimage/qemu_boot_log'
-        workspacedir = os.path.join(path_workdir, workspacedir)
-        rexp = re.compile("^Running postinst .*/(?P<postinst>.*)\.\.\.$")
-        with runqemu('core-image-minimal') as qemu:
-            with open(workspacedir) as f:
-                found = False
-                idx = 0
-                for line in f.readlines():
-                    line = line.strip().replace("^M","")
-                    if not line: # To avoid empty lines
-                        continue
-                    m = rexp.search(line)
-                    if m:
-                        self.assertEqual(postinst_list[idx], m.group('postinst'), "Fail")
-                        idx = idx+1
-                        found = True
-                    elif found:
-                        self.assertEqual(idx, len(postinst_list), "Not found all postinsts")
-                        break
-
     @OETestID(1545)
     def test_postinst_rootfs_and_boot(self):
         """
@@ -234,16 +185,22 @@  postinst-delayed-t \
                         for initialization managers: sysvinit and systemd.
 
         """
-        file_rootfs_name = "this-was-created-at-rootfstime"
-        fileboot_name = "this-was-created-at-first-boot"
-        rootfs_pkg = 'postinst-at-rootfs'
-        boot_pkg = 'postinst-delayed-a'
+
+        import oe.path
+
+        vars = get_bb_vars(("IMAGE_ROOTFS", "sysconfdir"), "core-image-minimal")
+        rootfs = vars["IMAGE_ROOTFS"]
+        self.assertIsNotNone(rootfs)
+        sysconfdir = vars["sysconfdir"]
+        self.assertIsNotNone(sysconfdir)
+        # Need to use oe.path here as sysconfdir starts with /
+        hosttestdir = oe.path.join(rootfs, sysconfdir, "postinst-test")
+        targettestdir = os.path.join(sysconfdir, "postinst-test")
 
         for init_manager in ("sysvinit", "systemd"):
             for classes in ("package_rpm", "package_deb", "package_ipk"):
                 with self.subTest(init_manager=init_manager, package_class=classes):
-                    features = 'MACHINE = "qemux86"\n'
-                    features += 'CORE_IMAGE_EXTRA_INSTALL += "%s %s "\n'% (rootfs_pkg, boot_pkg)
+                    features = 'CORE_IMAGE_EXTRA_INSTALL = "postinst-delayed-b"\n'
                     features += 'IMAGE_FEATURES += "package-management empty-root-password"\n'
                     features += 'PACKAGE_CLASSES = "%s"\n' % classes
                     if init_manager == "systemd":
@@ -255,13 +212,12 @@  postinst-delayed-t \
 
                     bitbake('core-image-minimal')
 
-                    file_rootfs_created = os.path.join(get_bb_var('IMAGE_ROOTFS', "core-image-minimal"),
-                                                       file_rootfs_name)
-                    found = os.path.isfile(file_rootfs_created)
-                    self.assertTrue(found, "File %s was not created at rootfs time by %s" % \
-                                    (file_rootfs_name, rootfs_pkg))
+                    self.assertTrue(os.path.isfile(os.path.join(hosttestdir, "rootfs")),
+                                    "rootfs state file was not created")
 
-                    testcommand = 'ls /etc/' + fileboot_name
                     with runqemu('core-image-minimal') as qemu:
-                        status, output = qemu.run_serial("-f /etc/" + fileboot_name)
-                        self.assertEqual(status, 0, 'File %s was not created at first boot (%s)' % (fileboot_name, output))
+                        # Make the test echo a string and search for that as
+                        # run_serial()'s status code is useless.'
+                        for filename in ("rootfs", "delayed-a", "delayed-b"):
+                            status, output = qemu.run_serial("test -f %s && echo found" % os.path.join(targettestdir, filename))
+                            self.assertEqual(output, "found", "%s was not present on boot" % filename)