Updating Layout test analyzer to add control to show issue detail or not.

Updating Layout test analyzer. 

It includes: 
* intoducing -z commandline option to show/not-show issue details
* fix the issues discovered by gpylint 
* adding bug link and flakiness dashboard link to test expection file display

BUG=109008, 107773
TEST= unit test passes and run script locally.

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

[imasaki1, 2012-02-27 23:48:36]

Thanks in advance.

[dennis_jeffrey, 2012-02-29 17:50:32]

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttest_analyzer.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer.py:416: file_object.write((
nit: I think the second ( in this line might be unnecessary, and the same with
the first ) in line 422 down below.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttest_analyzer_helpers.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:82: type_str: a string
indicating the test group to which |diff_map_element|
indent this line by 1 more space

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:121: whole_str =
''.join(whole_str.rsplit(',', 1))
Another way to do this without splitting and joining might be to use rfind to
find the index of the right-most comma.  Then you can concatenate the string
right before and right after that.  But I think it's also ok to leave it like
this.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:193: issue_detail_mode:
Please see |options|.
Since "options" is not defined in this file, it might be better to just describe
what this parameter is here, rather than referring to "options".

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:345:
email_only_change_mode: please refer to |options|.
Similar comment as line 193 above.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:414: # frakiness
dashboard.
frakiness --> flakiness

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:425: line =
line.replace(test_name, "<a href='%s'>%s</a>" % (
nit: I recommend swapping the single quotes and double quotes in this line. 
That way, the outer string in python will be defined with single quotes, and the
inner HTML string will have the double quotes.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:429: new_line =
'<li>%s</li>\n' % line.replace(bug.bug_txt, bug.__str__())
could we replace bug.__str__() with str(bug)?

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttests.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttests.py:17: import pysvn
nit: add a blank line above this, to separate the standard library imports above
from this third-party import.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttests.py:205: a list of parent directory for
given test names.
directory --> directories

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttests.py:205: a list of parent directory for
given test names.
for given --> for the given

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/trend_graph.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/trend_graph.py:18: class TrendGraph:
curious: why did you remove the "(object)" from here?

[imasaki1, 2012-03-02 19:12:07]

Thank you.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttest_analyzer.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer.py:416: file_object.write((
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> nit: I think the second ( in this line might be unnecessary, and the same with
> the first ) in line 422 down below.

I got gpylint error like the one below when I remove the brackets.
"W0106:416:UpdateDashboard: Expression "(file_object.write('<tr><td><a
href="%s">%s</a></td><td><a href="%s">dashboard</a></td><td>%s</td></tr>')) %
(((layouttest_root_path) + (testname), testname,
('http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=%s')
% (testname), data_map[tg][0][testname]))" is assigned to nothing"

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttest_analyzer_helpers.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:82: type_str: a string
indicating the test group to which |diff_map_element|
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> indent this line by 1 more space

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:121: whole_str =
''.join(whole_str.rsplit(',', 1))
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> Another way to do this without splitting and joining might be to use rfind to
> find the index of the right-most comma.  Then you can concatenate the string
> right before and right after that.  But I think it's also ok to leave it like
> this.

I prefer to leave as it is. but, thanks for the comments.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:193: issue_detail_mode:
Please see |options|.
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> Since "options" is not defined in this file, it might be better to just
describe
> what this parameter is here, rather than referring to "options".

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:345:
email_only_change_mode: please refer to |options|.
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> Similar comment as line 193 above.

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:414: # frakiness
dashboard.
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> frakiness --> flakiness

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:425: line =
line.replace(test_name, "<a href='%s'>%s</a>" % (
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> nit: I recommend swapping the single quotes and double quotes in this line. 
> That way, the outer string in python will be defined with single quotes, and
the
> inner HTML string will have the double quotes.

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer_helpers.py:429: new_line =
'<li>%s</li>\n' % line.replace(bug.bug_txt, bug.__str__())
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> could we replace bug.__str__() with str(bug)?

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttests.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttests.py:17: import pysvn
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> nit: add a blank line above this, to separate the standard library imports
above
> from this third-party import.

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttests.py:205: a list of parent directory for
given test names.
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> directory --> directories

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttests.py:205: a list of parent directory for
given test names.
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> for given --> for the given

Done.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/trend_graph.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/trend_graph.py:18: class TrendGraph:
On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> curious: why did you remove the "(object)" from here?

Mistake. I bring it back

[dennis_jeffrey, 2012-03-02 20:55:15]

LGTM

Just two minor comments to consider before you submit.  Thanks!

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttest_analyzer.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer.py:416: file_object.write((
On 2012/03/02 19:12:07, imasaki1 wrote:
> On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> > nit: I think the second ( in this line might be unnecessary, and the same
with
> > the first ) in line 422 down below.
> 
> I got gpylint error like the one below when I remove the brackets.
> "W0106:416:UpdateDashboard: Expression "(file_object.write('<tr><td><a
> href="%s">%s</a></td><td><a href="%s">dashboard</a></td><td>%s</td></tr>')) %
> (((layouttest_root_path) + (testname), testname,
>
('http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=%s')
> % (testname), data_map[tg][0][testname]))" is assigned to nothing"

Ok, I must have read the line incorrectly the first time.

https://chromiumcodereview.appspot.com/9476021/diff/13003/media/tools/layout_...
File media/tools/layout_tests/layouttest_analyzer_helpers.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/13003/media/tools/layout_...
media/tools/layout_tests/layouttest_analyzer_helpers.py:347: true. Otherwise,
always send email after each run.
nit: 'true' --> 'True'

https://chromiumcodereview.appspot.com/9476021/diff/13003/media/tools/layout_...
media/tools/layout_tests/layouttest_analyzer_helpers.py:410: rev_str += "<ul><a
href='%s'>%s->%s</a>\n" % (link, old_rev, new_rev)
I think this line was fine before you swapped the quotes here: we'd like the
python string to be defined with single quotes, and then the html string inside
of it can use double quotes:

rev_str += '<ul><a href="%s">%s->%s</a>\n' % (link, old_rev, new_rev)

[imasaki1, 2012-03-02 21:50:50]

Thanks. I am going to commit.

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
File media/tools/layout_tests/layouttest_analyzer.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/1/media/tools/layout_test...
media/tools/layout_tests/layouttest_analyzer.py:416: file_object.write((
On 2012/03/02 20:55:15, dennis_jeffrey wrote:
> On 2012/03/02 19:12:07, imasaki1 wrote:
> > On 2012/02/29 17:50:32, dennis_jeffrey wrote:
> > > nit: I think the second ( in this line might be unnecessary, and the same
> with
> > > the first ) in line 422 down below.
> > 
> > I got gpylint error like the one below when I remove the brackets.
> > "W0106:416:UpdateDashboard: Expression "(file_object.write('<tr><td><a
> > href="%s">%s</a></td><td><a href="%s">dashboard</a></td><td>%s</td></tr>'))
%
> > (((layouttest_root_path) + (testname), testname,
> >
>
('http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=%s')
> > % (testname), data_map[tg][0][testname]))" is assigned to nothing"
> 
> Ok, I must have read the line incorrectly the first time.

Done.

https://chromiumcodereview.appspot.com/9476021/diff/13003/media/tools/layout_...
File media/tools/layout_tests/layouttest_analyzer_helpers.py (right):

https://chromiumcodereview.appspot.com/9476021/diff/13003/media/tools/layout_...
media/tools/layout_tests/layouttest_analyzer_helpers.py:347: true. Otherwise,
always send email after each run.
On 2012/03/02 20:55:15, dennis_jeffrey wrote:
> nit: 'true' --> 'True'

Done.

https://chromiumcodereview.appspot.com/9476021/diff/13003/media/tools/layout_...
media/tools/layout_tests/layouttest_analyzer_helpers.py:410: rev_str += "<ul><a
href='%s'>%s->%s</a>\n" % (link, old_rev, new_rev)
On 2012/03/02 20:55:15, dennis_jeffrey wrote:
> I think this line was fine before you swapped the quotes here: we'd like the
> python string to be defined with single quotes, and then the html string
inside
> of it can use double quotes:
> 
> rev_str += '<ul><a href="%s">%s->%s</a>\n' % (link, old_rev, new_rev)

Done.