Generalize rebase-errors.py so it works for other files.

Generalize rebase-errors.py so it works for other files.

BUG=634443
Committed: https://crrev.com/c25f07964b8d1692816343ac50ff1668c255ac4d
Cr-Commit-Position: refs/heads/master@{#419295}

[eroman, 2016-09-14 23:37:24]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 23:37:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-14 23:41:01]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 23:41:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-14 23:42:42]

eroman@chromium.org changed reviewers:
+ mattm@chromium.org

[eroman, 2016-09-14 23:42:43]

I made it a bit less hacky so I can re-use it on tests where there isn't a
direct mapping between the test name and the PEM file.

(Like the ParseCertificate tests)

[mattm, 2016-09-15 00:42:05]

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
File net/data/verify_certificate_chain_unittest/rebase-errors.py (right):

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:17: The script will
search the unit-test stdout and looking for a failed value of:
awkward wording

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:32:
kFailedTestPattern = r"""
do the re.compile for these patterns here rather than where they are used
(mainly so you can see which flags are used with each pattern next to the
pattern itself, but I think it's also more idiomatic / efficient)

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:34: Actual: "(.*?)"
probably should be greedy (safe since this pattern isn't using DOTALL)

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:36: Test file: (.*?)
here too

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:41: -----END .*-----
non-greedy?

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:68: if not
base_dir.endswith('/verify_certificate_chain_unittest'):
os.path.split(base_dir)[1]

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:68: if not
base_dir.endswith('/verify_certificate_chain_unittest'):
reasoning for hard coding this? (Instead of say, building the .py path and
testing if it exists)

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:106: prog =
re.compile(r'^errors = """(.*)"""', re.MULTILINE | re.DOTALL)
guess this should be non-greedy too

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:123: print "Script
is not in expected location: " % (kScriptDir)
format string doesn't contain %s

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:124: exit(1)
sys.exit

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:126: src_dir =
script_dir[0:-len(kScriptDir)]
instead of hardcoding all that, how about something like:

  src_dir = os.path.dirname(os.path.realpath(__file__))
  while True:
    parent_dir, dirname = os.path.split(src_dir)
    if dirname == "src":
      break
    if not parent_dir or parent_dir == src_dir:
      print "couldn't find src dir"
      sys.exit(1)
    src_dir = parent_dir

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:127: return src_dir
+ rel_path
os.path.join

[eroman, 2016-09-16 01:14:12]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[eroman, 2016-09-16 01:14:30]

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
File net/data/verify_certificate_chain_unittest/rebase-errors.py (right):

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:17: The script will
search the unit-test stdout and looking for a failed value of:
On 2016/09/15 00:42:05, mattm wrote:
> awkward wording

Done.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:32:
kFailedTestPattern = r"""
On 2016/09/15 00:42:05, mattm wrote:
> do the re.compile for these patterns here rather than where they are used
> (mainly so you can see which flags are used with each pattern next to the
> pattern itself, but I think it's also more idiomatic / efficient)

Done. Agreed that is better! I also added an explanation for the patterns.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:34: Actual: "(.*?)"
On 2016/09/15 00:42:05, mattm wrote:
> probably should be greedy (safe since this pattern isn't using DOTALL)

Done.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:36: Test file: (.*?)
On 2016/09/15 00:42:05, mattm wrote:
> here too

Done.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:41: -----END .*-----
On 2016/09/15 00:42:05, mattm wrote:
> non-greedy?

Done. (Thanks for the help over IM on this).

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:68: if not
base_dir.endswith('/verify_certificate_chain_unittest'):
On 2016/09/15 00:42:05, mattm wrote:
> reasoning for hard coding this? (Instead of say, building the .py path and
> testing if it exists)

Done.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:106: prog =
re.compile(r'^errors = """(.*)"""', re.MULTILINE | re.DOTALL)
On 2016/09/15 00:42:05, mattm wrote:
> guess this should be non-greedy too

Done.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:123: print "Script
is not in expected location: " % (kScriptDir)
On 2016/09/15 00:42:05, mattm wrote:
> format string doesn't contain %s

Done

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:124: exit(1)
On 2016/09/15 00:42:05, mattm wrote:
> sys.exit

... clearly I didn't test this failure case.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:126: src_dir =
script_dir[0:-len(kScriptDir)]
On 2016/09/15 00:42:05, mattm wrote:
> instead of hardcoding all that, how about something like:
> 
>   src_dir = os.path.dirname(os.path.realpath(__file__))
>   while True:
>     parent_dir, dirname = os.path.split(src_dir)
>     if dirname == "src":
>       break
>     if not parent_dir or parent_dir == src_dir:
>       print "couldn't find src dir"
>       sys.exit(1)
>     src_dir = parent_dir

Done.

https://codereview.chromium.org/2346663002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/rebase-errors.py:127: return src_dir
+ rel_path
On 2016/09/15 00:42:05, mattm wrote:
> os.path.join

Done.

[commit-bot: I haz the power, 2016-09-16 01:14:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-16 01:18:45]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 01:19:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 01:25:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 01:25:38]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[eroman, 2016-09-16 18:29:37]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 18:30:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 19:04:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 19:04:20]

Dry run: This issue passed the CQ dry run.

[mattm, 2016-09-16 21:17:34]

lgtm

[eroman, 2016-09-16 22:03:45]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2016-09-16 22:04:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 22:12:32]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-16 22:15:09]

Description was changed from

==========
Generalize rebase-errors.py so it works for other files.

BUG=634443
==========

to

==========
Generalize rebase-errors.py so it works for other files.

BUG=634443

Committed: https://crrev.com/c25f07964b8d1692816343ac50ff1668c255ac4d
Cr-Commit-Position: refs/heads/master@{#419295}
==========

[commit-bot: I haz the power, 2016-09-16 22:15:10]

Patchset 6 (id:??) landed as
https://crrev.com/c25f07964b8d1692816343ac50ff1668c255ac4d
Cr-Commit-Position: refs/heads/master@{#419295}