1281932 - Intermittent dom/u2f/tests/test_frame.html | Test timed out.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Treeherder Bug Filer]

Filed by: [e-mail]

https://treeherder.mozilla.org/logviewer.html#?job_id=10074352&repo=fx-team

http://archive.mozilla.org/pub/firefox/tinderbox-builds/fx-team-win64-debug/1466687727/fx-team_win8_64-debug_test-mochitest-4-bm109-tests1-windows-build1004.txt.gz

Comment 1

[BMO Automation]

Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.

Comment 2

[J.C. Jones [:jcj] (he/him) [taking a break]]

Attachment: Bug 1281932 - Fix intermittent u2f tests

This is reworking the U2F tests to do two things:

1) Don't run all the tests in one big frame; that makes it hard to tell
   what test is actually dying in Treeherder.
2) Fix the obvious possible test races with the async functions which could be
   causing the intermittent

Review commit: https://reviewboard.mozilla.org/r/84354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/84354/

Comment 3

[J.C. Jones [:jcj] (he/him) [taking a break]]

Try run looks OK: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93590064e39f

Comment 4

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8799059 [details]
Bug 1281932 - Fix intermittent u2f tests

https://reviewboard.mozilla.org/r/84354/#review83230

Ok - lgtm.

::: dom/u2f/tests/mochitest.ini:2
(Diff revision 1)
>  [DEFAULT]
>  support-files =

My assumption would be that support-files should be sorted, but it's probably not a big deal

::: dom/u2f/tests/test_appid_facet.html:26
(Diff revision 1)
> +
> +SpecialPowers.pushPrefEnv({"set": [["security.webauth.u2f", true],
> +                                   ["security.webauth.u2f_enable_softtoken", true],
> +                                   ["security.webauth.u2f_enable_usbtoken", false]]},
> +function() {
> +  // listen for a messages from the mixed content test harness

nit: this comment appears to be a bit outdated at this point (same in all of the changed test_* files)

::: dom/u2f/tests/frame_appid_facet.html:14
(Diff revision 1)
>  "use strict";
>  
> +var countCompletions = 0;
> +var expectedCompletions = 5;
> +
> +function completed() {

Seems like completed could be defined in u2futil.js.

::: dom/u2f/tests/frame_appid_facet.html:16
(Diff revision 1)
> +var countCompletions = 0;
> +var expectedCompletions = 5;
> +
> +function completed() {
> +  countCompletions += 1
> +  if (countCompletions == expectedCompletions) {

It might be good to note a test failure if ever countCompletions > expectedCompletions.

::: dom/u2f/tests/u2futil.js:3
(Diff revision 1)
> +function handleEventMessage(event) {
> +    if ("test" in event.data) {
> +      var summary = event.data.test + ": " + event.data.msg;

nit: s/var/let/ here

::: dom/u2f/tests/u2futil.js:8
(Diff revision 1)
> +      var summary = event.data.test + ": " + event.data.msg;
> +      log(event.data.status + ": " + summary);
> +      ok(event.data.status, summary);
> +    } else if ("done" in event.data) {
> +      SimpleTest.finish();
> +    }

else indicate a test failure?

::: dom/u2f/tests/u2futil.js:11
(Diff revision 1)
> +    } else if ("done" in event.data) {
> +      SimpleTest.finish();
> +    }
> +  }
> +
> +function log(msg) {

console.log might be more helpful ultimately (or maybe in addition?)

Comment 5

[J.C. Jones [:jcj] (he/him) [taking a break]]

Comment on attachment 8799059 [details]
Bug 1281932 - Fix intermittent u2f tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84354/diff/1-2/

Comment 6

[J.C. Jones [:jcj] (he/him) [taking a break]]

Comment on attachment 8799059 [details]
Bug 1281932 - Fix intermittent u2f tests

https://reviewboard.mozilla.org/r/84354/#review83286

Comment 7

[J.C. Jones [:jcj] (he/him) [taking a break]]

Comment on attachment 8799059 [details]
Bug 1281932 - Fix intermittent u2f tests

https://reviewboard.mozilla.org/r/84354/#review83230

> Seems like completed could be defined in u2futil.js.

Yeah, I was lazy. :) Added `local_addTest` and `local_completeTest` to `u2futil.js` in patch 2.

> nit: s/var/let/ here

Thanks. I fixed this throughout `u2futil.js`.

> else indicate a test failure?

Good idea.

> console.log might be more helpful ultimately (or maybe in addition?)

Tossed it in there.

Comment 8

[J.C. Jones [:jcj] (he/him) [taking a break]]

Comment on attachment 8799059 [details]
Bug 1281932 - Fix intermittent u2f tests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/84354/diff/2-3/

Comment 9

[J.C. Jones [:jcj] (he/him) [taking a break]]

Second try run looks quite good. Marking checkin-needed.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6bd83dd40a3&selectedJob=28926054

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1477dd6413c
Fix intermittent u2f tests r=keeler

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c1477dd6413c

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cb53dde69746

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ed4dbff3f511