| Message ID | 20170826222056.6119-3-ricardo.martincoski@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 27-08-17 00:20, Ricardo Martincoski wrote: > From: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br> > > Some test cases can use only the configure step as setup, while the > build is part of the test itself. > Allow the caller to build() to override the logfile used. This way all > the log for the actual test can be directed to *-run.log. This feels weird to me. If you need this kind of argument, then there is something wrong with the logfile member of Builder. Your explanation here seems to suggest that the only reason to have this is to split the -build and the -run logs so the -run logs contains something useful for the git tests. I don't think that that's a good enough reason. What's wrong with collecting all the output in the -build log? But looking at later patches, it looks like you're actually trying to solve a problem here that a file is opened from two places and they run over each other. If that is the case, I think a better solution needs to be found to handle the log files. Could be as simple as the log file being maintained by the test class itself, and passed as an argument to the Builder functions every time. Regards, Arnout > > This change will be needed to make nice logs for git downloader test > cases. > > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br> > --- > Changes v1 -> v2: > - new patch to adapt the test infra to test git download > --- > support/testing/infra/builder.py | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/support/testing/infra/builder.py b/support/testing/infra/builder.py > index de7c141a76..d634c211ac 100644 > --- a/support/testing/infra/builder.py > +++ b/support/testing/infra/builder.py > @@ -46,7 +46,13 @@ class Builder(object): > # that calls make. > # e.g. env={"BR2_DL_DIR": "/path"} > # > - def build(self, makecmdline=None, env=None): > + # ovrdlogfile: when the build is part of the actual test case instead of > + # the setup, this parameter can be used to override the logfile used. > + # e.g. ovrdlogfile=handler_for_the_run_log > + # > + def build(self, makecmdline=None, env=None, ovrdlogfile=None): > + logfile = ovrdlogfile or self.logfile > + > buildenv = os.environ.copy() > if env: > buildenv.update(env) > @@ -55,7 +61,7 @@ class Builder(object): > if makecmdline: > cmd += makecmdline > > - ret = subprocess.call(cmd, stdout=self.logfile, stderr=self.logfile, > + ret = subprocess.call(cmd, stdout=logfile, stderr=logfile, > env=buildenv) > if ret != 0: > raise SystemError("Build failed") >
Hello, On Fri, Oct 06, 2017 at 05:50 PM, Arnout Vandecappelle <arnout@mind.be> wrote: > On 27-08-17 00:20, Ricardo Martincoski wrote: >> From: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br> >> >> Some test cases can use only the configure step as setup, while the >> build is part of the test itself. >> Allow the caller to build() to override the logfile used. This way all >> the log for the actual test can be directed to *-run.log. > > This feels weird to me. If you need this kind of argument, then there is Rethinking now, after your comments, this patch is hackish. > something wrong with the logfile member of Builder. > > Your explanation here seems to suggest that the only reason to have this is to > split the -build and the -run logs so the -run logs contains something useful > for the git tests. I don't think that that's a good enough reason. What's wrong > with collecting all the output in the -build log? Nothing wrong. I will remove this patch from the series. > > But looking at later patches, it looks like you're actually trying to solve a > problem here that a file is opened from two places and they run over each other. > If that is the case, I think a better solution needs to be found to handle the > log files. Could be as simple as the log file being maintained by the test class > itself, and passed as an argument to the Builder functions every time. That's not the case. I was only trying to get all the "setup" output to -build.log and all the "run" output to -run.log. Silly and not needed. Regards, Ricardo
diff --git a/support/testing/infra/builder.py b/support/testing/infra/builder.py index de7c141a76..d634c211ac 100644 --- a/support/testing/infra/builder.py +++ b/support/testing/infra/builder.py @@ -46,7 +46,13 @@ class Builder(object): # that calls make. # e.g. env={"BR2_DL_DIR": "/path"} # - def build(self, makecmdline=None, env=None): + # ovrdlogfile: when the build is part of the actual test case instead of + # the setup, this parameter can be used to override the logfile used. + # e.g. ovrdlogfile=handler_for_the_run_log + # + def build(self, makecmdline=None, env=None, ovrdlogfile=None): + logfile = ovrdlogfile or self.logfile + buildenv = os.environ.copy() if env: buildenv.update(env) @@ -55,7 +61,7 @@ class Builder(object): if makecmdline: cmd += makecmdline - ret = subprocess.call(cmd, stdout=self.logfile, stderr=self.logfile, + ret = subprocess.call(cmd, stdout=logfile, stderr=logfile, env=buildenv) if ret != 0: raise SystemError("Build failed")