blink/run-webkit-tests: Write layout test environment to json file.

blink/run-webkit-tests: Write layout test environment to json file.

To better understand the command line arguments and environment which causes a
layout test result to occur, this adds an env.json which includes;
 * The options used to run
 * The environment variables
 * Python's `platform` module information
 * Random seed used

BUG=601332

[mithro, 2016-06-21 05:58:06]

tansell@chromium.org changed reviewers:
+ chrishall@chromium.org, dpranke@chromium.org, qyearsley@chromium.org

[mithro, 2016-06-21 05:58:07]

Hi!

This patch adds logging the environment that a layout test ran under to a new
.json file. This information is useful for trying to reproduce a test run.

Tim 'mithro' Ansell

[Dirk Pranke, 2016-06-21 21:57:54]

"blink/run-webkit-py" isn't actually a thing. Can you update the subject and
description to refer to the right things?

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:107:
'env': dict(os.environ),
The args and environ are dumped to stderr as part of the logging run. Do we
really need to write them to a file as well? (though you should make sure the
random seed is logged).

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:114:
for func in dir(platform):
Is platform actually defined somewhere? I'm also actually leery of logging what
I'm guessing you're trying to log here. I don't know when you would actually
need this information?

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:95:
]))
Don't add stuff to the main code just for testing purposes.

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:556:
self.assertTrue(json_string.find('"DUMMY-ENV-KEY":"dummy-env-value"') != -1)
Why not just look at the real arguments that are being passed?

[mithro, 2016-06-22 07:44:20]

Description was changed from

==========
blink/run-webkit-py: Write layout test environment to json file.

To better understand the command line arguments and environment which causes a
layout test result to occur, this adds an env.json which includes;
 * The options used to run
 * The environment variables
 * Python's `platform` module information
 * Random seed used

BUG=601332
==========

to

==========
blink/run-webkit-tests: Write layout test environment to json file.

To better understand the command line arguments and environment which causes a
layout test result to occur, this adds an env.json which includes;
 * The options used to run
 * The environment variables
 * Python's `platform` module information
 * Random seed used

BUG=601332
==========

[mithro, 2016-06-22 08:42:24]

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:107:
'env': dict(os.environ),
On 2016/06/21 21:57:54, Dirk Pranke wrote:
> The args and environ are dumped to stderr as part of the logging run. Do we
> really need to write them to a file as well? (though you should make sure the
> random seed is logged).

Does the logging output get saved with the test results somewhere? 

The aim here is to provide as much information as possible (in a computer
readable version) to allow a person to reproduce the layout test results.

Ideally the env information would include content_shell build information in the
future too.

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:114:
for func in dir(platform):
On 2016/06/21 21:57:54, Dirk Pranke wrote:
> Is platform actually defined somewhere? I'm also actually leery of logging
what
> I'm guessing you're trying to log here. I don't know when you would actually
> need this information?

platform is a standard library module -
https://docs.python.org/2/library/platform.html

The information I'm interested here include processor + architecture, operating
system version, python information.

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:556:
self.assertTrue(json_string.find('"DUMMY-ENV-KEY":"dummy-env-value"') != -1)
On 2016/06/21 21:57:54, Dirk Pranke wrote:
> Why not just look at the real arguments that are being passed?

I wanted an argument which had no effect on the run in any way. There doesn't
appear to be an existing "no op" argument?

[Dirk Pranke, 2016-06-23 23:57:26]

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:107:
'env': dict(os.environ),
On 2016/06/22 08:42:24, mithro wrote:
> On 2016/06/21 21:57:54, Dirk Pranke wrote:
> > The args and environ are dumped to stderr as part of the logging run. Do we
> > really need to write them to a file as well? (though you should make sure
the
> > random seed is logged).
> 
> Does the logging output get saved with the test results somewhere? 

Well, the logging output is on the builder alongside the links to the test
results. I suppose
it depends on where you're getting the test results from, but for me for
triaging infra/harness
type issues it's almost always way more useful to have the log than the test
results.

