Closed Bug 443017 Opened 16 years ago Closed 16 years ago

Create tests for new application cache stuff

Categories

(Core :: Networking: Cache, defect)

Product:
Core
Component:
Networking: Cache
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

Attachments

(1 file, 9 obsolete files)

ng 1.1
(deleted), patch
dcamp
: review+
jst
: superreview+
Details | Diff | Splinter Review
Attached patch First simple tests (obsolete) (deleted) — — Splinter Review 
New tests for:
- opportunistic caching
- whitelisting
- foreign marking
- fallbacks

We will need to find out how to modify manifest files to test updates and swapCache. Also I am not sure how to do testing of fallback for images, probably introduce a kind of reftesting to mochitest or wait for bug 430652 to land.
Depends on: 443023
Depends on: 443610
Attached patch Complete tests, v1 (obsolete) (deleted) — — Splinter Review 
Tests for:
- opportunistic caching
- opportunistic caching propagation (among different caches)
- whitelisting + check we load from network
- foreign marking
- fallbacks
- complex test for cache update
Assignee: nobody → honzab
Attachment #327668 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Test for opportunistic caching propagation changed to 'todo'. After bug 444807 gets fixed the test will be reintroduced.
Attachment #328983 - Attachment is obsolete: true
Attachment #329152 - Flags: review?(dcamp)
Attached patch Complete test, v3 (obsolete) (deleted) — — Splinter Review 
same as v2, cache update test enhanced of dynamic entries carry check.
Attachment #329152 - Attachment is obsolete: true
Attachment #329243 - Flags: review?(dcamp)
Attachment #329152 - Flags: review?(dcamp)
Attached patch Complete test, v4 (obsolete) (deleted) — — Splinter Review 
fixed cache discard in opportunistic propagation test and added new test for behavior in offline mode.
Attachment #329243 - Attachment is obsolete: true
Attachment #330196 - Flags: review?(dcamp)
Attachment #329243 - Flags: review?(dcamp)
Comment on attachment 330196 [details] [diff] [review]
Complete test, v4


>-  applicationCache.swapCache();
>+  // XXX: @666 Why is swapCache() call here? there is not an update
>+  // nor a newer cache and swapCache() therefor throws an exception
>+  // according to the spec what breaks the test.
>+  //applicationCache.swapCache();

The swapCache() call was there because we need 442812 before the cache is assoicated by default.  So there is a newer cache available.

Rather than commenting it out, this should be changed to a test - make sure that the exception IS thrown (and if the tests land before 442812, this can be a todo() test).
This bug introduces a bunch of tests for all the offline cache stuff.
Attached patch v5 (obsolete) (deleted) — — Splinter Review 
Some new tests, addressed comments.
Attachment #330196 - Attachment is obsolete: true
Attachment #334324 - Flags: review?(dcamp)
Attachment #330196 - Flags: review?(dcamp)
Attached patch v6 (obsolete) (deleted) — — Splinter Review 
- changed blocking to false in waitForAdd because I almost every time experience deadlock in this method; that is more a quick workaround then real fix, see bug 445733
- updated the way of on-server data override management; bug 443610 has been landed
Attachment #334324 - Attachment is obsolete: true
Attachment #337306 - Flags: review?(dcamp)
Attachment #334324 - Flags: review?(dcamp)
Depends on: 461327
Attached patch v7 (obsolete) (deleted) — — Splinter Review 
These are tests fixed to work with new nsIApplicationCacheNamespace interface. There are TODOs for new implicit (master) entries caching and for opportunistic cache propagation (autocaching).

