Disable testharness.js timeout.

Disable testharness.js timeout.

Reason: Our test runner has a separate timeout mechanism; currently whether the test result is Failure or Timeout depends on which happens first: hitting the testharness.js timeout or the run-webkit-tests timeout.

Disabling the testharness.js timeout by default by setting explicit_timeout was suggested by tkent@.

In some cases, this means that some tests that were hitting the testharness.js timeout in Debug builds will now be continuing to run longer, and either timing out or failing in a different way.

BUG=664268
Committed: https://crrev.com/1a8851a5030c4e6dbb51c98db5c3600d742985be
Cr-Commit-Position: refs/heads/master@{#432271}

[qyearsley, 2016-11-14 18:32:56]

Description was changed from

==========
Disable testharness.js timeout.

BUG=664268
==========

to

==========
Disable testharness.js timeout.

Reason: Our test runner has a separate timeout mechanism; currently whether the
test result is Failure or Timeout depends on which happens first: hitting the
testharness.js timeout or the run-webkit-tests timeout.

Disabling the testharness.js timeout by default by setting explicit_timeout was
suggested by tkent@.

In some cases, this means that some tests that were hitting the testharness.js
timeout in Debug builds will now be continuing to run longer, and either timing
out or failing in a different way.

BUG=664268
==========

[qyearsley, 2016-11-14 18:32:56]

qyearsley@chromium.org changed reviewers:
+ jsbell@chromium.org, tkent@chromium.org

[jsbell, 2016-11-14 19:00:57]

jsbell@chromium.org changed reviewers:
+ mek@chromium.org

[jsbell, 2016-11-14 19:00:57]

Suggesting mek@ as another reviewer here, since he had opinions about how to
address this.

[jsbell, 2016-11-14 19:10:09]

On 2016/11/14 19:00:57, jsbell wrote:
> Suggesting mek@ as another reviewer here, since he had opinions about how to
> address this.

Ah, he already commented on the bug. :)

Change lgtm per the docs, w/ follow-up expectation adjustments - assuming the
bots have only timeout-related changes (Harness Error -> [Timeout])

[tkent, 2016-11-14 22:47:55]

lgtm

[Marijn Kruisselbrink, 2016-11-14 22:49:18]

lgtm

[qyearsley, 2016-11-14 23:41:37]

On 2016/11/14 at 22:49:18, mek wrote:
> lgtm

Alright, so there's a set of tests which had baselines that included TIMEOUT;
this CL also now removes those baselines and sets the expectation line for those
tests to Timeout.

[qyearsley, 2016-11-14 23:41:41]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 23:43:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jsbell, 2016-11-15 00:01:57]

On 2016/11/14 23:41:37, qyearsley wrote:
> Alright, so there's a set of tests which had baselines that included TIMEOUT;
> this CL also now removes those baselines and sets the expectation line for
those
> tests to Timeout.

Cool - that's about the set I was expecting from some grepping.

third_party/WebKit/LayoutTests/imported/wpt/service-workers/service-worker/clients-matchall-client-types.https-expected.txt
is the only one that's not "harness_status.status = 2" but it looks fine (i.e.
failures are in line with other ones, test must just be constructed slightly
differently)

so still lgtm \o/

[jsbell, 2016-11-15 00:06:21]

On 2016/11/15 00:01:57, jsbell wrote:
>
third_party/WebKit/LayoutTests/imported/wpt/service-workers/service-worker/clients-matchall-client-types.https-expected.txt
> is the only one that's not "harness_status.status = 2" but it looks fine (i.e.
> failures are in line with other ones, test must just be constructed slightly
> differently)

... and on that note, a tangent: do we want to flag any output with expectations
containing "harness_status.status" in the auto-importer and/or PRESUBMIT? After
this change, that should indicate that the imported test is buggy in a novel
and/or our testharnessreport (etc) has a flaw.

(Not required for this CL, obviously, but could go in a backlog somewhere)

[qyearsley, 2016-11-15 00:17:12]

On 2016/11/15 at 00:06:21, jsbell wrote:
> On 2016/11/15 00:01:57, jsbell wrote:
> >
third_party/WebKit/LayoutTests/imported/wpt/service-workers/service-worker/clients-matchall-client-types.https-expected.txt
> > is the only one that's not "harness_status.status = 2" but it looks fine
(i.e.
> > failures are in line with other ones, test must just be constructed slightly
> > differently)
> 
> ... and on that note, a tangent: do we want to flag any output with
expectations containing "harness_status.status" in the auto-importer and/or
PRESUBMIT? After this change, that should indicate that the imported test is
buggy in a novel and/or our testharnessreport (etc) has a flaw.
> 
> (Not required for this CL, obviously, but could go in a backlog somewhere)

Besides logging an error, or adding a comment to the auto-import CL, or aborting
the import, are there any other good ways of flagging test that have test
harness errors?

[commit-bot: I haz the power, 2016-11-15 02:47:41]

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

[commit-bot: I haz the power, 2016-11-15 02:47:42]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-11-15 22:13:21]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2016-11-15 22:13:22]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
mek@chromium.org
Link to the patchset: https://codereview.chromium.org/2500053002/#ps20001
(title: "Change TIMEOUT baselines to Timeout expectations")

[commit-bot: I haz the power, 2016-11-15 22:14:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-15 22:30:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-15 22:35:41]

Description was changed from

==========
Disable testharness.js timeout.

Reason: Our test runner has a separate timeout mechanism; currently whether the
test result is Failure or Timeout depends on which happens first: hitting the
testharness.js timeout or the run-webkit-tests timeout.

Disabling the testharness.js timeout by default by setting explicit_timeout was
suggested by tkent@.

In some cases, this means that some tests that were hitting the testharness.js
timeout in Debug builds will now be continuing to run longer, and either timing
out or failing in a different way.

BUG=664268
==========

to

==========
Disable testharness.js timeout.

Reason: Our test runner has a separate timeout mechanism; currently whether the
test result is Failure or Timeout depends on which happens first: hitting the
testharness.js timeout or the run-webkit-tests timeout.

Disabling the testharness.js timeout by default by setting explicit_timeout was
suggested by tkent@.

In some cases, this means that some tests that were hitting the testharness.js
timeout in Debug builds will now be continuing to run longer, and either timing
out or failing in a different way.

BUG=664268

Committed: https://crrev.com/1a8851a5030c4e6dbb51c98db5c3600d742985be
Cr-Commit-Position: refs/heads/master@{#432271}
==========

[commit-bot: I haz the power, 2016-11-15 22:35:42]

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