[telemetry] Retry unit tests after failures in setUpClass and setUpModule.

[telemetry] Retry unit tests after failures in setUpClass and setUpModule.

unittest produces a suite._ErrorHolder object in these cases. We can parse its description to figure out what class/module it is and what tests we have in that class/module.


BUG=405766
TEST=Make a TestCase fail setUpClass, run it with --retry-limit=3 and check for retries.
R=dpranke, tonyg

Committed: https://crrev.com/fa7127af0bb95dcfbfd6b8c084145396140413d3
Cr-Commit-Position: refs/heads/master@{#292524}

[tonyg, 2014-08-28 17:21:33]

lgtm

[Dirk Pranke, 2014-08-28 17:40:05]

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

[Dirk Pranke, 2014-08-28 17:40:05]

this can probably land as-is, but I think there might be one bug, and there's a
few things that can probably be clearer ...

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
File tools/telemetry/telemetry/unittest/json_results.py (right):

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:176: match =
re.match('setUp[a-zA-z]+ \\((.+)\\)', test.id())
Do we see anything other than 'setUpClass' in practice? The [a-zA-Z] seems
unnecessary, and unless I miss my guess when reading this font, the second 'z'
actually looks like a lower case 'z' (and hence probably a bug)?

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:178: for test_name in
AllTestNames(suite):
Am I understanding this correctly that this is looking for all of the tests in
the class that failed the setupClass call()? This could use a comment or a
couple of helper functions to make this clearer.

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:186: raise
NotImplementedError('Unknown test type: %s' % test.__class__)
nit: I'd probably use asserts rather than NotImplementedErrors here, but it
doesn't really matter.

[dtu, 2014-08-28 21:51:43]

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
File tools/telemetry/telemetry/unittest/json_results.py (right):

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:176: match =
re.match('setUp[a-zA-z]+ \\((.+)\\)', test.id())
On 2014/08/28 17:40:05, Dirk Pranke wrote:
> Do we see anything other than 'setUpClass' in practice? The [a-zA-Z] seems
> unnecessary, and unless I miss my guess when reading this font, the second 'z'
> actually looks like a lower case 'z' (and hence probably a bug)?

I know from reading the Python source code that only setUpClass, setUpModule,
tearDownClass, and tearDownModule are possible. I don't think we use setUpModule
in Telemetry, however.

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:178: for test_name in
AllTestNames(suite):
On 2014/08/28 17:40:04, Dirk Pranke wrote:
> Am I understanding this correctly that this is looking for all of the tests in
> the class that failed the setupClass call()? This could use a comment or a
> couple of helper functions to make this clearer.

Done.

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:186: raise
NotImplementedError('Unknown test type: %s' % test.__class__)
On 2014/08/28 17:40:05, Dirk Pranke wrote:
> nit: I'd probably use asserts rather than NotImplementedErrors here, but it
> doesn't really matter.

Done.

[Dirk Pranke, 2014-08-28 21:55:05]

lgtm

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
File tools/telemetry/telemetry/unittest/json_results.py (right):

https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/unittest/json_results.py:176: match =
re.match('setUp[a-zA-z]+ \\((.+)\\)', test.id())
On 2014/08/28 21:51:43, dtu wrote:
> On 2014/08/28 17:40:05, Dirk Pranke wrote:
> > Do we see anything other than 'setUpClass' in practice? The [a-zA-Z] seems
> > unnecessary, and unless I miss my guess when reading this font, the second
'z'
> > actually looks like a lower case 'z' (and hence probably a bug)?
> 
> I know from reading the Python source code that only setUpClass, setUpModule,
> tearDownClass, and tearDownModule are possible. I don't think we use
setUpModule
> in Telemetry, however.

I would imagine we don't see failures from tearDownClass or tearDownModule at
all. I wonder if setUpModule() manifests the same way, or differently ...

I know loadTests failures show up differently and will require a different fix.

[dtu, 2014-08-28 22:04:27]

On 2014/08/28 21:55:05, Dirk Pranke wrote:
> lgtm
> 
>
https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
> File tools/telemetry/telemetry/unittest/json_results.py (right):
> 
>
https://chromiumcodereview.appspot.com/515263003/diff/1/tools/telemetry/telem...
> tools/telemetry/telemetry/unittest/json_results.py:176: match =
> re.match('setUp[a-zA-z]+ \\((.+)\\)', test.id())
> On 2014/08/28 21:51:43, dtu wrote:
> > On 2014/08/28 17:40:05, Dirk Pranke wrote:
> > > Do we see anything other than 'setUpClass' in practice? The [a-zA-Z] seems
> > > unnecessary, and unless I miss my guess when reading this font, the second
> 'z'
> > > actually looks like a lower case 'z' (and hence probably a bug)?
> > 
> > I know from reading the Python source code that only setUpClass,
setUpModule,
> > tearDownClass, and tearDownModule are possible. I don't think we use
> setUpModule
> > in Telemetry, however.
> 
> I would imagine we don't see failures from tearDownClass or tearDownModule at
> all. I wonder if setUpModule() manifests the same way, or differently ...
> 
> I know loadTests failures show up differently and will require a different
fix.

Right, I didn't cover the tearDowns here because they wouldn't affect the set of
failed tests. The tearDowns definitely should not fail. (Though you could argue
the setUps shouldn't fail either...)

[dtu, 2014-08-28 22:04:33]

The CQ bit was checked by dtu@chromium.org

[commit-bot: I haz the power, 2014-08-28 22:05:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dtu@chromium.org/515263003/20001

[commit-bot: I haz the power, 2014-08-28 23:27:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-29 00:44:52]

Committed patchset #2 (id:20001) as 2ba764f6be345a88e9262bc7c5b42962ae1360b4

[commit-bot: I haz the power, 2014-09-10 03:04:26]

Patchset 2 (id:??) landed as
https://crrev.com/fa7127af0bb95dcfbfd6b8c084145396140413d3
Cr-Commit-Position: refs/heads/master@{#292524}