support/tests: allow properly indented config fragment

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.

Quoted 6 lines

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?

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" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 support/testing/infra/builder.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

support/testing/infra/builder.py +0 -0

index a475bb0a30..81735dec96 100644

support/testing/infra/builder.py +0 -0

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

Quoted 19 lines

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" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 support/testing/infra/builder.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

support/testing/infra/builder.py +0 -0

index a475bb0a30..81735dec96 100644
@@ -16,7 +16,8 @@ class Builder(object):
 

support/testing/infra/builder.py class Builder(object): · line 20

Quoted 5 lines

support/testing/infra/builder.py +2 -1

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

Arnout, All,

On 2017-07-18 21:37 +0200, Arnout Vandecappelle spake thusly:

Quoted 3 lines

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?

Quoted 4 lines

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() ).

Quoted 3 lines

Arnout, All,

On 2017-07-18 21:37 +0200, Arnout Vandecappelle spake thusly:

[snip]

Quoted 20 lines

Quoted 3 lines

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

Quoted 1 line

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.

Quoted 6 lines

Quoted 3 lines

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