After bug 443023 and bug 461327 are both landed we can land these tests.
Attachment #337306 - Attachment is obsolete: true
Attachment #344493 - Flags: review?(dcamp)
Attachment #337306 - Flags: review?(dcamp)
Hrm, can we remove the opportunistic tests, since we're about to remove that functionality anyway?
Er... we are?  Why did we implement it, then?
It was removed from the spec recently after some argument that it's not a particularly useful feature (there was some significant overlap with implicitly-cached entries).  Tracked in bug 460327.
Blocks: 460353
Attached patch ng 1 (obsolete) (deleted) — — Splinter Review 
Includes simplified test set (no opportunistic caching).
Attachment #344493 - Attachment is obsolete: true
Attachment #345627 - Flags: review?(dcamp)
Attachment #344493 - Flags: review?(dcamp)
Depends on: 445544, 460327
Comment on attachment 345627 [details] [diff] [review]
ng 1

Kinda hard to follow the flow of the patch top to bottom, so I'm gonna go through each test_* and their necessary files in a separate comment.  First up, test_fallback.html:

>diff --git a/dom/tests/mochitest/ajax/offline/test_fallback.html b/dom/tests/mochitest/ajax/offline/test_fallback.html
>new file mode 100644
>--- /dev/null
>+++ b/dom/tests/mochitest/ajax/offline/test_fallback.html
>@@ -0,0 +1,132 @@
>+<html xmlns="http://www.w3.org/1999/xhtml" manifest="http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/fallback.cacheManifest">
>+<head>
>+<title>Fallback entry test</title>
>+
>+<script type="text/javascript" src="/MochiKit/packed.js"></script>
>+<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
>+<script type="text/javascript" src="/tests/dom/tests/mochitest/ajax/offline/offlineTests.js"></script>
>+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
>+
>+<script class="testbody" type="text/javascript">
>+
>+/**
>+ * This test check we appropriatelly fall back to correct
>+ * fallback entries in nested namespace contained in "opportunistic"
>+ * manifest. There are two fallback entries in namespace1/ and
>+ * in namespace1/sub/ directories. The former one must fall back to
>+ * fallback1.html and the letter one to fallback2.html.
>+ * The test is performed two times to check there is not created
>+ * any cache entry for non-existing items. That 1) is wrong 2) would
>+ * lead to failure of fall back redirection because we would try
>+ * to load the non-existing page from offline application cache.
>+ */

"This tests that we fall back to the correct fallback entries when loading from fallback namespaces listed in the manifest.  The test is performed twice to make sure that no cache entries are created for the nonexistent items."
That would lead to a failure to fallback, as there would always be an entry in the application cache."

