Openembedded Core Discussions
 help / color / mirror / Atom feed
From: "Stanacar, StefanX" <stefanx.stanacar@intel.com>
To: "sipke.vriend@xilinx.com" <sipke.vriend@xilinx.com>
Cc: "openembedded-core@lists.openembedded.org"
	<openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH] lib/oeqa: allow a layer to provide it's own TEST_TARGET class
Date: Thu, 23 Jan 2014 08:27:31 +0000	[thread overview]
Message-ID: <1390465720.1197.8.camel@firebird.rb.intel.com> (raw)
In-Reply-To: <e7ee03fa-b951-4d84-afd9-3a0cb987b18f@AM1EHSMHS005.ehs.local>

Hi Sipke,

On Thu, 2014-01-23 at 14:48 +1000, Sipke Vriend wrote:
> Hi Stefan,
> 
> I have started using this patch in master to extend test targets to our
> bsp layer, and it works well. Thanks.
> Will the extension of a controllers.py file work for multiple bsp layers
> in a single build/conf/bblayers.conf?
> I did a quick test and think that the first controller.py found is the
> only one "extended". If this is true then  the current implementation
> could get a false negative for a given TEST_TARGET, if more than one
> layer implements controllers.py.

Good catch!
You are right, only the first one found is imported (and that probably
has to do with how imports are made and the order of layers - lib/ is
added to pythonpath for all layers AFAIK)

> 
> I tried a minor modification which assumes each testtarget class is in
> its own file with the name <testtarget>.py (lower case). This extended
> across multiple layers, assuming no duplication of test target names. I
> put them in the utils directory for the test, but they could go into a
> controllers folder as that might make more sense.

Looks good, and yes, a oeqa.controllers package might make more sense.
Now, the only thing that bothers me is using testtarget's value as the
filename (as it is now it was supposed to be the class name). Naming
things is hard...
Anyway, could you please send it as a patch?

Cheers,
Stefan

> 
> diff --git a/meta/lib/oeqa/targetcontrol.py b/meta/lib/oeqa/targetcontrol.py
> index 757f9d3..d7cab7e 100644
> --- a/meta/lib/oeqa/targetcontrol.py
> +++ b/meta/lib/oeqa/targetcontrol.py
> @@ -28,10 +28,11 @@ def get_target_controller(d):
>           except AttributeError:
>               # nope, perhaps a layer defined one
>               try:
> -                module = __import__("oeqa.utils.controllers",
> globals(), locals(), [testtarget])
> +                modulename = "oeqa.utils.{0}".format(testtarget.lower())
> +                module = __import__(modulename, globals(), locals(),
> [testtarget])
>                   controller = getattr(module, testtarget)
>               except ImportError as e:
> -                bb.fatal("Failed to import oeqa.utils.controllers:\n%s"
> % traceback.format_exc())
> +                bb.fatal("Failed to import
> {0}:\n{1}".format(modulename,traceback.format_exc()))
>               except AttributeError:
>                   bb.fatal("\"%s\" is not a valid value for TEST_TARGET"
> % testtarget)
>           return controller(d)
> 
> Cheers
> Sipke
> 
> On 16/01/2014 10:48 PM, Stefan Stanacar wrote:
> > Allows a layer to define new classes in<layer>/lib/oeqa/utils/controllers.py
> > and completely control or extend deployment of a target. (core currently
> > has QemuTarget and SimpleRemoteTarget).
> > The value of TEST_TARGET must be the name of the new class.
> >
> > Signed-off-by: Stefan Stanacar<stefanx.stanacar@intel.com>
> > ---
> >   meta/lib/oeqa/targetcontrol.py  | 23 +++++++++++++++++++----
> >   meta/lib/oeqa/utils/__init__.py |  3 +++
> >   2 files changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/meta/lib/oeqa/targetcontrol.py b/meta/lib/oeqa/targetcontrol.py
> > index dee38ec..757f9d3 100644
> > --- a/meta/lib/oeqa/targetcontrol.py
> > +++ b/meta/lib/oeqa/targetcontrol.py
> > @@ -8,18 +8,33 @@ import os
> >   import shutil
> >   import subprocess
> >   import bb
> > -
> > +import traceback
> >   from oeqa.utils.sshcontrol import SSHControl
> >   from oeqa.utils.qemurunner import QemuRunner
> >
> >
> >   def get_target_controller(d):
> > -    if d.getVar("TEST_TARGET", True) == "qemu":
> > +    testtarget = d.getVar("TEST_TARGET", True)
> > +    # old, simple names
> > +    if testtarget == "qemu":
> >           return QemuTarget(d)
> > -    elif d.getVar("TEST_TARGET", True) == "simpleremote":
> > +    elif testtarget == "simpleremote":
> >           return SimpleRemoteTarget(d)
> >       else:
> > -        bb.fatal("Please set a valid TEST_TARGET")
> > +        # use the class name
> > +        try:
> > +            # is it a core class defined here?
> > +            controller = getattr(__name__, testtarget)
> > +        except AttributeError:
> > +            # nope, perhaps a layer defined one
> > +            try:
> > +                module = __import__("oeqa.utils.controllers", globals(), locals(), [testtarget])
> > +                controller = getattr(module, testtarget)
> > +            except ImportError as e:
> > +                bb.fatal("Failed to import oeqa.utils.controllers:\n%s" % traceback.format_exc())
> > +            except AttributeError:
> > +                bb.fatal("\"%s\" is not a valid value for TEST_TARGET" % testtarget)
> > +        return controller(d)
> >
> >
> >   class BaseTarget(object):
> > diff --git a/meta/lib/oeqa/utils/__init__.py b/meta/lib/oeqa/utils/__init__.py
> > index e69de29..8eda927 100644
> > --- a/meta/lib/oeqa/utils/__init__.py
> > +++ b/meta/lib/oeqa/utils/__init__.py
> > @@ -0,0 +1,3 @@
> > +# Enable other layers to have modules in the same named directory
> > +from pkgutil import extend_path
> > +__path__ = extend_path(__path__, __name__)
> >
> 
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 


  reply	other threads:[~2014-01-23  8:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-16 12:48 [PATCH] lib/oeqa: allow a layer to provide it's own TEST_TARGET class Stefan Stanacar
2014-01-23  4:48 ` Sipke Vriend
2014-01-23  8:27   ` Stanacar, StefanX [this message]
  -- strict thread matches above, loose matches on Subject: below --
2014-01-23 22:44 Sipke Vriend

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1390465720.1197.8.camel@firebird.rb.intel.com \
    --to=stefanx.stanacar@intel.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=sipke.vriend@xilinx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox