support/testing: unbreak run-tests -l
PATCH 1/2 support/testing: unbreak run-tests -l
In commit b78b50465c20c1733753a8dd47945cf80c9155f8, the initialisation of BRTest.builddir was moved to the __init__ function. However, it is set based on BRTest.outputdir and that is only set when the -o argument is given to run-tests. When called as "run-tests -l", there is no -o argument so BRTest.outputdir remains unset.
To fix, keep BRTest.builddir at None when BRTest.outputdir is None.
While we're at it, drop the direct access to the class member. If a subclass wishes to set outputdir to something else before calling BRTest.__init__, they are free to do so.
--- support/testing/infra/basetest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
support/testing/infra/basetest.py +1 -1
@@ -39,7 +39,7 @@ class BRTest(unittest.TestCase): def __init__(self, names): super(BRTest, self).__init__(names) self.testname = self.__class__.__name__ - self.builddir = os.path.join(self.__class__.outputdir, self.testname) + self.builddir = self.outputdir and os.path.join(self.outputdir, self.testname) self.emulator = None def show_msg(self, msg):
Signature · 1 line
2.13.2
PATCH 2/2 support/testing: add test of BR2_CCACHE with an external toolchain
We reuse TestExternalToolchainBuildrootuClibc and add ccache to its configuration.
--- support/testing/tests/toolchain/test_external.py | 12 ++++++++++++ 1 file changed, 12 insertions(+)
support/testing/tests/toolchain/test_external.py +12 -0
@@ -229,3 +229,15 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y kernel="builtin", options=["-initrd", img]) self.emulator.login() + +class TestExternalToolchainCCache(TestExternalToolchainBuildrootuClibc): + extraconfig = \ +""" +BR2_CCACHE=y +BR2_CCACHE_DIR="{builddir}/ccache-dir" +""" + + def __init__(self, names): + super(TestExternalToolchainBuildrootuClibc, self).__init__(names) + self.config = self.config + self.extraconfig.format(builddir = self.builddir) +
Signature · 1 line
2.13.2
PATCH 2/2 support/testing: add test of BR2_CCACHE with an external toolchain
Hello,
Quoted 4 lines · Arnout Vandecappelle (Essensium/Mind)
We reuse TestExternalToolchainBuildrootuClibc and add ccache to its configuration.
[snip]
Quoted 1 line
+ self.config = self.config +
self.extraconfig.format(builddir = self.builddir)
3 nits:
- why not += ?
- usually there are no spaces around parameter equals
- line is too long (not that much)
from these I suggest to change to:
self.config += self.extraconfig.format(builddir=self.builddir)
Quoted 2 lines
+
nit: This empty line at end of file is not needed.
with the 4 nits fixed
[the test passes, host-ccache is built either with or without ccache
pre-installed in the host machine, the ccache dir is created inside the builddir and has valid data inside:
$ cd ${builddir} && host/bin/ccache -s
...
cache size 13.1 MB
max cache size 5.0 GB]
Tested-by: Ricardo Martincoski <[email protected]>Regards,
Ricardo
PATCH 2/2 support/testing: add test of BR2_CCACHE with an external toolchain
Quoted 7 lines · Ricardo Martincoski
Hello,
Quoted 3 lines · Arnout Vandecappelle (Essensium/Mind)
We reuse TestExternalToolchainBuildrootuClibc and add ccache to its configuration.
Quoted 6 lines
Quoted 1 line
[snip]
Quoted 1 line
+ self.config = self.config +
self.extraconfig.format(builddir = self.builddir)3 nits:
- why not += ?
Because config is initialized as a class member, and += would update the class
member instead of the instance member like we want. As it happens, this test is executed last so it doesn't matter, but if any test comes after it, it would get the extraconfig as well...
Cfr. the following test:
In [49]: class Foo:
...: foo = [0]
...:In [50]: class Foo1(Foo):
...: def __init__(self):
...: self.foo += [1]
...:In [51]: Foo1().foo
Out[51]: [0, 1]
In [52]: Foo1().foo
Out[52]: [0, 1, 1]
In [53]: class Foo2(Foo):
...: def __init__(self):
...: self.foo = self.foo + [2]
...:In [54]: Foo2().foo
Out[54]: [0, 1, 1, 2]
In [55]: Foo2().foo
Out[55]: [0, 1, 1, 2]
Quoted 2 lines
- usually there are no spaces around parameter equals
- line is too long (not that much)
I'll take care of these, and check it with flake8 while I'm at it.
Quoted 7 lines
from these I suggest to change to:
self.config += self.extraconfig.format(builddir=self.builddir)Quoted 2 lines
+
nit: This empty line at end of file is not needed.
Hm, I usually don't make that kind of mistake :-)
Quoted 3 lines
with the 4 nits fixed
Reviewed-by Ricardo Martincoski <[email protected]>
Can you confirm that this still stands if I don't do the +=, as long as the
line stays below 79 characters?
Regards,
Arnout
Quoted 13 lines
[the test passes, host-ccache is built either with or without ccache
pre-installed in the host machine, the ccache dir is created inside the builddir and has valid data inside:$ cd ${builddir} && host/bin/ccache -s ... cache size 13.1 MB max cache size 5.0 GB] Tested-by: Ricardo Martincoski <[email protected]>Regards,
Ricardo
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 2/2 support/testing: add test of BR2_CCACHE with an external toolchain
Hello,
Quoted 10 lines · Arnout Vandecappelle
Quoted 5 lines · Ricardo Martincoski
[snip]
Quoted 1 line
+ self.config = self.config +
self.extraconfig.format(builddir = self.builddir)3 nits:
- why not += ?Because config is initialized as a class member, and += would update the class
member instead of the instance member like we want. As it happens, this test is executed last so it doesn't matter, but if any test comes after it, it would get
From command line there is no guarantee on execution order when using -t.
In my tests without -t I observe the testcases being executed in a case insensitive reverse alphabetical order, no matter if I move one test to another place in the file.
$ support/testing/run-tests -d d -o o -k tests.toolchain.test_external 22:32:39 TestExternalToolchainSourceryArmv7 Starting 22:32:43 TestExternalToolchainSourceryArmv7 Cleaning up .22:32:43 TestExternalToolchainSourceryArmv5 Starting 22:32:49 TestExternalToolchainSourceryArmv5 Cleaning up .22:32:49 TestExternalToolchainSourceryArmv4 Starting 22:32:54 TestExternalToolchainSourceryArmv4 Cleaning up .22:32:54 TestExternalToolchainLinaroArm Starting 22:32:58 TestExternalToolchainLinaroArm Cleaning up .22:32:58 TestExternalToolchainCtngMusl Starting 22:33:01 TestExternalToolchainCtngMusl Cleaning up .22:33:01 TestExternalToolchainCCache Starting 22:33:04 TestExternalToolchainCCache Cleaning up .22:33:04 TestExternalToolchainBuildrootuClibc Starting 22:33:08 TestExternalToolchainBuildrootuClibc Cleaning up .22:33:08 TestExternalToolchainBuildrootMusl Starting 22:33:11 TestExternalToolchainBuildrootMusl Cleaning up .
But I guess you are talking about gitlab. I never used it.
Quoted 24 lines
the extraconfig as well...
Cfr. the following test:
In [49]: class Foo:
...: foo = [0] ...:In [50]: class Foo1(Foo):
...: def __init__(self): ...: self.foo += [1] ...:In [51]: Foo1().foo
Out[51]: [0, 1]In [52]: Foo1().foo
Out[52]: [0, 1, 1]In [53]: class Foo2(Foo):
...: def __init__(self): ...: self.foo = self.foo + [2] ...:
Quoted 5 lines
In [54]: Foo2().foo
Out[54]: [0, 1, 1, 2]In [55]: Foo2().foo
Out[55]: [0, 1, 1, 2]
The same does not apply to strings, only to mutable objects.
But if i.e. for some reason we change the type of config from string to an array of lines the += would need to be changed, while your code would still work.
[snip]
Quoted 5 lines
Quoted 2 lines
with the 4 nits fixed
Reviewed-by Ricardo Martincoski <[email protected]>Can you confirm that this still stands if I don't do the +=, as long as the
line stays below 79 characters?
Sure. It still stands without += .
Regards,
Ricardo
PATCH 2/2 support/testing: add test of BR2_CCACHE with an external toolchain
Quoted 11 lines · Ricardo Martincoski
Hello,
Quoted 7 lines · Arnout Vandecappelle
Quoted 4 lines · Ricardo Martincoski
[snip]
Quoted 1 line
+ self.config = self.config +
self.extraconfig.format(builddir = self.builddir)3 nits:
- why not += ?Because config is initialized as a class member, and += would update the class
member instead of the instance member like we want.
[snip]
Quoted 1 line
The same does not apply to strings, only to mutable objects.
Right, I didn't think of that! I just learned to be wary of using += when the
left-hand side is a class member (or function argument with a default value), I didn't consider mutability.
Quoted 2 lines
But if i.e. for some reason we change the type of config from string to an array of lines the += would need to be changed, while your code would still work.
However, with Yann's change in '[PATCHv3] support/tests: allow properly
indented config fragment', self.config will already have been updated to be an instance member instead of a class member. Therefore, += should be fine.
I'll respin with all your nits applied.
Regards,
Arnout
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/2 support/testing: unbreak run-tests -l
In reply to Arnout Vandecappelle (Essensium/Mind)Arnout, All,
On 2017-07-10 22:40 +0200, Arnout Vandecappelle (Essensium/Mind) spake thusly:
Quoted 6 lines
In commit
b78b50465c20c1733753a8dd47945cf80c9155f8, the initialisation of BRTest.builddir was moved to the__init__function. However, it is set based on BRTest.outputdir and that is only set when the -o argument is given to run-tests. When called as "run-tests -l", there is no -o argument so BRTest.outputdir remains unset.
Quoted 7 lines
To fix, keep BRTest.builddir at None when BRTest.outputdir is None.
While we're at it, drop the direct access to the class member. If a subclass wishes to set outputdir to something else before calling BRTest.__init__, they are free to do so.
Thanks for the quick fixing! :-)
Regards,
Yann E. MORIN.
Quoted 24 lines
--- support/testing/infra/basetest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)support/testing/infra/basetest.py +1
-1@@ -39,7 +39,7 @@ class BRTest(unittest.TestCase): def __init__(self, names): super(BRTest, self).__init__(names) self.testname = self.__class__.__name__ - self.builddir = os.path.join(self.__class__.outputdir, self.testname) + self.builddir = self.outputdir and os.path.join(self.outputdir, self.testname) self.emulator = None def show_msg(self, msg):Signature · 6 lines
2.13.2 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
Thomas Petazzoni
AppliedPATCH 1/2 support/testing: unbreak run-tests -l
In reply to Arnout Vandecappelle (Essensium/Mind)Hello,
Quoted 8 lines · Arnout Vandecappelle (Essensium/Mind)
In commit
b78b50465c20c1733753a8dd47945cf80c9155f8, the initialisation of BRTest.builddir was moved to the__init__function. However, it is set based on BRTest.outputdir and that is only set when the -o argument is given to run-tests. When called as "run-tests -l", there is no -o argument so BRTest.outputdir remains unset.To fix, keep BRTest.builddir at None when BRTest.outputdir is None.
Quoted 8 lines
While we're at it, drop the direct access to the class member. If a subclass wishes to set outputdir to something else before calling BRTest.__init__, they are free to do so.
--- support/testing/infra/basetest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to master, thanks.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com