Code review comment for lp:~nuclearbob/utah/bug1155615

On 04/26/2013 01:01 PM, Max Brustkern wrote:

> https://code.launchpad.net/~nuclearbob/utah/bug1155615/+merge/161205

> === modified file 'utah/client/runner.py'

> def __init__(self, install_type, runlist=None, result_class=Result,
> testdir=UTAH_DIR, state_agent=None,
> - resume=False, old_results=None):
> + resume=False, old_results=None, output=None):
>
> # Runlist URL passed through the command line
> self.testdir = testdir
> self.revision = "Unknown"
> + self.output = (output or
> + os.path.join('/', 'var', 'lib', 'utah', 'utah.out'))

does os.path.join really gain anything here? ie wouldn't it be more
concise to just do "/var/lib/utah/utah.out"? might even default it in
the parameters to the function so you don't have to check for None here.

> with open(rc_local, 'w') as fp:
> - fp.write(self.rc_local_content.format(runlist=runlist))
> + fp.write(self.rc_local_content.format(runlist=runlist,
> + output=self.output))

I think you forgot to update rc_local_content at the top of the file to
take in this new parameter. ie:

rc_local_content = """#!/bin/sh

/usr/bin/utah --resume -o {outout} -r {runlist}
"""

Also - would it be possible to add a test case to ensure we do write to
non-standard output directory?