1089190 - Add testing for offline network pages

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: offline-test v0.1

This test adds some testing for offline network error pages. I stole the core of this test from:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/browser/browser_bug680727.js

Try run here, show the test passing (look at the logs themselves) in RC3 (Android 2.3) and RC4 (Android 4)

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0b127fa4c060

Comment 1

[Brian Nicholson (:bnicholson)]

Comment on attachment 8511545 [details] [diff] [review]
offline-test v0.1

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

::: mobile/android/base/tests/testOfflinePage.java
@@ +1,1 @@
> +package org.mozilla.gecko.tests;

Not sure why so many of our tests are missing the license header, but we should probably include it.

::: mobile/android/base/tests/testOfflinePage.js
@@ +71,5 @@
> +  browser.removeEventListener("DOMContentLoaded", errorListener, true);
> +  ok(Services.io.offline, "Services.io.offline is true.");
> +
> +  // This is an error page.
> +  is(browser.contentDocument.documentURI.substring(0, 27), "about:neterror?e=netOffline", "Document URI is the error page.");

Heh, I know this is copied, but this is silly. We should extract this string into a variable and use .length like sane people.

@@ +86,5 @@
> +
> +  ok(browser.contentDocument.getElementById("errorTryAgain"), "The error page has got a #errorTryAgain element");
> +  chromeWin.setTimeout(function() {
> +    browser.contentDocument.getElementById("errorTryAgain").click();
> +  }, 1000);

I'm guessing this delay is needed from offline true -> false switch? Arbitrary timeouts always suck, so I wonder if there's a more reliable way to detect whether we're ready.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Brian Nicholson (:bnicholson) from comment #1)

> ::: mobile/android/base/tests/testOfflinePage.java
> @@ +1,1 @@
> > +package org.mozilla.gecko.tests;
> 
> Not sure why so many of our tests are missing the license header, but we
> should probably include it.

Done

> ::: mobile/android/base/tests/testOfflinePage.js

> > +  is(browser.contentDocument.documentURI.substring(0, 27), "about:neterror?e=netOffline", "Document URI is the error page.");
> 
> Heh, I know this is copied, but this is silly. We should extract this string
> into a variable and use .length like sane people.

I kept this since we need to make sure it matches the about:error string.

> > +  ok(browser.contentDocument.getElementById("errorTryAgain"), "The error page has got a #errorTryAgain element");
> > +  chromeWin.setTimeout(function() {
> > +    browser.contentDocument.getElementById("errorTryAgain").click();
> > +  }, 1000);
> 
> I'm guessing this delay is needed from offline true -> false switch?
> Arbitrary timeouts always suck, so I wonder if there's a more reliable way
> to detect whether we're ready.

Needed for visual local testing. I removed the timeout

https://hg.mozilla.org/integration/fx-team/rev/0dce9d625fb5

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0dce9d625fb5

Comment 4

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> (In reply to Brian Nicholson (:bnicholson) from comment #1)
> 
> > ::: mobile/android/base/tests/testOfflinePage.java
> > @@ +1,1 @@
> > > +package org.mozilla.gecko.tests;
> > 
> > Not sure why so many of our tests are missing the license header, but we
> > should probably include it.
> 
> Done

Aren't tests usually in the public domain, not MPL?

Also, I wonder if we could figure out how to reproduce what I'm seeing in bug 1083213 with a test.

Comment 5

[Richard Newman [:rnewman]]

(In reply to :Margaret Leibovic from comment #4)

> Aren't tests usually in the public domain, not MPL?

If I recall from long ago, that was only because the PD license block was much shorter than the MPL 1.1 license block!

Comment 6

[Kevin Brosnan [Ex-Mozilla]]

https://www.mozilla.org/MPL/license-policy.html suggests PD for test code