[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Yann E. MORIN]

Currently, defining a config fragment in the runtime test infra requires
that the fragment not to be indented. This is beark, and causes grievance
when looking at the code (e.g. to fix it).

Just strip out all leading spaces/tabs when writing the configuration
lines into the config file.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 support/testing/infra/builder.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/support/testing/infra/builder.py b/support/testing/infra/builder.py
index a475bb0a30..81735dec96 100644
--- a/support/testing/infra/builder.py
+++ b/support/testing/infra/builder.py
@@ -16,7 +16,8 @@ class Builder(object):
 
         config_file = os.path.join(self.builddir, ".config")
         with open(config_file, "w+") as cf:
-            cf.write(self.config)
+            for line in self.config.splitlines():
+                cf.write("{}\n".format(line.lstrip()))
 
         cmd = ["make",
                "O={}".format(self.builddir),
-- 
2.11.0

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Thomas Petazzoni]

Hello,

On Sun, 16 Jul 2017 00:49:49 +0200, Yann E. MORIN wrote:
> Currently, defining a config fragment in the runtime test infra requires
> that the fragment not to be indented. This is beark, and causes grievance
> when looking at the code (e.g. to fix it).
> 
> Just strip out all leading spaces/tabs when writing the configuration
> lines into the config file.

Seems good in principle, but could you give an example of an indented
config fragment you intend to use?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Yann E. MORIN]

Thomas, All,

On 2017-07-16 09:18 +0200, Thomas Petazzoni spake thusly:
> On Sun, 16 Jul 2017 00:49:49 +0200, Yann E. MORIN wrote:
> > Currently, defining a config fragment in the runtime test infra requires
> > that the fragment not to be indented. This is beark, and causes grievance
> > when looking at the code (e.g. to fix it).
> > 
> > Just strip out all leading spaces/tabs when writing the configuration
> > lines into the config file.
> 
> Seems good in principle, but could you give an example of an indented
> config fragment you intend to use?

Sure. Here's a diff against master, for example:

    diff --git a/support/testing/tests/fs/test_squashfs.py b/support/testing/tests/fs/test_squashfs.py
    index b205b6a55a..9fad28f834 100644
    --- a/support/testing/tests/fs/test_squashfs.py
    +++ b/support/testing/tests/fs/test_squashfs.py
    @@ -5,12 +5,12 @@ import infra.basetest
     
     class TestSquashfs(infra.basetest.BRTest):
         config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
    -"""
    -BR2_TARGET_ROOTFS_SQUASHFS=y
    -# BR2_TARGET_ROOTFS_SQUASHFS4_GZIP is not set
    -BR2_TARGET_ROOTFS_SQUASHFS4_LZ4=y
    -# BR2_TARGET_ROOTFS_TAR is not set
    -"""
    +        """
    +        BR2_TARGET_ROOTFS_SQUASHFS=y
    +        # BR2_TARGET_ROOTFS_SQUASHFS4_GZIP is not set
    +        BR2_TARGET_ROOTFS_SQUASHFS4_LZ4=y
    +        # BR2_TARGET_ROOTFS_TAR is not set
    +        """
     
         def test_run(self):
             unsquashfs_cmd = ["host/bin/unsquashfs", "-s", "images/rootfs.squashfs"]

And here is a big file which is easier to read with indented config
fragments:
    https://git.buildroot.org/~ymorin/git/buildroot/tree/support/testing/tests/init/test_systemd.py?h=yem/systemd-skeleton-2

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Arnout Vandecappelle]

On 16-07-17 00:49, Yann E. MORIN wrote:
> Currently, defining a config fragment in the runtime test infra requires
> that the fragment not to be indented. This is beark, and causes grievance
> when looking at the code (e.g. to fix it).
> 
> Just strip out all leading spaces/tabs when writing the configuration
> lines into the config file.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  support/testing/infra/builder.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/support/testing/infra/builder.py b/support/testing/infra/builder.py
> index a475bb0a30..81735dec96 100644
> --- a/support/testing/infra/builder.py
> +++ b/support/testing/infra/builder.py
> @@ -16,7 +16,8 @@ class Builder(object):
>  
>          config_file = os.path.join(self.builddir, ".config")
>          with open(config_file, "w+") as cf:
> -            cf.write(self.config)
> +            for line in self.config.splitlines():
> +                cf.write("{}\n".format(line.lstrip()))

 I like nitpicking :-)

 I would do this in BRTest instead of down here, so that self.config contains a
correct config file. In fact, I would do it in its constructor instead of in
setUp(), so that it is done only once even if a class has two test functions.

 I would also do it as a oneliner:

	self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))

but some people don't like oneliners :-)


 Regards,
 Arnout

>  
>          cmd = ["make",
>                 "O={}".format(self.builddir),
> 

-- 
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

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Yann E. MORIN]

Arnout, All,

On 2017-07-18 21:37 +0200, Arnout Vandecappelle spake thusly:
> On 16-07-17 00:49, Yann E. MORIN wrote:
> > Currently, defining a config fragment in the runtime test infra requires
> > that the fragment not to be indented. This is beark, and causes grievance
> > when looking at the code (e.g. to fix it).
> > 
> > Just strip out all leading spaces/tabs when writing the configuration
> > lines into the config file.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  support/testing/infra/builder.py | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/support/testing/infra/builder.py b/support/testing/infra/builder.py
> > index a475bb0a30..81735dec96 100644
> > --- a/support/testing/infra/builder.py
> > +++ b/support/testing/infra/builder.py
> > @@ -16,7 +16,8 @@ class Builder(object):
> >  
> >          config_file = os.path.join(self.builddir, ".config")
> >          with open(config_file, "w+") as cf:
> > -            cf.write(self.config)
> > +            for line in self.config.splitlines():
> > +                cf.write("{}\n".format(line.lstrip()))
> 
>  I like nitpicking :-)

Please nit-pick, python is not my cup of tea, you know well! ;-)

>  I would do this in BRTest instead of down here, so that self.config contains a
> correct config file. In fact, I would do it in its constructor instead of in
> setUp(), so that it is done only once even if a class has two test functions.

Meh, I have no real clue on how those class- or instance-scoped
variables play iwth each others... I think this is sad that we use
class-scoped config variable for the actual tests; they should be
instance-scoped instead, but that requires one to actually write an
__init__ function, so a classscoped is quicker to write.

Be if we ever want to derive a Test class into another, then we'll have
issues I guess...

But if I understand you correctly, you'd prefer (code elided for
brevity):

    class BRTest():
        def __init__(self):
            self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))

Right?

>  I would also do it as a oneliner:
> 	self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))
> but some people don't like oneliners :-)

Well, I prefer it, because it is rather self-explanatory. But I could
not have wrote it myself (I did not know about map() ).

Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Arnout Vandecappelle]

On 18-07-17 21:45, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-07-18 21:37 +0200, Arnout Vandecappelle spake thusly:
[snip]
>>  I would do this in BRTest instead of down here, so that self.config contains a
>> correct config file. In fact, I would do it in its constructor instead of in
>> setUp(), so that it is done only once even if a class has two test functions.
> 
> Meh, I have no real clue on how those class- or instance-scoped
> variables play iwth each others... I think this is sad that we use
> class-scoped config variable for the actual tests; they should be
> instance-scoped instead, but that requires one to actually write an
> __init__ function, so a classscoped is quicker to write.
> 
> Be if we ever want to derive a Test class into another, then we'll have
> issues I guess...
> 
> But if I understand you correctly, you'd prefer (code elided for
> brevity):
> 
>     class BRTest():
>         def __init__(self):
>             self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))
> 
> Right?

 Exactly.

 Note that this conveniently converts the class-scoped variable to an
instance-scoped one (the reference on the right-hand side refers to the class
variable, the reference on the left-hand side creates the instance variable).
Still a big Meh for you, but at least after __init__ it feels natural.

> 
>>  I would also do it as a oneliner:
>> 	self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))
>> but some people don't like oneliners :-)
> 
> Well, I prefer it, because it is rather self-explanatory. But I could
> not have wrote it myself (I did not know about map() ).

 TBH, nowadays Python people prefer the [ for ] construct, so

    self.config = '\n'.join([line.lstrip for line in self.config.splitlines()])

I'm still an old LISP guy so I know map() better :-)

 Regards,
 Arnout

-- 
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

[Buildroot] [PATCH] support/tests: allow properly indented config fragment

[Yann E. MORIN]

Arnout, All,

On 2017-07-18 21:55 +0200, Arnout Vandecappelle spake thusly:
> On 18-07-17 21:45, Yann E. MORIN wrote:
> > Arnout, All,
> > 
> > On 2017-07-18 21:37 +0200, Arnout Vandecappelle spake thusly:
> [snip]
> >>  I would do this in BRTest instead of down here, so that self.config contains a
> >> correct config file. In fact, I would do it in its constructor instead of in
> >> setUp(), so that it is done only once even if a class has two test functions.
> > 
> > Meh, I have no real clue on how those class- or instance-scoped
> > variables play iwth each others... I think this is sad that we use
> > class-scoped config variable for the actual tests; they should be
> > instance-scoped instead, but that requires one to actually write an
> > __init__ function, so a classscoped is quicker to write.
> > 
> > Be if we ever want to derive a Test class into another, then we'll have
> > issues I guess...
> > 
> > But if I understand you correctly, you'd prefer (code elided for
> > brevity):
> > 
> >     class BRTest():
> >         def __init__(self):
> >             self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))
> > 
> > Right?
> 
>  Exactly.
> 
>  Note that this conveniently converts the class-scoped variable to an
> instance-scoped one (the reference on the right-hand side refers to the class
> variable, the reference on the left-hand side creates the instance variable).
> Still a big Meh for you, but at least after __init__ it feels natural.

Ack.

> >>  I would also do it as a oneliner:
> >> 	self.config = '\n'.join(map(str.lstrip, self.config.splitlines()))
> >> but some people don't like oneliners :-)
> > 
> > Well, I prefer it, because it is rather self-explanatory. But I could
> > not have wrote it myself (I did not know about map() ).
> 
>  TBH, nowadays Python people prefer the [ for ] construct, so
> 
>     self.config = '\n'.join([line.lstrip for line in self.config.splitlines()])

Ah, that was the construct I wanted to write, but for the sake of me, I
could not remeber the exact syntax... That's what I'll use, because it
is more pythonic (as you also noted).

> I'm still an old LISP guy so I know map() better :-)

LISP. Meh... A whole lot of insipid and stupid parentehses, that's all I
remember of it. ;-]

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'