Fix 'test_runner.py linker' logcat output

Fix 'test_runner.py linker' logcat output

This patch stops the linker test runner from printing each character in
the logcat output on a separate line.

BUG=460468
Committed: https://crrev.com/1b2552acec5513c6974460d5e7ced2f80e5f94e8
Cr-Commit-Position: refs/heads/master@{#317338}

[petrcermak, 2015-02-20 15:05:55]

petrcermak@chromium.org changed reviewers:
+ jbudorick@chromium.org, perezju@chromium.org

[petrcermak, 2015-02-20 15:05:56]

Hi, please review this tiny patch.

Thanks,
Petr

[jbudorick, 2015-02-20 15:07:49]

While this works, I think the actual fix should have Logcat(dump=True) return an
iterable. It not doing so is a bug (introduced by my CL you referenced in the
bug).

[petrcermak, 2015-02-20 15:10:40]

On 2015/02/20 15:07:49, jbudorick wrote:
> While this works, I think the actual fix should have Logcat(dump=True) return
an
> iterable. It not doing so is a bug (introduced by my CL you referenced in the
> bug).

I'm absolutely fine with changing it, but what was then the point of
https://codereview.chromium.org/919763003 (because it only added this option of
not returning an iterator)?

[jbudorick, 2015-02-20 15:15:29]

On 2015/02/20 15:10:40, petrcermak wrote:
> On 2015/02/20 15:07:49, jbudorick wrote:
> > While this works, I think the actual fix should have Logcat(dump=True)
return
> an
> > iterable. It not doing so is a bug (introduced by my CL you referenced in
the
> > bug).
> 
> I'm absolutely fine with changing it, but what was then the point of
> https://codereview.chromium.org/919763003 (because it only added this option
of
> not returning an iterator)?

Should've been clearer in my comment -- "iterable" encompasses iterators (which
IterRunDeviceAdbCmd returns) and lists. I think Logcat(dump=True) should return
a list. Interested to hear perezju's thoughts, though.

The point of that CL was to avoid using IterRunDeviceAdbCmd because it doesn't
run the command at all if you don't iterate through the returned iterator (which
clients calling Logcat(clear=True) generally don't).

[perezju, 2015-02-20 15:20:08]

On 2015/02/20 15:10:40, petrcermak wrote:
> On 2015/02/20 15:07:49, jbudorick wrote:
> > While this works, I think the actual fix should have Logcat(dump=True)
return
> an
> > iterable. It not doing so is a bug (introduced by my CL you referenced in
the
> > bug).
> 
> I'm absolutely fine with changing it, but what was then the point of
> https://codereview.chromium.org/919763003 (because it only added this option
of
> not returning an iterator)?

Yes dump=True should return a list of lines, i.e., calling splitlines on the
result,
or just return the iterator itself. Not sure if there should be a strong
preference
for either of these two approaches.

[petrcermak, 2015-02-20 15:25:07]

On 2015/02/20 15:20:08, perezju wrote:
> On 2015/02/20 15:10:40, petrcermak wrote:
> > On 2015/02/20 15:07:49, jbudorick wrote:
> > > While this works, I think the actual fix should have Logcat(dump=True)
> return
> > an
> > > iterable. It not doing so is a bug (introduced by my CL you referenced in
> the
> > > bug).
> > 
> > I'm absolutely fine with changing it, but what was then the point of
> > https://codereview.chromium.org/919763003 (because it only added this option
> of
> > not returning an iterator)?
> 
> Yes dump=True should return a list of lines, i.e., calling splitlines on the
> result,
> or just return the iterator itself. Not sure if there should be a strong
> preference
> for either of these two approaches.

I guess it would make more sense to return a list because it feels more natural
if you pass dump=True and especially clear=True.

[perezju, 2015-02-20 15:30:17]

sgtm

[petrcermak, 2015-02-20 15:42:40]

PTAL.

[perezju, 2015-02-20 15:53:34]

lgtm, thanks for the fix!

[petrcermak, 2015-02-20 15:55:05]

The CQ bit was checked by petrcermak@chromium.org

[commit-bot: I haz the power, 2015-02-20 15:56:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/948433002/20001

[commit-bot: I haz the power, 2015-02-20 16:54:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-20 16:54:48]

Patchset 2 (id:??) landed as
https://crrev.com/1b2552acec5513c6974460d5e7ced2f80e5f94e8
Cr-Commit-Position: refs/heads/master@{#317338}