[ServiceWorker] Tests for Cache

LayoutTests/http/tests/serviceworker/resources/cache-match-test-worker.js

+importScripts('worker-test-harness.js');
+//
+// Variations:
+//   - ignoreSearch : Ignores search parameter.
+//   - prefixMatch : only matches a prefix of the URL.
+//   - ignoreVary : ignores a 'Vary' header if there is one.
+var entries = {
+  a: {
+    request: new Request('http://example.com/a'),
+    response: new Response('')
+  },
+
+  b: {
+    request: new Request('http://example.com/b'),
+    response: new Response('')
+  },
+
+  a_with_query: {
+    request: new Request('http://example.com/a?q=r'),
+    response: new Response('')
+  },
+
+  A: {
+    request: new Request('http://example.com/A'),
+    response: new Response('')
+  },
+
+  a_https: {
+    request: new Request('https://example.com/a'),
+    response: new Response('')
+  },
+
+  a_org: {
+    request: new Request('http://example.org/a'),
+    response: new Response('')
+  },
+
+  cat: {
+    request: new Request('http://example.com/cat'),
+    response: new Response('')
+  },
+
+  cat_with_fragment: {
+    request: new Request('http://example.com/cat#mouse'),
+    response: new Response('')
+  },
+
+  cat_in_the_hat: {
+    request: new Request('http://example.com/cat/in/the/hat'),
+    response: new Response('')
+  },
+
+  c_is_for_cookie: {
+    request: new Request('http://example.com/c',
+                         { headers: { 'Cookies': 'is-for-cookie' } }),

[jsbell, 2014/08/15 20:49:34]

That's good enough for me!

+    response: new Response('',
+                           { headers: { 'Vary': 'Cookies' } })

[jsbell, 2014/08/15 20:49:34]

But actually.. per style guide, no space after { or before } for single-line
initializers.

+  },
+
+  c_is_for_cake: {
+    request: new Request('http://example.com/c',
+                         { headers: { 'Cookies': 'is-for-cake' } }),
+    response: new Response('',
+                           { headers: { 'Vary': 'Cookies' } })
+  },
+
+  c_x_key_is_1: {
+    request: new Request('http://example.com/c',
+                         { headers: { 'Cookies': 'x', 'X-Key': '1'} }),
+    response: new Response('',
+                           { headers: { 'Vary': '*' } })
+  },
+
+  c_y_key_is_1: {
+    request: new Request('http://example.com/c',
+                         { headers: { 'Cookies': 'y', 'X-Key': '1' } }),
+    response: new Response('',
+                           { headers: { 'Vary': 'Cookies,X-Key'} })
+  },
+
+  c_y_key_is_2: {
+    request: new Request('http://example.com/c',
+                         { headers: { 'Cookies': 'y', 'X-Key': '2' } }),
+    response: new Response('',
+                           { headers: { 'Vary': 'Cookies,X-Key'} })
+  }
+};
+
+promise_test(function(t) {
+    var cache = new Cache();

[jsbell, 2014/08/15 20:49:33]

Just noticing that the constructor isn't actually in the spec, but
https://github.com/slightlyoff/ServiceWorker/issues/372 says there will be one.
Hopefully it stays!

[asanka, 2014/08/20 03:11:58]

I replaced the Cache() constructor with a helper that creates a new temporary
cache.

[jsbell, 2014/08/20 18:18:08]

Great idea!

+
+    put_all_requests_in_cache(cache)
+
+      .then(function() {
+          return cache.matchAll(entries.a.request.url);
+        })
+      .then(t.step_func(function(result) {

[jsbell, 2014/08/15 20:49:33]

t.step_func shouldn't be necessary in a promise_test (here and below)

[asanka, 2014/08/20 03:11:58]

Removed.

+          assert_array_equals(result, [entries.a.response],
+                              'Cache.matchAll should match by URL.');
+        }))
+
+      .then(function() {

[jsbell, 2014/08/15 20:49:33]

Since these cases are independent, can we split this up rather than having it be
one big promise_test?

If something fails (or the impl is incomplete) it'd be nice to see 'pass / pass
/ pass / fail (reason #1) / pass / fail (reason #2)' in the output rather than
just 'fail (reason #1)'

[asanka, 2014/08/20 03:11:58]

Done.

+          return cache.matchAll(entries.a.request);
+        })
+      .then(t.step_func(function(result) {
+          assert_array_equals(result, [entries.a.response],
+                              'Cache.matchAll should match by Request.');
+        }))
+
+      .then(function() {
+          return cache.matchAll(new Request(entries.a.request.url));
+        })
+      .then(t.step_func(function(result) {
+          assert_array_equals(result, [entries.a.response],
+                              'Cache.matchAll should match by Request.');
+        }))
+
+      .then(function() {
+          return cache.match(entries.a.request);
+        })
+      .then(t.step_func(function(result) {
+          assert_equals(result, entries.a.response,
+                        'Cache.match should match by Request.');
+        }))
+
+      .then(function() {
+          return cache.matchAll(entries.a.request, { ignoreSearch: true });
+        })
+      .then(t.step_func(function(result) {
+          assert_array_equivalent(
+            result, [entries.a.response, entries.a_with_query.response],
+            'Cache.matchAll with ignoreSearch should ignore the search ' +
+            'parameters of cached request.');
+        }))
+
+      .then(function() {
+          return cache.matchAll(entries.a_with_query.request,
+                                { ignoreSearch: true });
+        })
+      .then(t.step_func(function(result) {
+          assert_array_equivalent(
+            result, [entries.a.response, entries.a_with_query.response],
+            'Cache.matchAll with ignoreSearch should ignore the search ' +
+            'parameters of request.');
+        }))
+
+      .then(function() {
+          return cache.matchAll(entries.cat.request);
+        })
+      .then(t.step_func(function(result) {
+          assert_array_equivalent(
+            result, [entries.cat.response, entries.cat_with_fragment.response],
+            'Cache.matchAll should ignore URL hash.');
+        }))
+
+      .then(function() {
+          return cache.matchAll('http');
+        })
+      .then(t.step_func(function(result) {
+          assert_array_equivalent(
+            result, [],
+            'Cache.matchAll should treat query as a URL and not just a string fragment.');

[jsbell, 2014/08/15 20:49:34]

nit: line length (80 cols)

[asanka, 2014/08/20 03:11:58]

Done.

+        }))
+
+      .then(function() {
+          return cache.matchAll('http://example.com/cat', { prefixMatch: true });

[jsbell, 2014/08/15 20:49:34]

nit: line length (80 cols)

[asanka, 2014/08/20 03:11:58]

Done.

+        })
+      .then(t.step_func(function(result) {
+          assert_array_equivalent(
+            result,
+            [ entries.cat.response,

[jsbell, 2014/08/15 20:49:33]

nit: break after [ and before ] for multi-line initializers, per style guide

[asanka, 2014/08/20 03:11:58]

Done.

+              entries.cat_with_fragment.response,
+              entries.cat_in_the_hat.response ],
+            'Cache.matchAll should honor prefixMatch.');
+        }))
+
+      .then(function() {
+          return cache.matchAll('http://example.com/cat/', { prefixMatch: true });

[jsbell, 2014/08/15 20:49:34]

nit: line length (80 cols)

[asanka, 2014/08/20 03:11:58]

Done.

+        })
+      .then(t.step_func(function(result) {
+          assert_array_equivalent(
+            result, [ entries.cat_in_the_hat.response ],
+            'Cache.matchAll should honor prefixMatch.');
+        }));
+
+  }, 'Cache.match');
+
+// Helpers ---
+
+function put_all_requests_in_cache(cache) {

[jsbell, 2014/08/15 20:49:34]

Could be written as:

return Promise.all(entries.map(function(e) {
  return cache.put(e.request, e.response); 
}));

This also avoids using for...in to enumerate properties, which (while fine here)
is one of those fragile bits of JS.

[asanka, 2014/08/20 03:11:58]

Ah. Thanks! Changed.

+  var promise_array = [];
+  for (v in entries) {
+    promise_array.push(cache.put(entries[v].request,
+                                 entries[v].response));
+  }
+  return Promise.all(promise_array);
+}
+
+function assert_array_equivalent(actual, expected, description) {

[jsbell, 2014/08/15 20:49:34]

For consistency with assert_array_equals(), rename to: assert_array_equivalent()
?

[jsbell, 2014/08/15 20:49:34]

Can you add a comment that this is comparing the contents of arrays ignoring
order, since the match result order may be indeterminate?

[asanka, 2014/08/20 03:11:58]

Added a comment.

[asanka, 2014/08/20 03:11:58]

Hmm. Were you going to suggest a different name?

[jsbell, 2014/08/20 18:18:08]

I must have been misreading it. Name is fine, sorry!

+  assert_true(Array.isArray(actual), description);
+  assert_equals(actual.length, expected.length, description);
+  for (var index = 0; index < expected.length; ++index) {
+    assert_greater_than_equal(actual.indexOf(expected[index]), 0, description);

[jsbell, 2014/08/15 20:49:34]

This could use assert_in_array()

[asanka, 2014/08/20 03:11:58]

Done.

+  }
+}