Thanks all! On Wed, Dec 13, 2023 at 5:10 AM Philip Jägenstedt <foo...@chromium.org> wrote:
> 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. > To write reliable tests for that, we'd need a way to manipulate the clock. Is there a way to do that from WPT? I can't seem to find it if so. > > 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/CAO4XGS8e1G_dL-2%3DPnT6pN62SuzJKPZGht%2BRjH7g2Lb0LW3jdQ%40mail.gmail.com.
