Re: [OE-core] [PATCH 20/30] oeqa/selftest/cases: runqemu enable thraded runs
s/thraded/threaded/ in the subject. There are more spelling mistakes elsewhere ("wrapper methos"). I don't know how important that is. On Tue, 2017-07-11 at 15:23 -0500, Aníbal Limón wrote: > - Update to use wrappers {bitbake,get_bb_var} from OESelfTestCase class. s/OESelfTestCase/OESelftestTestCase/ Sorry to be pedantic, but I wanted to look up what these wrappers do and couldn't even find the class ;-} I've found them in the "oeqa/selftest/case: Add wrappers to utils.commands modules" patch. I'm a bit worried that this entire patch series is introducing concepts and methods without any documentation or explanations why things are done this way. I suspect it will make it very hard to write selftests correctly. For example, this patch and others like it seem fairly arbitrary. It doesn't explain why self.bitbake() is better than bitbake(). If in some future patch or test someone were to use bitbake() when they should have used self.bitbake() it's not going to be obvious either whether that is correct. Perhaps all OE tests should have these wrappers and only OESelftestTestCase does something special with them? Then we can gradually replace the direct calls to oeqa.utils.commands completely. Add also a wrapper for runqemu and we can get rid of "from oeqa.utils.commands import" completely. > - Run into the main thread because it needs tinfoil to run. > > Signed-off-by: Aníbal Limón > --- > meta/lib/oeqa/selftest/cases/runqemu.py | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/meta/lib/oeqa/selftest/cases/runqemu.py > b/meta/lib/oeqa/selftest/cases/runqemu.py > index 4050a4123ba..e30cb24046f 100644 > --- a/meta/lib/oeqa/selftest/cases/runqemu.py > +++ b/meta/lib/oeqa/selftest/cases/runqemu.py > @@ -6,12 +6,11 @@ import re > import logging > > from oeqa.selftest.case import OESelftestTestCase > -from oeqa.utils.commands import bitbake, runqemu, get_bb_var > +from oeqa.utils.commands import runqemu > from oeqa.core.decorator.oeid import OETestID > > class RunqemuTests(OESelftestTestCase): > """Runqemu test class""" > - > image_is_ready = False > deploy_dir_image = '' > > @@ -37,8 +36,8 @@ SYSLINUX_TIMEOUT = "10" > ) > > if not RunqemuTests.image_is_ready: > -RunqemuTests.deploy_dir_image = get_bb_var('DEPLOY_DIR_IMAGE') > -bitbake(self.recipe) > +RunqemuTests.deploy_dir_image = > self.get_bb_var('DEPLOY_DIR_IMAGE') > +self.bitbake(self.recipe) > RunqemuTests.image_is_ready = True > > @OETestID(2001) -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH 20/30] oeqa/selftest/cases: runqemu enable thraded runs
On 07/12/2017 02:15 AM, Patrick Ohly wrote: > s/thraded/threaded/ in the subject. There are more spelling mistakes > elsewhere ("wrapper methos"). I don't know how important that is. > > On Tue, 2017-07-11 at 15:23 -0500, Aníbal Limón wrote: >> - Update to use wrappers {bitbake,get_bb_var} from OESelfTestCase class. > > s/OESelfTestCase/OESelftestTestCase/ > > Sorry to be pedantic, but I wanted to look up what these wrappers do and > couldn't even find the class ;-} Yes np, > > I've found them in the "oeqa/selftest/case: Add wrappers to > utils.commands modules" patch. I'm a bit worried that this entire patch > series is introducing concepts and methods without any documentation or > explanations why things are done this way. I suspect it will make it > very hard to write selftests correctly. > > For example, this patch and others like it seem fairly arbitrary. It > doesn't explain why self.bitbake() is better than bitbake(). If in some > future patch or test someone were to use bitbake() when they should have > used self.bitbake() it's not going to be obvious either whether that is > correct. > > Perhaps all OE tests should have these wrappers and only > OESelftestTestCase does something special with them? Then we can > gradually replace the direct calls to oeqa.utils.commands completely. You are right, the reason for add the wrappers is to be able to run bitbake, runCmd, get_bb_var{,s}, in certain build directory. For compatibility reasons the base class OESelftestTestCase is set to use the main thread and the original builddir, and that's the reason to don't delete the oeqa.utils.commands. I guess the best way to address this is to include a README explaining this situation into the meta/lib/oeqa/selftest folder. > > Add also a wrapper for runqemu and we can get rid of "from > oeqa.utils.commands import" completely. Yes we can after the refkit and other selftest are adapted. Cheers, Anibal > >> - Run into the main thread because it needs tinfoil to run. >> >> Signed-off-by: Aníbal Limón >> --- >> meta/lib/oeqa/selftest/cases/runqemu.py | 7 +++ >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/meta/lib/oeqa/selftest/cases/runqemu.py >> b/meta/lib/oeqa/selftest/cases/runqemu.py >> index 4050a4123ba..e30cb24046f 100644 >> --- a/meta/lib/oeqa/selftest/cases/runqemu.py >> +++ b/meta/lib/oeqa/selftest/cases/runqemu.py >> @@ -6,12 +6,11 @@ import re >> import logging >> >> from oeqa.selftest.case import OESelftestTestCase >> -from oeqa.utils.commands import bitbake, runqemu, get_bb_var >> +from oeqa.utils.commands import runqemu >> from oeqa.core.decorator.oeid import OETestID >> >> class RunqemuTests(OESelftestTestCase): >> """Runqemu test class""" >> - >> image_is_ready = False >> deploy_dir_image = '' >> >> @@ -37,8 +36,8 @@ SYSLINUX_TIMEOUT = "10" >> ) >> >> if not RunqemuTests.image_is_ready: >> -RunqemuTests.deploy_dir_image = get_bb_var('DEPLOY_DIR_IMAGE') >> -bitbake(self.recipe) >> +RunqemuTests.deploy_dir_image = >> self.get_bb_var('DEPLOY_DIR_IMAGE') >> +self.bitbake(self.recipe) >> RunqemuTests.image_is_ready = True >> >> @OETestID(2001) > -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH 20/30] oeqa/selftest/cases: runqemu enable thraded runs
On Wed, 2017-07-12 at 10:01 -0500, Aníbal Limón wrote: > > Add also a wrapper for runqemu and we can get rid of "from > > oeqa.utils.commands import" completely. > > Yes we can after the refkit and other selftest are adapted. My proposal wasn't to remove oeqa.utils.commands, only the "import" statements for it in selftest test cases. There might be legitimate uses for oeqa.utils.commands outside of selftests. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH 20/30] oeqa/selftest/cases: runqemu enable thraded runs
On 07/14/2017 04:35 AM, Patrick Ohly wrote: > On Wed, 2017-07-12 at 10:01 -0500, Aníbal Limón wrote: >>> Add also a wrapper for runqemu and we can get rid of "from >>> oeqa.utils.commands import" completely. >> >> Yes we can after the refkit and other selftest are adapted. > > My proposal wasn't to remove oeqa.utils.commands, only the "import" > statements for it in selftest test cases. There might be legitimate uses > for oeqa.utils.commands outside of selftests. Agree, Anibal > -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core