[cookie-store] Promote stability in test results #16415
Conversation
Allow service worker activation to proceed in the absence of the API under test. This prevents the workers in failing implementations from entering the "redundant" state, which can interfere with the harness's ability to detect all tests.
| { name: 'cookie-prefix', matchType: 'starts-with' }, | ||
| ]); | ||
|
|
||
| // If the worker enters the "redundant" state, the UA may terminate it |
There was a problem hiding this comment.
nit: should probably be indented 2 more spaces
| await cookieStore.subscribeToChanges([ | ||
| { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]); | ||
|
|
||
| // If the worker enters the "redundant" state, the UA may terminate it |
There was a problem hiding this comment.
nit: should probably be indented 2 more spaces
| try { | ||
| await cookieStore.subscribeToChanges([]); | ||
|
|
||
| // If the worker enters the "redundant" state, the UA may terminate it |
There was a problem hiding this comment.
nit: should probably be indented 2 more spaces
| await cookieStore.subscribeToChanges([ | ||
| { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]); | ||
|
|
||
| // If the worker enters the "redundant" state, the UA may terminate it |
There was a problem hiding this comment.
nit: should probably be indented 2 more spaces
| await cookieStore.subscribeToChanges([ | ||
| { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]); | ||
|
|
||
| // If the worker enters the "redundant" state, the UA may terminate it |
There was a problem hiding this comment.
nit: should probably be indented 2 more spaces
| @@ -6,8 +6,14 @@ importScripts("/resources/testharness.js"); | |||
|
|
|||
| self.addEventListener('install', (event) => { | |||
| event.waitUntil((async () => { | |||
| await cookieStore.subscribeToChanges([ | |||
| { name: 'cookie-name', matchType: 'equals', url: '/scope/path' }]); | |||
| try{ | |||
There was a problem hiding this comment.
nit: space between try and {
|
Thanks for the review, @inexorabletash. Here's an initial idea for addressing this in the harness. It triggers a harness error if the worker enters the redundant state. That makes the problem more clear, but it still requires timing out, and it's still susceptible to instability. I think the true solution will require a more involved patch which is not something I'd like to embark on without support from the service workers experts. |
+1; moar qualified eyeballs needed. |
* [cookie-store] Promote stability in test results Allow service worker activation to proceed in the absence of the API under test. This prevents the workers in failing implementations from entering the "redundant" state, which can interfere with the harness's ability to detect all tests. * fixup! [cookie-store] Promote stability in test results * fixup! [cookie-store] Promote stability in test results
Allow service worker activation to proceed in the absence of the API
under test. This prevents the workers in failing implementations from
entering the "redundant" state, which can interfere with the harness's
ability to detect all tests.
I've observed inconsistent test results from these tests in Safari Technology
Preview 80. This change stabilizes the results and also makes the tests fail
immediately (instead of timing out).
In general, it seems preferable to defer all fallible logic until after the
service worker's been activated. I'm not familiar with the cookie-store draft,
so I've assumed that these operations have to be done during installation.
For cases like that, it might be nice if the harness could be more resilient.
For instance, it might react to the "redundant" state by discarding all tests
(given that it's suspect whether all tests have been transmitted back to the
client) and reporting the "ERROR" status. However, I imagine doing so would
interfere with the service-workers tests that concern redundant workers.
@mattto / @mkruisselbrink do you have any thoughts about that?