Support CSV output format for analyzer results.

Support CSV output format for analyzer results in layout test analyzer.

CSV output format is used for integration with Google spreadsheet.

Plus several minor issue fixes and adding minor features.

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

[imasaki1, 2011-11-10 23:27:45]

Thank you.

[scherkus (not reviewing), 2011-11-10 23:33:45]

out of curiosity (the CL description doesn't mention) but what are we using CSV
for?

[scherkus (not reviewing), 2011-11-10 23:34:10]

On 2011/11/10 23:33:45, scherkus wrote:
> out of curiosity (the CL description doesn't mention) but what are we using
CSV
> for?

ah sorry read too soon -- didn't see the Google spreadsheet comment!

nevermind :)

[dennis_jeffrey, 2011-11-11 00:01:36]

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
File media/tools/layout_tests/layouttest_analyzer.py (right):

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer.py:291: prev_time = None
I think we could remove this line and just put 'None' where needed in line 292
below.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer.py:464: # Create CSV texts and save
them for bug spreadsheet.
I recommend adding a blank line right above this, and a blank line right below
line 474 below, to separate the chunk of code related to storing the CSV text
from the rest of the code in this function.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
File media/tools/layout_tests/layouttest_analyzer_helpers.py (right):

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:137: """Convert
result_map into stats and issues text in CSV format.
'result_map' --> |self.result_map|

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:139: Both are used for
inputs for Google spreadsheet.
nit: 'for inputs' --> 'as inputs'

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:142: current_time: a
string shows current time (e.g, 2011-11-08-16)
current_time: a string depicting a time in year-month-day-hour
    format (e.g., 2011-11-08-16).

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:151: issues_txt: issues
listed in CSV format that shows:
Could this include information for multiple issues?  If so, maybe we could give
an example with 2 issues at line 155 down below.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:155: "71543,TIMEOUT
PASS,media/media-element-play-after-eos.html,"
This example seems to be missing the "BUGWK"/"BUGCR" at the front, as described
in line 152 above.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:158:
str(len(self.result_map['skip'].keys())),
nit: indent this by 1 more space.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:159:
str(len(self.result_map['nonskip'].keys()))])
nit: indent this by 2 more spaces

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:161: for (bug_txt,
test_info_list) in (
I believe the parens are unnecessary in '(bug_txt, test_info_list)'

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:162:
self.GetListOfBugsForNonSkippedTests().iteritems()):
nit: indent this by 2 more spaces

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:164: bug_prefix = ''
nit: 'bug_prefix' --> 'bug_suffix', since it's the 'CR' or the 'WK' part after
the 'BUG'

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:171: (test_name,
te_info) = test_info
no need for parentheses here

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:174: return (stats,
issues_txt)
no need for parentheses here

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:340: change_str =
'Status Change'
add a space right after the 'e' at the end of this string (see the related
comment at line 341 down below; these comments make sure there are no extra
spaces in the subject line in the event that change_str is the empty string).

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:341: subject = 'Layout
Test Analyzer Result %s (%s): %s' % (change_str,
remove the space right after the first '%s'

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
File media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py (right):

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py:190:
current_time = '11-10-10-2011'
Remove this line and just inline the value where needed at line 191 below.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py:191: (data,
issues_txt) = analyzerResultMapBase.ConvertToCSVText(current_time)
no need for the parens in '(data, issues_txt)'

[imasaki1, 2011-11-11 00:25:52]

Thanks!

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
File media/tools/layout_tests/layouttest_analyzer.py (right):

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer.py:291: prev_time = None
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> I think we could remove this line and just put 'None' where needed in line 292
> below.

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer.py:464: # Create CSV texts and save
them for bug spreadsheet.
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> I recommend adding a blank line right above this, and a blank line right below
> line 474 below, to separate the chunk of code related to storing the CSV text
> from the rest of the code in this function.

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
File media/tools/layout_tests/layouttest_analyzer_helpers.py (right):

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:137: """Convert
result_map into stats and issues text in CSV format.
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> 'result_map' --> |self.result_map|

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:139: Both are used for
inputs for Google spreadsheet.
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> nit: 'for inputs' --> 'as inputs'

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:142: current_time: a
string shows current time (e.g, 2011-11-08-16)
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> current_time: a string depicting a time in year-month-day-hour
>     format (e.g., 2011-11-08-16).

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:151: issues_txt: issues
listed in CSV format that shows:
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> Could this include information for multiple issues?  If so, maybe we could
give
> an example with 2 issues at line 155 down below.

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:155: "71543,TIMEOUT
PASS,media/media-element-play-after-eos.html,"
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> This example seems to be missing the "BUGWK"/"BUGCR" at the front, as
described
> in line 152 above.

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:158:
str(len(self.result_map['skip'].keys())),
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> nit: indent this by 1 more space.

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:159:
str(len(self.result_map['nonskip'].keys()))])
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> nit: indent this by 2 more spaces

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:161: for (bug_txt,
test_info_list) in (
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> I believe the parens are unnecessary in '(bug_txt, test_info_list)'

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:162:
self.GetListOfBugsForNonSkippedTests().iteritems()):
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> nit: indent this by 2 more spaces

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:164: bug_prefix = ''
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> nit: 'bug_prefix' --> 'bug_suffix', since it's the 'CR' or the 'WK' part after
> the 'BUG'

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:171: (test_name,
te_info) = test_info
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> no need for parentheses here

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:174: return (stats,
issues_txt)
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> no need for parentheses here

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:340: change_str =
'Status Change'
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> add a space right after the 'e' at the end of this string (see the related
> comment at line 341 down below; these comments make sure there are no extra
> spaces in the subject line in the event that change_str is the empty string).

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers.py:341: subject = 'Layout
Test Analyzer Result %s (%s): %s' % (change_str,
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> remove the space right after the first '%s'

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
File media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py (right):

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py:190:
current_time = '11-10-10-2011'
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> Remove this line and just inline the value where needed at line 191 below.

Done.

http://codereview.chromium.org/8469017/diff/1/media/tools/layout_tests/layout...
media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py:191: (data,
issues_txt) = analyzerResultMapBase.ConvertToCSVText(current_time)
On 2011/11/11 00:01:36, dennis_jeffrey wrote:
> no need for the parens in '(data, issues_txt)'

Done.

[dennis_jeffrey, 2011-11-11 00:32:30]

LGTM

Just 1 more nit.  Thank you!

http://codereview.chromium.org/8469017/diff/5001/media/tools/layout_tests/lay...
File media/tools/layout_tests/layouttest_analyzer.py (right):

http://codereview.chromium.org/8469017/diff/5001/media/tools/layout_tests/lay...
media/tools/layout_tests/layouttest_analyzer.py:475: if not options.debug and
(result_change or not prev_analyzer_result_map):
I recommend adding a blank line right above this, to separate the CSV stuff
above from the rest of the code here and below.

[imasaki1, 2012-03-01 23:52:18]

https://chromiumcodereview.appspot.com/8469017/diff/5001/media/tools/layout_t...
File media/tools/layout_tests/layouttest_analyzer.py (right):

https://chromiumcodereview.appspot.com/8469017/diff/5001/media/tools/layout_t...
media/tools/layout_tests/layouttest_analyzer.py:475: if not options.debug and
(result_change or not prev_analyzer_result_map):
On 2011/11/11 00:32:30, dennis_jeffrey wrote:
> I recommend adding a blank line right above this, to separate the CSV stuff
> above from the rest of the code here and below.

Done.