Add a first test for tools/find_runtime_symbols.

Add a first test for tools/find_runtime_symbols.


BUG=123749
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=169023

[Dai Mikurube (NOT FULLTIME), 2012-11-20 07:39:41]

Hi,

It's a test for tools/find_runtime_symbols.  Could you take a look when you have
time?

(maybe Thanksgiving this week...)

[M-A Ruel, 2012-11-20 14:30:00]

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/fi...
File tools/find_runtime_symbols/find_runtime_symbols.py (right):

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/fi...
tools/find_runtime_symbols/find_runtime_symbols.py:23: def output(self, address,
symbol): # pylint: disable=W0613
two spaces

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/pr...
File tools/find_runtime_symbols/proc_maps.py (right):

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/pr...
tools/find_runtime_symbols/proc_maps.py:61: table._append(ProcMapsEntry( #
pylint: disable=W0212
2 spaces

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/te...
File tools/find_runtime_symbols/tests/proc_maps_tests.py (right):

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/te...
tools/find_runtime_symbols/tests/proc_maps_tests.py:18: class
ProcMapsTest(unittest.TestCase):
It is still preferable to restrict to 80 cols. Split it in substrings. I won't
block the CL on this though.

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/te...
tools/find_runtime_symbols/tests/proc_maps_tests.py:34:
self.assertEqual(entries[0].begin, 0x0)
If you had something like entries[0].as_dict(), it would become a simple dict
comparison.

You could also make an utility function to reduce copy-paste, e.g.

def _verify_entry(self, entry, name, begin, end, ...):
  self.assertEqual(entries[0].name, name)
  self.assertEqual(entries[0].begin, begin)
  self.assertEqual(entries[0].end, end)
  ...


expected = [
  (0, '', 0x1000, ...),
  ('', 0x80b000, 0x1000, ...),
  ...
]

for index, (name, begin, end, ...) in enumerate(expected):
  self._verify(entry[index], name, begin, end, ...)

There are other way to write it if  you get too many items to compare but
reducing copy-pasting in testing code is important.

[Dai Mikurube (NOT FULLTIME), 2012-11-21 05:51:45]

Thanks, maruel.

Fixed them.  Could you take another look?

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/fi...
File tools/find_runtime_symbols/find_runtime_symbols.py (right):

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/fi...
tools/find_runtime_symbols/find_runtime_symbols.py:23: def output(self, address,
symbol): # pylint: disable=W0613
On 2012/11/20 14:30:00, Marc-Antoine Ruel wrote:
> two spaces

Done.

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/pr...
File tools/find_runtime_symbols/proc_maps.py (right):

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/pr...
tools/find_runtime_symbols/proc_maps.py:61: table._append(ProcMapsEntry( #
pylint: disable=W0212
On 2012/11/20 14:30:00, Marc-Antoine Ruel wrote:
> 2 spaces

Done.

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/te...
File tools/find_runtime_symbols/tests/proc_maps_tests.py (right):

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/te...
tools/find_runtime_symbols/tests/proc_maps_tests.py:18: class
ProcMapsTest(unittest.TestCase):
On 2012/11/20 14:30:00, Marc-Antoine Ruel wrote:
> It is still preferable to restrict to 80 cols. Split it in substrings. I won't
> block the CL on this though.

Done.

https://codereview.chromium.org/11299095/diff/1/tools/find_runtime_symbols/te...
tools/find_runtime_symbols/tests/proc_maps_tests.py:34:
self.assertEqual(entries[0].begin, 0x0)
On 2012/11/20 14:30:00, Marc-Antoine Ruel wrote:
> If you had something like entries[0].as_dict(), it would become a simple dict
> comparison.
> 
> You could also make an utility function to reduce copy-paste, e.g.
> 
> def _verify_entry(self, entry, name, begin, end, ...):
>   self.assertEqual(entries[0].name, name)
>   self.assertEqual(entries[0].begin, begin)
>   self.assertEqual(entries[0].end, end)
>   ...
> 
> 
> expected = [
>   (0, '', 0x1000, ...),
>   ('', 0x80b000, 0x1000, ...),
>   ...
> ]
> 
> for index, (name, begin, end, ...) in enumerate(expected):
>   self._verify(entry[index], name, begin, end, ...)
> 
> There are other way to write it if  you get too many items to compare but
> reducing copy-pasting in testing code is important.

Thanks for the good suggestion.  Done.

Also added new tests based on them.

[M-A Ruel, 2012-11-21 09:09:01]

lgtm

https://codereview.chromium.org/11299095/diff/6001/tools/find_runtime_symbols...
File tools/find_runtime_symbols/proc_maps.py (right):

https://codereview.chromium.org/11299095/diff/6001/tools/find_runtime_symbols...
tools/find_runtime_symbols/proc_maps.py:44: }
align at return, e.g. -4.

[Dai Mikurube (NOT FULLTIME), 2012-11-21 09:14:41]

Fixed it.  Thank you!

https://codereview.chromium.org/11299095/diff/6001/tools/find_runtime_symbols...
File tools/find_runtime_symbols/proc_maps.py (right):

https://codereview.chromium.org/11299095/diff/6001/tools/find_runtime_symbols...
tools/find_runtime_symbols/proc_maps.py:44: }
On 2012/11/21 09:09:01, Marc-Antoine Ruel wrote:
> align at return, e.g. -4.

Done.

[commit-bot: I haz the power, 2012-11-21 09:14:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmikurube@chromium.org/11299095/10

[commit-bot: I haz the power, 2012-11-21 09:51:10]

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-11-21 10:28:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmikurube@chromium.org/11299095/10

[commit-bot: I haz the power, 2012-11-21 12:33:53]

Change committed as 169023