Rework run-mojo-python-tests to use layout-test-compatible json output.

Rework run-mojo-python-tests to use layout-test-compatible json output.

This patch adds the ability to record the results of a test run into
a JSON file that can be uploaded to the flakiness dashboard.

R=viettrungluu@chromium.org, phajdan.jr@chromium.org, ojan@chromium.org, szager@chromium.org
BUG=364709
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279836

[Paweł Hajdan Jr., 2014-06-25 01:01:03]

LGTM with comments.

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
File mojo/tools/run_mojo_python_tests.py (right):

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:6: import json
nit: Sort.

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:33: print >> sys.stderr, ('Error: malformed
metadata "%s"' % val)
Why not parser.error(...)?

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:65: fp.write("\n")
Is this needed?

[Dirk Pranke, 2014-06-25 01:05:11]

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
File mojo/tools/run_mojo_python_tests.py (right):

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:6: import json
On 2014/06/25 01:01:03, Paweł Hajdan Jr. wrote:
> nit: Sort.

whoops. will fix, thanks!

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:33: print >> sys.stderr, ('Error: malformed
metadata "%s"' % val)
On 2014/06/25 01:01:03, Paweł Hajdan Jr. wrote:
> Why not parser.error(...)?

parser.error() calls sys.exit() directly, which makes it hard to test. Hence,
I'm not a fan of it.

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:65: fp.write("\n")
On 2014/06/25 01:01:03, Paweł Hajdan Jr. wrote:
> Is this needed?

No, but it makes the file slightly easier to read if you're looking at it by
hand, just as the indent=2 does.

[viettrungluu, 2014-06-25 01:08:24]

LGTM w/nits (I think everything I had to say about the previous version of the
change was rectified by the v4 -> v3 change).

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
File mojo/tools/run_mojo_python_tests.py (right):

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:74: "Convert the unittest results to the
Chromium JSON test result format.
" -> """?

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:92: 'Failure': len(failed_test_names),
nit: indentation

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:100: 'expected': 'PASS',
"

[ojan, 2014-06-25 01:11:24]

I'm Ok with this, but it makes me sad to have more places we generate JSON that
don't use shared code. I understand that the shared code wouldn't do much. It
would just take as inputs the stuff that you are putting in the JSON already.
But, it could abstract out all the trie stuff, for example. You could pass it a
flat list of tests that it then turns into the trie data structure. Also, it
would be a centralized location where we create the FAIL/PASS/Failure/Pass
strings so that we can modify them without need to touch many places.

It's technical debt we'll have to pay off later. It should at least have a TODO.

[Dirk Pranke, 2014-06-25 01:23:45]

On 2014/06/25 01:11:24, OOO-only-code-yellow-reviews wrote:
> I'm Ok with this, but it makes me sad to have more places we generate JSON
that
> don't use shared code. I understand that the shared code wouldn't do much. It
> would just take as inputs the stuff that you are putting in the JSON already.
> But, it could abstract out all the trie stuff, for example. You could pass it
a
> flat list of tests that it then turns into the trie data structure. Also, it
> would be a centralized location where we create the FAIL/PASS/Failure/Pass
> strings so that we can modify them without need to touch many places.
> 
> It's technical debt we'll have to pay off later. It should at least have a
TODO.

I can add a TODO. I would be happy to use shared code, except that that doesn't
exist today :). Feel free to tell me where to put such shared code, and I will
try to make it happen.

[Dirk Pranke, 2014-06-25 18:43:46]

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
File mojo/tools/run_mojo_python_tests.py (right):

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:74: "Convert the unittest results to the
Chromium JSON test result format.
On 2014/06/25 01:08:23, viettrungluu wrote:
> " -> """?

Whoops. Done.

https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
mojo/tools/run_mojo_python_tests.py:92: 'Failure': len(failed_test_names),
On 2014/06/25 01:08:23, viettrungluu wrote:
> nit: indentation

python style uses 4-space hanging indents as far as I know; am I missing
something?

[Dirk Pranke, 2014-06-25 19:36:07]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-06-25 19:36:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dpranke@chromium.org/344233008/100001

[viettrungluu, 2014-06-25 20:27:01]

On 2014/06/25 18:43:46, Dirk Pranke wrote:
>
https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
> File mojo/tools/run_mojo_python_tests.py (right):
> 
>
https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
> mojo/tools/run_mojo_python_tests.py:74: "Convert the unittest results to the
> Chromium JSON test result format.
> On 2014/06/25 01:08:23, viettrungluu wrote:
> > " -> """?
> 
> Whoops. Done.
> 
>
https://codereview.chromium.org/344233008/diff/60001/mojo/tools/run_mojo_pyth...
> mojo/tools/run_mojo_python_tests.py:92: 'Failure': len(failed_test_names),
> On 2014/06/25 01:08:23, viettrungluu wrote:
> > nit: indentation
> 
> python style uses 4-space hanging indents as far as I know; am I missing
> something?

Hrm, I don't care very much but....

Assuming Chromium style is PEP-8, except with 2-space indents:
- PEP-8 is actually quite flexible about the indentation level for hanging
indents (I know this now having read it)
- I tend to agree that hanging indents should be double-indents (4 spaces for
Chromium)
- I've never thought of list/dictionary declarations as really being
hanging/continuation indents (since the closing delimiter is at the same level
as the indentation of the line with the opening delimiter); PEP-8 is vague and
flexible on this

But the (internal) Google style guide agrees with you, so I guess you're right.

[commit-bot: I haz the power, 2014-06-25 22:18:22]

Change committed as 279836