> The aim here is to provide as much information as possible (in a computer
> readable version) to allow a person to reproduce the layout test results.
> 
> Ideally the env information would include content_shell build information in
the
> future too.

This information is also available in the build logs. I wouldn't try to
duplicate that
information in the results archives (though the chromium build hash is already
in the 
JSON test results).

If you were going to save more data, I would save it in the fulL_results.json,
rather than
creating another json file.

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:114:
for func in dir(platform):
On 2016/06/22 08:42:24, mithro wrote:
> On 2016/06/21 21:57:54, Dirk Pranke wrote:
> > Is platform actually defined somewhere? I'm also actually leery of logging
> what
> > I'm guessing you're trying to log here. I don't know when you would actually
> > need this information?
> 
> platform is a standard library module -
> https://docs.python.org/2/library/platform.html
> 
> The information I'm interested here include processor + architecture,
operating
> system version, python information.

I was actually referring to the symbol itself. For whatever reason, I didn't see
the "import platform"
in your patch before.

Most of this information is already available in the build logs. The stuff that
isn't isn't historically
very useful, and I'm not generally a fan of logging everything because it might
be useful.

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:542:
json_results_generator.write_json(self._filesystem, self._env(), env_json_path)
see comment above about just using the existing json file.

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:556:
self.assertTrue(json_string.find('"DUMMY-ENV-KEY":"dummy-env-value"') != -1)
On 2016/06/22 08:42:24, mithro wrote:
> On 2016/06/21 21:57:54, Dirk Pranke wrote:
> > Why not just look at the real arguments that are being passed?
> 
> I wanted an argument which had no effect on the run in any way. There doesn't
> appear to be an existing "no op" argument?

I'm not sure why it matters whether the argument has any effect or not?

[chrishall, 2016-08-08 04:32:57]

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:109:
'options': self._options.__dict__,
Minor nitpick:

above to convert environ we used `dict(os.environ)`,
why are we here using `self._options.__dict___` ?

could we instead use `dict(self._options)` for consistency ?

https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:114:
for func in dir(platform):
If our goal is to produce an easy-to-consume JSON file then it may be worthwhile
trying to make this step process the input a little more - rather than slurping
everything and requiring later steps to massage the data.

For example it would be nice to have a key 'os_name' and a value under that of
'windows'.


Platform is a little gross in that it seems to expose each operating system's
information in slightly different ways, I have code to deal with this - see
below.

This is particularly ugly when dealing with mac versioning (special case) - see
my 'mac_ver' code - linked below.

I implemented similar things for OS metric gathering which captures:
os name
os version
os arch
python arch

Which you can find in:

https://codereview.chromium.org/2106953006/diff/140001/infra/services/sysmon/...

[qyearsley, 2016-08-23 21:37:15]

On 2016/08/08 at 04:32:57, chrishall wrote:
>
https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
> File
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
(right):
> 
>
https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
>
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:109:
'options': self._options.__dict__,
> Minor nitpick:
> 
> above to convert environ we used `dict(os.environ)`,
> why are we here using `self._options.__dict___` ?
> 
> could we instead use `dict(self._options)` for consistency ?
> 
>
https://codereview.chromium.org/2088713002/diff/1/third_party/WebKit/Tools/Sc...
>
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:114:
for func in dir(platform):
> If our goal is to produce an easy-to-consume JSON file then it may be
worthwhile trying to make this step process the input a little more - rather
than slurping everything and requiring later steps to massage the data.
> 
> For example it would be nice to have a key 'os_name' and a value under that of
'windows'.
> 
> 
> Platform is a little gross in that it seems to expose each operating system's
information in slightly different ways, I have code to deal with this - see
below.
> 
> This is particularly ugly when dealing with mac versioning (special case) -
see my 'mac_ver' code - linked below.
> 
> I implemented similar things for OS metric gathering which captures:
> os name
> os version
> os arch
> python arch
> 
> Which you can find in:
> 
>
https://codereview.chromium.org/2106953006/diff/140001/infra/services/sysmon/...

Ping