Update build/android/symbolize.py to match base/debug/stack_trace_android.cc format.

Update build/android/symbolize.py to match base/debug/stack_trace_android.cc format.

BUG=259554
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211449

[scherkus (not reviewing), 2013-07-11 22:42:24]

bulach: please review

(pythonic tips 'n advice welcome!)

[bulach, 2013-07-12 09:56:23]

lgtm, thanks! just some suggestions below:

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize.py
File build/android/symbolize.py (right):

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize.py#ne...
build/android/symbolize.py:28: '(?P<lib>[^+]+)\+0x(?P<addr>[0-9a-f]{8,8})')
I don't quite know what the use here would be, but should this be more flexible?
for instance, add (\s*)? at the beginning so it skips whitespaces (I'm thinking
of user doing just adb logcat rather than chromium:V for instance?) also,
a-fA-F, and \s+ rather than single whitespaces? wdyt?

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize_test.py
File build/android/symbolize_test.py (right):

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize_test....
build/android/symbolize_test.py:87: self.assertEqual(expected,
RunSymbolizer(expected))
this whole lot maybe simpler as data-driven, like:
EXPECTED = [
  # Leading '#' is required.
  expected = '00 0x00000000 ' + LIB_A_PATH + '+0x00000254\n',
  # Whitespace should be exactly one space.
  expected = '#00  0x00000000 ' + LIB_A_PATH + '+0x00000254\n',
  ...
]
for expected in EXPECTED:
  self.assertEquals(expected, RunSymbolizer(expected))

[scherkus (not reviewing), 2013-07-12 17:47:45]

thanks for the review!

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize.py
File build/android/symbolize.py (right):

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize.py#ne...
build/android/symbolize.py:28: '(?P<lib>[^+]+)\+0x(?P<addr>[0-9a-f]{8,8})')
On 2013/07/12 09:56:23, bulach wrote:
> I don't quite know what the use here would be, but should this be more
flexible?
> for instance, add (\s*)? at the beginning so it skips whitespaces (I'm
thinking
> of user doing just adb logcat rather than chromium:V for instance?) also,
> a-fA-F, and \s+ rather than single whitespaces? wdyt?

Now that this script lives in the Chromium repository, we can guarantee that the
regexp will exactly match the stack trace format produced by
base/debug/stack_trace_android.cc (e.g., if we ever tweak the format we can
update this script at the same time)

It's currently a non-goal to have symbolize.py work for stack traces produced by
other systems.

The leading/trailing text is captured + reprinted (check out the "pre" and
"post" variables on lines 67-68)

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize_test.py
File build/android/symbolize_test.py (right):

https://codereview.chromium.org/19010004/diff/1/build/android/symbolize_test....
build/android/symbolize_test.py:87: self.assertEqual(expected,
RunSymbolizer(expected))
On 2013/07/12 09:56:23, bulach wrote:
> this whole lot maybe simpler as data-driven, like:
> EXPECTED = [
>   # Leading '#' is required.
>   expected = '00 0x00000000 ' + LIB_A_PATH + '+0x00000254\n',
>   # Whitespace should be exactly one space.
>   expected = '#00  0x00000000 ' + LIB_A_PATH + '+0x00000254\n',
>   ...
> ]
> for expected in EXPECTED:
>   self.assertEquals(expected, RunSymbolizer(expected))

my only beef with that is if it fails it's not explicitly obvious which case
failed

we could print out the test case or the index number ... but with a lot of the
cases involving similar text I'd rather be more verbose in exchange for exact
line numbers when a test case fails

[commit-bot: I haz the power, 2013-07-12 18:01:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/19010004/8001

[commit-bot: I haz the power, 2013-07-12 20:17:56]

Change committed as 211449