Note that I left out the repetition of the manifest - you actually had a mismatch (namespace1 seems to load "fallback.html" and sub/ loads "fallback2.html".

>+var gManifestUpdated = false;
>+var gStep = 1;
>+var gChildLoad = false;
>+var gTopWindow = null;
>+var gCompleteTimeout = null;
>+
>+function manifestUpdated()
>+{
>+  gManifestUpdated = true;
>+  fallbackFrame.location = "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/non-existing.html";

To make this a bit easier to follow, could you mention which fallback should be loaded each time you change the location here?

>+    case 005:
>+      // Try all over again. This checks there are no entries for non-existing
>+      // pages created/leaked. That would prevent fallback load.
>+      gStep = 100;
>+      fallbackFrame.location = "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/namespace1/non-existing.html";
>+      // no break
Comment on attachment 345627 [details] [diff] [review]
ng 1

OK, now test_foreign.html:

>diff --git a/dom/tests/mochitest/ajax/offline/test_foreign.html b/dom/tests/mochitest/ajax/offline/test_foreign.html
>new file mode 100644
>+/**
>+ * This test loads a manifest that contains as an explicit entry
>+ * foreign2.html page. After manifest is cached and presence of the
>+ * foreign2.html page is checked we redirect to foreign2.html
>+ * page. Then the test continues inside that page as follows:
>+ * On load of foreign2.html we check there is no associated cache with it

just "no associated cache"

>+ * because the foreign2.html page was marked as FOREIGN in foreign1 cache.
>+ * After the foreign2 manifest is updated we check foreign2 cache is
>+ * choosed by foreign.html page.

s/choosed/chosen/ here and elsewhere.
Comment on attachment 345627 [details] [diff] [review]
ng 1

OK a few at once here...

test_offlineMode.html looks fine...
offlineTests.js looks fine...

>diff --git a/dom/tests/mochitest/ajax/offline/opportunisticPropagation2.html b/dom/tests/mochitest/ajax/offline/opportunisticPropagation2.html
>diff --git a/dom/tests/mochitest/ajax/offline/test_opportunisticCaching.html b/dom/tests/mochitest/ajax/offline/test_opportunisticCaching.html
>diff --git a/dom/tests/mochitest/ajax/offline/test_opportunisticPropagation.html b/dom/tests/mochitest/ajax/offline/test_opportunisticPropagation.html

We should completely remove the opportunistic tests.

>diff --git a/dom/tests/mochitest/ajax/offline/test_updatingManifest.html b/dom/tests/mochitest/ajax/offline/test_updatingManifest.html
>new file mode 100644
>--- /dev/null
>+++ b/dom/tests/mochitest/ajax/offline/test_updatingManifest.html

>+
>+const NAMESPACE_BYPASS = Components.interfaces.nsIApplicationCacheNamespace.NAMESPACE_BYPASS;
>+const NAMESPACE_FALLBACK = Components.interfaces.nsIApplicationCacheNamespace.NAMESPACE_FALLBACK;
>+const NAMESPACE_OPPORTUNISTIC = Components.interfaces.nsIApplicationCacheNamespace.NAMESPACE_OPPORTUNISTIC;

We should probably remove/rename references to opportunistic cache in this file (there are a few places).

>+
>+var manifestVersion2Content =
>+  "CACHE MANIFEST\n" +
>+  "# v2\n" +
>+  "\n" +
>+  "http://localhost:8888/tests/SimpleTest/SimpleTest.js\n" +
>+  "http://localhost:8888/MochiKit/packed.js\n" +
>+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js\n" +
>+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingIFrame.html" +
>+  "\n" +
>+  "FALLBACK:\n" +
>+  "namespace1/ fallback.html\n" +
>+  "namespace1/sub/ fallback2.html\n";
>+
>+var manifestVersion3Content =
>+  "CACHE MANIFEST\n" +
>+  "# v3\n" +
>+  "\n" +
>+  "http://localhost:8888/MochiKit/packed.js\n" +
>+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/offlineTests.js\n" +
>+  "http://localhost:8888/tests/dom/tests/mochitest/ajax/offline/updatingIFrame.html" +
>+  "\n" +
>+  "FALLBACK:\n" +
>+  "namespace1/sub fallback2.html\n" +
>+  "\n" +
>+  "NETWORK:\n" +
>+  "onwhitelist.html\n";
>+

Just out of curiosity, have you ever run into race conditions or other problems here with the manifests being cached by the http cache?  I had some trouble with that, I wonder if we should add support to the PUT command in the test server for setting Cache-Control headers?

>diff --git a/dom/tests/mochitest/ajax/offline/test_whitelisting.html b/dom/tests/mochitest/ajax/offline/test_whitelisting.html
>new file mode 100644
>--- /dev/null
>+++ b/dom/tests/mochitest/ajax/offline/test_whitelisting.html

This test is fine for this implementation, and is presumably changed in the patch to have a cache-per-iframe...
OK, I think that should be everything.  Mostly just nits.
Attached patch ng 1.1 (deleted) — — Splinter Review 
Completely removed tests for opportunistic caching + few nits.
Attachment #345627 - Attachment is obsolete: true
Attachment #345728 - Flags: review?(dcamp)
Attachment #345627 - Flags: review?(dcamp)
Blocks: 461325
Blocks: 460328
Blocks: 460335
Attachment #345728 - Flags: review?(dcamp) → review+
Attachment #345728 - Flags: superreview+
Landed as http://hg.mozilla.org/mozilla-central/rev/d824478adb04
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Depends on: 463804
Any ideas why a couple of the tests added here are failing with the HTML5 parser? See bug 541079.
Depends on: 541396
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: