Hi Evan,

Thanks for looking into this, and for answering some of my questions
off-list. To summarize, eviction is spec'd very loosely and a testdriver.js
API could probably only be "evict everything" or "evict data in this
bucket". Neither of those would test the interesting parts of how quota,
expiry time and probably other signals are used to determine which buckets
to evict. Bottom line, if we can't see a WebDriver endpoint that would poke
at the right internals *in all browsers*, then it wouldn't be testing
production code beyond what is already tested.

It looks like get or expire a bucket
<https://wicg.github.io/storage-buckets/#get-or-expire-a-bucket> could be
tested using bucket.setExpires(), but AFAICT this isn't tested.

So, LGTM1 to ship with added tests for bucket expiration.

Best regards,
Philip

P.S. I disagree that testdriver.js should only be used in exceptional
cases. Rather often there's a spec concept which is invoked by the user or
under implementation-defined conditions, and it's straightforward to poke
this concept with a WebDriver endpoint.
https://fedidcg.github.io/FedCM/#automation is a good recent example. API
owners don't insist on this currently mainly because testdriver.js needs to
be implemented for both Chrome and content_shell. With wptrunner in
Chromium CI (thanks @Weizhong Xia <weizh...@google.com>!) I expect the need
for double work will be removed, and I would like to raise the bar for test
automation. (But even then there will be cases where it wouldn't be testing
additional production code and isn't worthwhile.)

On Thu, Dec 7, 2023 at 6:43 PM Evan Stade <est...@chromium.org> wrote:

> Hi Philip,
>
> After conferring with the team, we feel that there is an appropriate
> distribution of coverage provided by WPT and Chromium unit and browser
> tests. Leveraging testdriver for one-off hooks such as this would introduce
> substantial test-only code flows, diminishing the confidence that the tests
> were correctly verifying production behavior, while also imposing a larger
> maintenance burden going forward. The benefit, as we understand it, would
> be that the WPT could be shared by other vendors, but the test itself in
> this case would only be a few lines. The vast majority of LOC would
> be plumbing and internals logic which other vendors would need to provide
> for themselves.
>
> Getting a bit off in the weeds relative to this i2s discussion, but I'll
> continue on this topic for the sake of debate:
>
> We can observe that new testdriver APIs are seldom added. I only see ~4
> changes originating from the Chromium project in the last 5 years. Putting
> all other reasoning aside, this fact alone would suggest that testdriver
> should be touched only in exceptional cases. I would not argue that
> testdriver is without utility. Unlike the one-off actions we'd need for
> these buckets WPT, more generic testdriver actions such as setting
> permission state can enable a wide range of tests. In those cases the
> calculus of cost vs reward is much more favorable. So I would assume that
> changes to testdriver should meet a high bar of broad utility, similar to
> how we treat core libraries in //base/.
>
> Hope that addresses your concerns.
>
> -- Evan Stade
>
>
> On Fri, Dec 1, 2023 at 9:03 AM Philip Jägenstedt <foo...@chromium.org>
> wrote:
>
>> Thanks Evan for listing out the tests, they were more spread out than I
>> thought. Great to see more of the tests being upstreamed too!
>>
>> I wonder if we can do even better and test eviction however? Have you
>> looked into defining a WebDriver endpoint for triggering eviction? It could
>> make sense in
>> https://web-platform-tests.org/writing-tests/testdriver.html#storage and
>> there are plenty of examples in that documentation that might be close to
>> the hook/trigger you need.
>>
>> If you think this is an unreasonable request, please push back, my
>> interest is only that this can be implemented interoperably. Tests are our
>> best way of ensuring that, but it's not the only way, and it's always a
>> tradeoff of effort vs. utility.
>>
>> Best regards,
>> Philip
>>
>> On Wed, Nov 29, 2023 at 6:32 PM Evan Stade <est...@chromium.org> wrote:
>>
>>> Hi Philip,
>>>
>>> Here is a comprehensive list of web tests that verify storageBuckets
>>> behavior. We do feel the size of this list aligns well with the spec,
>>> which is fairly concise. As you can see, some are spread out in directories
>>> for the other storage APIs that storageBuckets brokers.
>>>
>>> That said, not all behavior is verifiable from the web. For example,
>>> it's not possible to reliably trigger an automatic eviction event from the
>>> web, so we can't really know if `persisted` is *actually* respected.
>>> Browser tests provide that kind of coverage.
>>>
>>> Some of the WPT are internal but they can/should be moved
>>> <https://chromium-review.googlesource.com/c/chromium/src/+/5072535> to
>>> the external WPT directory. Thanks for the prompt.
>>>
>>> external/wpt/IndexedDB/storage-buckets.https.any.js:
>>> external/wpt/fs/script-tests/FileSystemBaseHandle-buckets.js:
>>>
>>> external/wpt/service-workers/cache-storage/cache-storage-buckets.https.any.js:
>>>
>>> external/wpt/storage/buckets/bucket-quota-indexeddb.tentative.https.any.js:
>>>
>>> external/wpt/storage/buckets/bucket-storage-policy.tentative.https.any.js:
>>> external/wpt/web-locks/storage-buckets.tentative.https.any.js:
>>>
>>> http/tests/inspector-protocol/storage/cache-storage-request-cache-names.js:
>>>
>>> http/tests/inspector-protocol/storage/cache-storage-storage-key-request-cache-names.js:
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-clear.js:
>>>
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-delete-db.js:
>>>
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-delete-object-store-entries.js:
>>>
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-get-metadata.js:
>>>
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-request-data.js:
>>>
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-request-database-names.js:
>>>
>>> http/tests/inspector-protocol/storage/indexed-db-storage-key-request-database.js:
>>>
>>> http/tests/inspector-protocol/storage/storage-bucket-storage-key-track-untrack.js:
>>>
>>> http/tests/inspector-protocol/storage/storage-buckets-delete-storage-bucket.js:
>>> wpt_internal/storage/buckets/bucket_names.tentative.https.any.js:
>>> wpt_internal/storage/buckets/buckets_basic.tentative.https.any.js:
>>>
>>> wpt_internal/storage/buckets/buckets_storage_policy.tentative.https.any.js:
>>> wpt_internal/storage/buckets/detached-iframe.https.html:
>>> wpt_internal/storage/buckets/idlharness-worker.https.any.js:
>>> wpt_internal/storage/buckets/opaque-origin.https.window.js:
>>> wpt_internal/storage/buckets/resources/opaque-origin-sandbox.html:
>>>
>>> wpt_internal/storage/buckets/storage_bucket_object.tentative.https.any.js:
>>>
>>> -- Evan Stade
>>>
>>>
>>> On Wed, Nov 29, 2023 at 9:02 AM Philip Jägenstedt <foo...@chromium.org>
>>> wrote:
>>>
>>>> Hi Evan,
>>>>
>>>> Is
>>>> https://wpt.fyi/results/storage/buckets?label=experimental&label=master&aligned
>>>> the full set of tests? Given the size of the spec, does that cover the
>>>> whole feature well?
>>>>
>>>> Best regards,
>>>> Philip
>>>>
>>>> On Wed, Nov 29, 2023 at 5:58 PM Evan Stade <est...@chromium.org> wrote:
>>>>
>>>>> [reply-all this time]
>>>>>
>>>>> Hi Philip,
>>>>>
>>>>> thanks for pointing out those two oversights. I have fixed the
>>>>> checkboxes on the chromestatus entry. It is in fact tested by WPT and
>>>>> supported on all Blink platforms.
>>>>>
>>>>> On Wed, Nov 29, 2023 at 8:43 AM Philip Jägenstedt <foo...@chromium.org>
>>>>> wrote:
>>>>>
>>>>>> Hi Evan,
>>>>>>
>>>>>> A few questions inline.
>>>>>>
>>>>>> On Tue, Nov 14, 2023 at 9:38 PM Evan Stade <est...@chromium.org>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> Will this feature be supported on all six Blink platforms (Windows,
>>>>>>> Mac, Linux, Chrome OS, Android, and Android WebView)?
>>>>>>>
>>>>>>> No
>>>>>>>
>>>>>>
>>>>>> Which platform will this not be supported on?
>>>>>>
>>>>>>
>>>>>>> Is this feature fully tested by web-platform-tests
>>>>>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md>
>>>>>>> ?
>>>>>>>
>>>>>>> No
>>>>>>>
>>>>>>
>>>>>> Can this be fully tested in WPT? If it's not tested in WPT, how is it
>>>>>> currently tested?
>>>>>>
>>>>>> Best regards,
>>>>>> Philip
>>>>>>
>>>>>

-- 
You received this message because you are subscribed to the Google Groups 
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYfLUBkQCsJG4RYHSUqxLkN42NdMTRWAK817hofLU6CrPA%40mail.gmail.com.

Reply via email to