Switch telemetry over to use typ to run the unit tests.

Switch telemetry over to use typ to run the unit tests.

Using typ allows us to run the tests in parallel and share
the logic for parsing the results and uploading them to the
flakiness dashboard with other python test steps.

R=dtu@chromium.org, tonyg@chromium.org, nduca@chromium.org
BUG=402172, 388256
Committed: https://crrev.com/3e7c2a6ee80e05fc6b8090bee850c40b8a2a3810
Cr-Commit-Position: refs/heads/master@{#302570}

[Dirk Pranke, 2014-10-21 23:22:51]

dpranke@chromium.org changed reviewers:
+ nduca@chromium.org, tonyg@chromium.org

[tonyg, 2014-10-21 23:31:22]

lg2m, but deferring full review to dtu

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/backends/chrome/inspector_network_unittest.py
(right):

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/inspector_network_unittest.py:72:
def testCacheableHTTPResponse(self):
This test was recently disabled for flakiness. So maybe there's no problem w/
typ at all and this test is just bad.

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/un...
File tools/telemetry/telemetry/unittest/run_tests_unittest.py (right):

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/un...
tools/telemetry/telemetry/unittest/run_tests_unittest.py:32: @unittest.skip
This test verifies that the decorators work which seems relevant. Any way we can
keep it?

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/we...
File tools/telemetry/telemetry/web_perf/timeline_based_measurement_unittest.py
(right):

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/we...
tools/telemetry/telemetry/web_perf/timeline_based_measurement_unittest.py:210:
@decorators.Isolated('mac')
If these tests and that other flaky one are the only reasons to add Isolated(),
I'd vote for leaving it out and just disabling these tests w/ a bug to fix them.

[Dirk Pranke, 2014-10-22 01:16:10]

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/backends/chrome/inspector_network_unittest.py
(right):

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/inspector_network_unittest.py:72:
def testCacheableHTTPResponse(self):
On 2014/10/21 23:31:22, tonyg wrote:
> This test was recently disabled for flakiness. So maybe there's no problem w/
> typ at all and this test is just bad.

Ok.

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/un...
File tools/telemetry/telemetry/unittest/run_tests_unittest.py (right):

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/un...
tools/telemetry/telemetry/unittest/run_tests_unittest.py:32: @unittest.skip
On 2014/10/21 23:31:22, tonyg wrote:
> This test verifies that the decorators work which seems relevant. Any way we
can
> keep it?

Oh, whoops, I meant to put these back in (or at least take a look at them and
update them to make sure they ran).

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/we...
File tools/telemetry/telemetry/web_perf/timeline_based_measurement_unittest.py
(right):

https://codereview.chromium.org/659293003/diff/1/tools/telemetry/telemetry/we...
tools/telemetry/telemetry/web_perf/timeline_based_measurement_unittest.py:210:
@decorators.Isolated('mac')
On 2014/10/21 23:31:22, tonyg wrote:
> If these tests and that other flaky one are the only reasons to add
Isolated(),
> I'd vote for leaving it out and just disabling these tests w/ a bug to fix
them.

That's fine by me. I will do so unless dtu objects.

[Dirk Pranke, 2014-10-24 01:14:59]

Patchset #2 (id:20001) has been deleted

[Dirk Pranke, 2014-10-24 01:15:26]

Okay, updated with review feedback; Dave, can you please take a look?

(Tony feel free to do so also).

[tonyg, 2014-10-27 17:45:11]

lgtm

https://codereview.chromium.org/659293003/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/decorators.py (right):

https://codereview.chromium.org/659293003/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/decorators.py:95: # TODO(dpranke): Remove if we don't
need this.
Yes, please :)

[achuithb, 2014-10-27 21:11:34]

achuith@chromium.org changed reviewers:
+ achuith@chromium.org

[achuithb, 2014-10-27 21:11:34]

Dirk: fyi, cros runs telemetry unit tests like so:
https://cs.corp.google.com/#chromeos_public/src/third_party/autotest/files/cl...

We'll need to re-do this wrapper as well, right?

[Dirk Pranke, 2014-10-27 21:52:38]

On 2014/10/27 21:11:34, achuithb wrote:
> Dirk: fyi, cros runs telemetry unit tests like so:
>
https://cs.corp.google.com/#chromeos_public/src/third_party/autotest/files/cl...
> 

I see. First I've heard of this :).

> We'll need to re-do this wrapper as well, right?

Yes. Given that this looks to be in third_party, I guess I can't fix that as
part of this patch, but if I land this patch as-is, I'm guessing that your
script will flat out break on the next roll. Is that (a) correct, and (b)
acceptable?

