Bug 1110493 (cachetests)

improve Service Worker Cache tests

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
5 years ago
3 months ago

People

(Reporter: bkelly, Assigned: Ehsan)

Tracking

(Depends on 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

On maple we currently have only very basic tests of the Service Worker Cache.  All the methods are used, but not all the corner cases are hit.  These tests need to be fleshed out before we can enable the feature by default.
As part of this, verify that UTF-16 characters can be used in cache names, etc.
Assignee

Updated

4 years ago
Assignee: nobody → ehsan
Assignee

Updated

4 years ago
Depends on: 1140658
Status: NEW → ASSIGNED
Assignee

Updated

4 years ago
Depends on: 1142333
Assignee

Updated

4 years ago
Depends on: 1142772
Assignee

Updated

4 years ago
Depends on: 1142790
See bug 1142333 comment 2 and bug 1142333 comment 3 for more things that need to be tested.
Assignee

Updated

4 years ago
Alias: cachetests
Assignee

Updated

4 years ago
Depends on: 1143193
Assignee

Updated

4 years ago
Depends on: 1143219
Assignee

Updated

4 years ago
Depends on: 1143222
Assignee

Updated

4 years ago
Depends on: 1143511
Assignee

Updated

4 years ago
Depends on: 1143813
Assignee

Updated

4 years ago
Depends on: 1143820
Assignee

Updated

4 years ago
Depends on: 1143833
Assignee

Updated

4 years ago
Depends on: 1143894
Assignee

Updated

4 years ago
Depends on: 1143959
Assignee

Updated

4 years ago
Depends on: 1144236
Assignee

Comment 3

4 years ago
(In reply to Ben Kelly [:bkelly] from comment #2)
> See bug 1142333 comment 2 and bug 1142333 comment 3 for more things that
> need to be tested.

I think I've covered everything on this list with my latest patches.  If something is missing, please let me know!
Assignee

Updated

4 years ago
Depends on: 1144337
Assignee

Updated

4 years ago
Depends on: 1144725
Assignee

Updated

4 years ago
Depends on: 1145445
Assignee

Updated

4 years ago
Depends on: 1145792
Assignee

Updated

4 years ago
Depends on: 1146202
Assignee

Updated

4 years ago
Depends on: 1146585
Assignee

Updated

4 years ago
Depends on: 1146612
Assignee

Updated

4 years ago
Depends on: 1147184
Assignee

Comment 4

4 years ago
Ben, with bug 1147184, I've run out of ideas on what else needs to be tested.  If you can think of others, please let me know.  Otherwise I think we can call this done.
Flags: needinfo?(bkelly)
Thanks for the awesome work Ehsan!

The only thing I can think of is to try to force some corner cases.  Things like using the Cache after its deleted, etc.  I think we do some of that already.  Another one I can think of is:

1) Open cache 'foo'
2) Put a response with a long body in the Cache
3) Match the response back out of the Cache
4) caches.delete('foo')
5) drop ref to the Cache object and force GC
6) Call response.text() to see if we can still read the stream

Also, now that we're pref'd on we could do wpt tests. :-)

I'm also ok just declaring this closed now.  I think we have pretty good coverage.
Flags: needinfo?(bkelly)
Assignee

Comment 6

4 years ago
(In reply to Ben Kelly [:bkelly] from comment #5)
> Thanks for the awesome work Ehsan!
> 
> The only thing I can think of is to try to force some corner cases.  Things
> like using the Cache after its deleted, etc

This should be tested already.

> I think we do some of that
> already.  Another one I can think of is:
> 
> 1) Open cache 'foo'
> 2) Put a response with a long body in the Cache
> 3) Match the response back out of the Cache
> 4) caches.delete('foo')
> 5) drop ref to the Cache object and force GC
> 6) Call response.text() to see if we can still read the stream

We do test something along these lines in test_cache_match/matchAll_request.js.  Unfortunately SpecialPowers is unavailable on workers so we can't force a GC there.  But if you want I can do that ony on the main thread.

> Also, now that we're pref'd on we could do wpt tests. :-)

Yeah, I think jgraham is working on that.  But we can't check those tests in until we're preffed on for release, otherwise an uplift into beta will turn the tests orange.

> I'm also ok just declaring this closed now.  I think we have pretty good
> coverage.

Let's do that, since this bug has served its tracking purpose now.  :)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.