Merge lp://qastaging/~liuyq0307/lava-android-test/extract-attachments into lp://qastaging/lava-android-test

On 08/06/2012 02:02 AM, Yongqin Liu wrote:
> === modified file 'lava_android_test/commands.py'
> --- lava_android_test/commands.py 2012-06-28 11:29:49 +0000
> +++ lava_android_test/commands.py 2012-08-06 07:01:28 +0000
<snip>
> + def invoke(self):
> +
> + if not os.path.exists(self.args.result_file):
> + raise LavaCommandError("The specified result file(%s) "
> + "is not existed." % self.args.result_file)

Bad grammar: This should be "The specified file(%s) does not exist"

> + msg = "extract attachment file from result bundle file(%s)" % (
> + self.args.result_file)
> + self.say_begin(msg)
> + badchars = "[^a-zA-Z0-9\._-]"

badchars? looks like these are "good" chars?

> + with open(self.args.result_file) as stream:
> + jobdata = stream.read()
> + result_data = DocumentIO.loads(jobdata)[1]
> + test_runs = result_data.get('test_runs')
> + attachment_dir = mkdtemp(prefix='attachments-', dir=os.path.curdir)
> + for test in test_runs:
> + test_id = test.get('test_id').replace(" ", "_")
> + test_id = re.sub(badchars, "_", test_id)
> + target_dir = mkdtemp(prefix='%s' % test_id, dir=attachment_dir)

maybe this should be a command line option for the user to specify the
output directory?

> + print "The test id is: %s" % test_id
> + attachments = test.get('attachments', [])
> + for attach in attachments:
> + pathname = attach.get('pathname')
> + file_name = os.path.basename(pathname)
> + content_decoded = base64.standard_b64decode(
> + attach.get("content"))
> + with open(os.path.join(target_dir, file_name), 'a') as fd:

you are opening the file in append mode, shouldn't it just be 'w' instead?

> On 08/06/2012 02:02 AM, Yongqin Liu wrote:
> > === modified file 'lava_android_test/commands.py'
> > --- lava_android_test/commands.py 2012-06-28 11:29:49 +0000
> > +++ lava_android_test/commands.py 2012-08-06 07:01:28 +0000
> <snip>
> > + def invoke(self):
> > +
> > + if not os.path.exists(self.args.result_file):
> > + raise LavaCommandError("The specified result file(%s) "
> > + "is not existed." %
> self.args.result_file)
>
> Bad grammar: This should be "The specified file(%s) does not exist"
Thanks, really very very bad:(

> > + msg = "extract attachment file from result bundle file(%s)" % (
> > +
> self.args.result_file)
> > + self.say_begin(msg)
> > + badchars = "[^a-zA-Z0-9\._-]"
>
> badchars? looks like these are "good" chars?
yes, the badchars are the chars that not "a-zA-Z0-9\._-" here.

> > + with open(self.args.result_file) as stream:
> > + jobdata = stream.read()
> > + result_data = DocumentIO.loads(jobdata)[1]
> > + test_runs = result_data.get('test_runs')
> > + attachment_dir = mkdtemp(prefix='attachments-',
> dir=os.path.curdir)
> > + for test in test_runs:
> > + test_id = test.get('test_id').replace(" ", "_")
> > + test_id = re.sub(badchars, "_", test_id)
> > + target_dir = mkdtemp(prefix='%s' % test_id,
> dir=attachment_dir)
>
> maybe this should be a command line option for the user to specify the
> output directory?
I add a directory option to support user specification.

> > + print "The test id is: %s" % test_id
> > + attachments = test.get('attachments', [])
> > + for attach in attachments:
> > + pathname = attach.get('pathname')
> > + file_name = os.path.basename(pathname)
> > + content_decoded = base64.standard_b64decode(
> > +
> attach.get("content"))
> > + with open(os.path.join(target_dir, file_name), 'a') as
> fd:
>
> you are opening the file in append mode, shouldn't it just be 'w' instead?
Thanks, change it to w