[achuithb, 2014-10-27 22:29:43]

On 2014/10/27 21:52:38, Dirk Pranke wrote:
> On 2014/10/27 21:11:34, achuithb wrote:
> > Dirk: fyi, cros runs telemetry unit tests like so:
> >
>
https://cs.corp.google.com/#chromeos_public/src/third_party/autotest/files/cl...
> > 
> 
> I see. First I've heard of this :).
> 
> > We'll need to re-do this wrapper as well, right?
> 
> Yes. Given that this looks to be in third_party, I guess I can't fix that as
> part of this patch, but if I land this patch as-is, I'm guessing that your
> script will flat out break on the next roll. Is that (a) correct, and (b)
> acceptable?

If there's a cros build break (which seems likely), it would be very painful for
the cros team. telemetry_UnitTests would need to be fixed while the chrome
pre-flight queue is red.

Is it possible to prepare a patch, which we could land as soon as the PFQ goes
red?

And no, you can't fix this in chromium, it's in the cros repository. Let me know
what help you need in preparing this patch.

[achuithb, 2014-10-27 22:31:37]

On 2014/10/27 22:29:43, achuithb wrote:
> On 2014/10/27 21:52:38, Dirk Pranke wrote:
> > On 2014/10/27 21:11:34, achuithb wrote:
> > > Dirk: fyi, cros runs telemetry unit tests like so:
> > >
> >
>
https://cs.corp.google.com/#chromeos_public/src/third_party/autotest/files/cl...
> > > 
> > 
> > I see. First I've heard of this :).
> > 
> > > We'll need to re-do this wrapper as well, right?
> > 
> > Yes. Given that this looks to be in third_party, I guess I can't fix that as
> > part of this patch, but if I land this patch as-is, I'm guessing that your
> > script will flat out break on the next roll. Is that (a) correct, and (b)
> > acceptable?
> 
> If there's a cros build break (which seems likely), it would be very painful
for
> the cros team. telemetry_UnitTests would need to be fixed while the chrome
> pre-flight queue is red.
> 
> Is it possible to prepare a patch, which we could land as soon as the PFQ goes
> red?
> 
> And no, you can't fix this in chromium, it's in the cros repository. Let me
know
> what help you need in preparing this patch.

We've had issues with this script in the past as well. It would be ideal if we
could move this logic to the chromium repository, and have the test use a simple
one-line invocation instead. This would aide future refactorings as well and
make these changes less painful for chromeos.

[Dirk Pranke, 2014-10-27 22:32:02]

On 2014/10/27 22:29:43, achuithb wrote:
> Is it possible to prepare a patch, which we could land as soon as the PFQ goes
> red? 

Yes, probably, though I've never actually submitted a patch to cros, so I'll
probably reach out to you for some help.

-- Dirk

[achuithb, 2014-10-27 23:02:51]

On 2014/10/27 22:32:02, Dirk Pranke wrote:
> On 2014/10/27 22:29:43, achuithb wrote:
> > Is it possible to prepare a patch, which we could land as soon as the PFQ
goes
> > red? 
> 
> Yes, probably, though I've never actually submitted a patch to cros, so I'll
> probably reach out to you for some help.
> 
> -- Dirk

I assume you're familiar with this:
http://www.chromium.org/chromium-os/developer-guide

[Dirk Pranke, 2014-10-28 01:56:15]

Achuith, can you take a look at the diffs in patchset #3 and make sure they look
approximately sane to you, assuming the other CL lands first?

[achuithb, 2014-10-28 06:53:51]

On 2014/10/28 01:56:15, Dirk Pranke wrote:
> Achuith, can you take a look at the diffs in patchset #3 and make sure they
look
> approximately sane to you, assuming the other CL lands first?

I'll try out your CL tomorrow and let you know how it goes.

[achuithb, 2014-10-29 19:42:02]

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
tools/telemetry/telemetry/unittest/run_tests.py:14:
util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'third_party', 'typ')
You need to add this to bootstrap_deps here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/bo...

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
tools/telemetry/telemetry/unittest/run_tests.py:16: import typ
It might perhaps make sense to use a wrapper for this import, similar to what we
do for crx_id, for instance:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

[Dirk Pranke, 2014-10-29 19:47:19]

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
tools/telemetry/telemetry/unittest/run_tests.py:14:
util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'third_party', 'typ')
On 2014/10/29 19:42:02, achuithb wrote:
> You need to add this to bootstrap_deps here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/bo...
> 

