Enhance waitForPageLoad() with more details for the load status in case of timeout

RESOLVED FIXED

Status

Testing Graveyard
Mozmill
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: AndreeaMatei, Assigned: whimboo)

Tracking

unspecified
Bug Flags:
in-testsuite +

Details

(Whiteboard: [mentor=whimboo][lang=js][mozmill-2.0.6], URL)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
In bug 826693, we still have some tests that fail from time to time, the result being the page not being loaded: "controller.waitForPageLoad(): Timeout waiting for page loaded."

We need to add more details to the method so we can see what happens exactly in this situation.

Mozmill 2.0.3 clearly has improved the rate of this failure, but we still encounter it.
(Assignee)

Comment 1

4 years ago
I would also suggest that we add which URL we were waiting for to be loaded. Also in case if the wait was successful we should add the URL.
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Updated

4 years ago
Whiteboard: [mentor=whimboo][lang=js]
Hi Henrik. I would like to work on this bug. Can you please assign it to me?
(Assignee)

Comment 3

4 years ago
We have seen again a lot of waitForPageLoad issues recently, which are aren't sure what's causing those. So having this bug in the next release would be a great help.
Blocks: 971796
Whiteboard: [mentor=whimboo][lang=js] → [mentor=whimboo][lang=js][mozmill-2.0.6?]
(Assignee)

Updated

4 years ago
Assignee: nobody → sammygamer
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
The code which needs to be enhanced can be found here:
https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/extension/resource/driver/controller.js#L986

We will not be able to include the information by simply adding it to the message string, given that it will be evaluated only when entering the assert.waitFor() call. Looks like we have to catch the exception, and re-throw it with the details added.
(Assignee)

Comment 5

4 years ago
I will take it so that we can have it fixed for mozmill 2.0.6.
Assignee: sammygamer → hskupin
(Assignee)

Comment 6

4 years ago
Created attachment 8378984 [details] [diff] [review]
Patch v1

This patch adds the requested details to the log messages of waitForPageLoad(). So we now know about the URI being loaded, and the latest readyState.

Cosmin, can you please test it with our mozmill tests on Windows and OS X? Thanks.
Attachment #8378984 - Flags: review?(ahalberstadt)
Attachment #8378984 - Flags: feedback?(cosmin.malutan)
Comment on attachment 8378984 [details] [diff] [review]
Patch v1

Review of attachment 8378984 [details] [diff] [review]:
-----------------------------------------------------------------

::: mutt/mutt/tests/js/testUtils/testPageLoad.js
@@ +84,5 @@
> +  //   controller.waitForPageLoad(0);
> +
> +  //   throw new Error("controller.waitForPageLoad() not timed out for timeout=0.");
> +  // } catch (ex if ex isinstanceof errors.TimeoutError) {
> +  // }

Should probably delete this? The old version is still in the vcs.
Attachment #8378984 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 8

4 years ago
(In reply to Andrew Halberstadt [:ahal] from comment #7)
> > +  //   controller.waitForPageLoad(0);
> > +
> > +  //   throw new Error("controller.waitForPageLoad() not timed out for timeout=0.");
> > +  // } catch (ex if ex isinstanceof errors.TimeoutError) {
> > +  // }
> 
> Should probably delete this? The old version is still in the vcs.

I would leave this in. In a couple of cases we found that people were interested in fixing those issues. It's like a skip in the manifest, which we sadly cannot do here.

Waiting for the feedback from Cosmin before it gets landed.
I had ran the mutt testpy twice, mutt testall once, testrun_remote and testrun_functional for Windows 8.1 x86 and osx 10.7.5 machines.
Everything ran great, it doesn't broke anything.
Thanks
Attachment #8378984 - Flags: feedback?(cosmin.malutan) → feedback+
(Assignee)

Comment 10

4 years ago
https://github.com/mozilla/mozmill/commit/0ca6115f5952079be1a63ace839393000456467e (master)
https://github.com/mozilla/mozmill/commit/5c4f9ae38bbb70548bc5886f58adde404c648538 (hotfix-2.0)
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [mentor=whimboo][lang=js][mozmill-2.0.6?] → [mentor=whimboo][lang=js][mozmill-2.0.6]
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.