Hm. I don't really understand how (or who) is using this, and so whether someone
who isn't a telemetry dev would need to run the tests and wouldn't have a full
checkout. The 'standalone telemetry' users may not need this, for example.

Does ChromeOS not have a full Chrome checkout?

Dave, any thoughts on this?

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
tools/telemetry/telemetry/unittest/run_tests.py:16: import typ
On 2014/10/29 19:42:02, achuithb wrote:
> It might perhaps make sense to use a wrapper for this import, similar to what
we
> do for crx_id, for instance:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

So far typ is only imported in this one file, so I wouldn't create another
wrapper for it. I don't particularly expect other files to start including typ,
but if something changes, then, sure, a wrapper might make sense.

[achuithb, 2014-10-29 19:57:30]

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
tools/telemetry/telemetry/unittest/run_tests.py:14:
util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'third_party', 'typ')
On 2014/10/29 19:47:18, Dirk Pranke wrote:
> On 2014/10/29 19:42:02, achuithb wrote:
> > You need to add this to bootstrap_deps here:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/bo...
> > 
> 
> Hm. I don't really understand how (or who) is using this, and so whether
someone
> who isn't a telemetry dev would need to run the tests and wouldn't have a full
> checkout. The 'standalone telemetry' users may not need this, for example.
> 
> Does ChromeOS not have a full Chrome checkout?
> 
> Dave, any thoughts on this? 

ChromeOS uses bootstrap_deps for installing telemetry dependencies. 

I assume this is being used to generate standalone telemetry, but not sure -
Dave would know.

https://codereview.chromium.org/659293003/diff/100001/tools/telemetry/telemet...
tools/telemetry/telemetry/unittest/run_tests.py:16: import typ
On 2014/10/29 19:47:18, Dirk Pranke wrote:
> On 2014/10/29 19:42:02, achuithb wrote:
> > It might perhaps make sense to use a wrapper for this import, similar to
what
> we
> > do for crx_id, for instance:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...
> 
> So far typ is only imported in this one file, so I wouldn't create another
> wrapper for it. I don't particularly expect other files to start including
typ,
> but if something changes, then, sure, a wrapper might make sense.

Acknowledged.

[Dirk Pranke, 2014-11-03 22:52:44]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-11-03 22:54:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659293003/120002

[commit-bot: I haz the power, 2014-11-03 23:01:57]

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

[commit-bot: I haz the power, 2014-11-03 23:02:00]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Dirk Pranke, 2014-11-03 23:15:58]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2014-11-03 23:16:23]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2014-11-03 23:17:47]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-11-03 23:19:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659293003/150001

[commit-bot: I haz the power, 2014-11-03 23:19:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659293003/150001

[Dirk Pranke, 2014-11-04 02:59:58]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-11-04 03:01:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659293003/170001

[commit-bot: I haz the power, 2014-11-04 04:00:15]

Committed patchset #9 (id:170001)

[commit-bot: I haz the power, 2014-11-04 04:00:49]

Patchset 9 (id:??) landed as
https://crrev.com/3e7c2a6ee80e05fc6b8090bee850c40b8a2a3810
Cr-Commit-Position: refs/heads/master@{#302570}

[tzik, 2014-11-04 05:03:30]

A revert of this CL (patchset #9 id:170001) has been created in
https://codereview.chromium.org/700703003/ by tzik@chromium.org.

The reason for reverting is: This CL causes telemetry_perf_unittests failure:
http://build.chromium.org/p/chromium.linux/buildstatus?builder=Linux%20Tests&...
http://build.chromium.org/p/chromium.mac/buildstatus?builder=Mac%2010.6%20Tes...

Its stack trace was:
Traceback (most recent call last):
  <module> at tools/telemetry/telemetry/unittest/run_tests.py:179
    RunTestsCommand.main()
  main at tools/telemetry/telemetry/unittest/run_tests.py:85
    return obj.Run(options)
  Run at tools/telemetry/telemetry/unittest/run_tests.py:126
    ret, _, _ = runner.run()
  run at third_party/typ/typ/runner.py:187
    upload_ret = self._upload(full_results)
  _upload at third_party/typ/typ/runner.py:599
    full_results)
  make_upload_request at third_party/typ/typ/json_results.py:113
    content_type, data = _encode_multipart_form_data(attrs, full_results)
  _encode_multipart_form_data at third_party/typ/typ/json_results.py:183
    body = CRLF.join(lines)
TypeError: sequence item 3: expected string, NoneType found
.