Implementation report

[LJWatson]

We'll need an implementation report to exit CR. Is the test suite complete?

[pwnall]

@LJWatson To the best of my knowledge, we have test coverage for all the methods and error cases. We even have tests for some tricky bugs that have shown up in implementations. Modulo small bugs, Chrome, Firefox and Safari pass all the tests.

Does this answer your question?
(I'm trying to fill in for @inexorabletash who is on vacation)

[LJWatson]

Thanks @pwnall. It does answer the question, yes.

It sounds like we just need to generate a report from the test results (so we have something to point to when we request transition to Proposed Recommendation). Ideally it would be good to have this by early September if we can - and I think @inexorabletash is back by then.

[inexorabletash]

I'm back!

What exactly do we need for the implementation report?

We now have a live dashboard of web-platform-test results: http://wpt.fyi/IndexedDB - it lists each test file and for each browser (chrome, edge, firefox, safari) it gives the test case pass results, for example 2/2, 1/14, etc.

It does not summarize pass rate over the whole suite per browser; we may be able to add that if necessary.

[LJWatson]

Welcome back.

The dashboard will do the trick. No need to worry about a pass rate for the whole suite/browser because the interop requirement is for each feature to have at least two independent implementations.

It looks like some features do not have 100% pass rates in at least two browsers (idb-binary-key-roundtrip.htm for example). Is that enough to put those features at risk do you think?

[inexorabletash]

It looks like some features do not have 100% pass rates in at least two browsers (idb-binary-key-roundtrip.htm for example). Is that enough to put those features at risk do you think?

The dashboard is unfortunately not yet showing results from the latest Safari; that might be one problem with pointing at the dashboard. I believe Safari 10.2 passes most of the tests. We can do a manual pass.

(idbobjectstore_createIndex15-autoincrement.htm is one test with failures on most implementations; it does not represent new "2.0" features, but rather subtle interaction of "1.0" features that was not previously tested.)

[LJWatson]

If we can get the results from Safari into the implementation report, and assuming that gives us the interop we're missing based on the current report, we'll be ready to open the CFC to ask the WG if they're ok with us progressing to PR.

[haib56]

Connect ipad

[inexorabletash]

I've recruited someone to do a pass with the latest Safari TP, and will put a report together once that's done.

[inexorabletash]

I assume following the process at https://github.com/w3c/test-results would be preferred, though?

I'm getting server errors on the console from http://w3c-test.org/tools/runner/index.html "Manifest generation failed" :(

[ylafon]

@inexorabletash can you retry now? The Manifest generation should be back to normal.

[inexorabletash]

Thanks, will do!

FYI @bsittler

[inexorabletash]

Thanks, @ylafon ! I filed a PR with the updated results:

w3c/test-results#121

Here's the "less than 2":

https://rawgit.com/inexorabletash/test-results/idb2/IndexedDB/less-than-2.html

• /IndexedDB/clone-before-keypath-eval.html - edge cases discovered during rework of algorithms
• /IndexedDB/error-attributes.html - we agreed to drop DOMError from the platform and use DOMException instead
• /IndexedDB/idb-binary-key-detached.htm - maybe detaching is not a thing any more?
• /IndexedDB/idbobjectstore_createIndex15-autoincrement.htm - a super awesome edge case discovered by the nodejs implementation
• /IndexedDB/keypath-exceptions.htm - edge cases discovered during rework of algorithms
• /IndexedDB/keypath-special-identifiers.htm - functionality mentioned explicitly in v1 but untested; this is interesting as it requires a special case in the spec
• /IndexedDB/transaction-deactivation-timing.html - inconsistencies discovered while integrating with HTML
• /IndexedDB/upgrade-transaction-lifecycle-user-aborted.html - edge cases discovered during rework of algorithms

Caveat: Safari apparently has a nasty bug triggered by the /IndexedDB/keypath-exceptions.htm test. (cc: @beidson) Any IDB test run after that (in the same origin and tab) times out! So any all of the Safari results following that one are invalid. See:
https://rawgit.com/inexorabletash/test-results/idb2/IndexedDB/all.html

(I filed https://bugs.webkit.org/show_bug.cgi?177280 for the Safari issue)

So of those, the following are intentional normative additions or behavior changes "new in 2.0":

• /IndexedDB/error-attributes.html
• /IndexedDB/idb-binary-key-detached.htm

And the rest were underspecified in v1 (e.g. does a property change before or after an event fires? are keypaths evaluated against the original object - possibly triggering side effects - or a clone?)

[inexorabletash]

Manually testing in Safari TP, the last three tests would not trip the "less than 2 passes" mark.

• /IndexedDB/keypath-special-identifiers.htm - has two cases that fail in Safari TP, but they don't overlap the failing case in Firefox or Edge
• /IndexedDB/transaction-deactivation-timing.html - passes in Safari TP
• /IndexedDB/upgrade-transaction-lifecycle-user-aborted.html - passes in Safari TP

[siusin]

I can help to generate results in Safari TP for the manual tests.

[inexorabletash]

Thanks, @siusin ! How do you want to coordinate - should I look at merging the PR as-is so you can follow up, or do you want to take it over, or ... ?

[siusin]

@inexorabletash if you have the test result of Edge 15, could you upload the json file to the test-results repo? Thanks.

According to the test results in Ff58, Ch63 and Sf11, and the information at wpt.fyi, I believe there are only 5 less-than-2 cases:

• /IndexedDB/clone-before-keypath-eval.html (4/5, 80.00%, 0.32% of total)
• /IndexedDB/error-attributes.html (1/1, 100.00%, 0.08% of total)
• /IndexedDB/idb-binary-key-detached.htm (2/2, 100.00%, 0.16% of total)
• /IndexedDB/idbobjectstore_createIndex15-autoincrement.htm (1/2, 50.00%, 0.08% of total)
• /IndexedDB/keypath-exceptions.htm (3/6, 50.00%, 0.24% of total)

Any objection to ignore these edge cases and move to PR/REC?

I'll try to find some time next week to review the test coverage of the substantive changes introduced by Level 2.

[inexorabletash]

@siusin - done! (hopefully done correctly)

• Do we want to keep the results from older browser version, or are those just noise?
• I don't know why the interfaces.html results are showing up as failed (missing) in the "less-than-2" summary.

I'm fine with ignoring, but I'm biased. :) Seriously though... error-attributes.html is the only normative change where only one browser has so far adopted it.

[siusin]

@inexorabletash superb!

We usually only put the results of the latest version into the implementation report, but it doesn't hurt to keep the .json files in the repo as historical records.

I'm hoping we can move to PR with error-attributes.html as an exception... unless we find any missing test, I'll send out the transition request this week.

Btw, do you plan to continue the editing of IndexedDB after 2.0 exits CR? We are using a script to auto-generate the .html doc from .bs files and push it to gh-pages for the specs using Bikeshed these days, see the IntersectionObserver repo as an example. Would you like us to set up this workflow in the IndexedDB repo too?

[inexorabletash]

@siusin Yes I plan to continue editing, and yes please!

[siusin]

@xfq do you mind if we assign this issue to you?

1. review the test coverage of the L2 normative changes;
2. set up the auto-pub workflow for the repo.

[xfq]

@siusin Fine with me.

[xfq]

The auto-deployment of index.bs on w3c.github.io is ready now.

Sorry for messing up the Git history. I set it up in my own repo in one commit (the other commit is for testing), but for some reason (my mistakes) I need to commit several changes when setting up auto-deployment in the W3C repo.

[LJWatson]

Can we now tell whether the spec meets the interop requirement (at least two independent implementations for each feature)?

[xfq]

Hi all,

Here's my test coverage review of the level 2 changes. Because I'm not familiar with either Indexed DB or web-platform-tests, I might be missing something. Comments from @inexorabletash / @siusin are very appreciated!

TIA

---

• DONE Address comparison of empty arrays. (bug #27712)
  • https://www.w3.org/2017/10/idbfactory_cmp_empty_arrays.html

---

• DONE Correct source used for get(), getKey() and openKeyCursor() on IDBIndex.
  • *-request-source.html

---

• DONE Added details around garbage collection of IDBDatabase objects. (bug #25223)
  • Not testable

---

• DONE Added [Exposed=(Window,Worker)] annotations to interfaces.
  • interfaces.worker.js

---

• DONE Added forced flag to the steps to close a database connection, described the firing of a "close" event, and onclose. (bug #22540)
  • Not testable - depends on implementation encountering an error or implementation-specific user action (e.g., clearing browsing data)

---

• DONE Converted specification to a more algorithmic style, and define abstract types such as key more rigorously. (bug #17681)
  • Not testable

---

• DONE Added getAll() and getAllKeys() on IDBObjectStore, and getAll() and getAllKeys() on IDBIndex. (bug #16595)
  • idbobjectstore_getAll.html
  • idbobjectstore_getAllKeys.html
  • idbindex_getAll.html
  • idbindex_getAllKeys.html
  • idbobjectstore-query-exception-order.html
  • idbindex-query-exception-order.html

---

• DONE Replaced DOMError with DOMException. (bug Replace DOMError with DOMException #16)
  • error-attributes.html

---

• DONE Added objectStoreNames on IDBTransaction. (bug Add scope attribute to IDBTransaction #18)
  • idbtransaction_objectStoreNames.html

---

• DONE Added binary keys, including comparisons and ECMAScript bindings. (bug Support binary keys #21)
  • idb-binary-key-roundtrip.htm
  • idb_binary_key_conversion.htm
  • idbfactory_cmp3.htm
  • idbfactory_cmp4.htm

---

• DONE Allow renaming stores and indexes via IDBObjectStore's name and IDBIndex's name attribute setters. (bug Allow renaming object stores and indexes #22)
  • IndexedDB renaming and transaction abort tests. web-platform-tests/wpt#3854

---

• DONE Added continuePrimaryKey() on IDBCursor. (bug Add openCursor(key, primaryKey) and continuePrimaryKey #14)
  • idbcursor-continuePrimaryKey*.htm

---

• DONE Added includes() on IDBKeyRange. (bug Add IDBKeyRange.prototype.includes(key) #41)
  • idbkeyrange-includes.htm

---

• DONE Added getKey() on IDBObjectStore. (bug Add getKey to IDBObjectStore #26)
  • idbobjectstore_getKey.html

---

• DONE Clarified when a transaction can attempt to commit. (bug Enforce transaction commit ordering more strictly #77)
  • IndexedDB: Add test for commit order of empty transactions web-platform-tests/wpt#3154

---

• DONE Clarified open request / connection queue processing. (bug task behavior for open requests is poorly specified #9, bug deleteDatabase's "versionchange" should be deferred if upgrade is running #78, bug Specify IDBOpenDBRequest processing as a queue? #79, bug The steps for opening a database are racy #81)
  • open-request-queue.html
  • delete-request-queue.html

---

• DONE Ensure event firing is done in the context of queued tasks. (bug Asynchronously executing a request should queue a task to fire events #83)
  • Implicitly tested throughout, the spec was just not precise here.

---

• DONE Define precedence for exceptions when multiple error conditions apply. (bug Validate exception ordering against implementations #11)
  • idbdatabase-transaction-exception-order.html

---

• DONE Remove IDBEnvironment; expose globals with partial interface instead. (bug IDBEnvironment IDL is not valid #94)
  • Not testable - this is just a change in IDL style, not web-observable.

---

• DONE Clarify when deleteDatabase() can fail. (bug deleteDatabase error case should never happen #74)
  • Not testable. It would require the implementation to hit an internal I/O error.

---

• DONE Add non-normative documentation for every method. (bug Add non-normative docs for APIs #110)
  • Not testable

---

• DONE Throw SecurityError if open() or deleteDatabase() is called from an opaque origin. (bug specify what should happen if the origin of execution context is opaque #148)
  • IDBFactory methods and opaque origins web-platform-tests/wpt#4829

---

• DONE Integrate with legacyOutputDidListenersThrowFlag hook in [DOM], replacing monkey patching. (bug "If an exception was propagated out from any event handler" is not a thing #140)
  • IndexedDB: Add tests for HTML monkey patches web-platform-tests/wpt#4643

---

• DONE Define cleanup Indexed Database transactions hook for [HTML], replacing monkey patching. (bug Specify timing of transaction deactivation more precisely #87)
  • IndexedDB: Add tests for HTML monkey patches web-platform-tests/wpt#4643

---

• DONE Fix handling of edge cases in key generation algorithm. (bug Key generator behavior with explicit keys above 2^53 #147)
  • Indexed DB: Ensure large explicit keys consistently max out generator  web-platform-tests/wpt#5065

---

• DONE Use [HTML]'s StructuredSerialize and StructuredDeserialize hooks. (bug Update for structured cloning changes in HTML #170)
  • Not testable

---

• DONE Use [SameObject] / [NewObject] in IDL where appropriate. (issue Apply [NewObject] IDL extended attribute to relevant methods #193, issue Apply [SameObject] IDL extended attribute to relevant attributes #194)
  • *-SameObject.html

---

• DONE Transaction-is-active test can be an assertion in the asynchronously execute a request steps. (issue create/deleteObjectStore: InvalidStateError logic does not match impls/tests #192)
  • idbdatabase-createObjectStore-exception-order.htm
  • idbdatabase-deleteObjectStore-exception-order.htm

---

• DONE Use [HTML]'s StructuredSerializeForStorage hook. (issue Use StructuredSerializeForStorage #197, issue SharedArrayBuffer integration #152)
  • WIP: SharedArrayBuffer structured cloning tests web-platform-tests/wpt#5003

---

• DONE Define database's associated upgrade transaction to align exceptions thrown from createObjectStore() and deleteObjectStore() with tests and implementations. (issue create/deleteObjectStore: InvalidStateError logic does not match impls/tests #192)
  • idbdatabase-createObjectStore-exception-order.htm
  • idbdatabase-deleteObjectStore-exception-order.htm

---

• DONE Set [=request/result]/done flag appropriately for requests from open() and deleteDatabase(). (issue The done flag is never unset between upgradeneeded and success #161)
  • IndexedDB: Add tests for request properties during/after open/delete web-platform-tests/wpt#5899

[pwnall]

Renaming is testable, and the test landed in web-platform-tests/wpt#3854

[xfq]

@pwnall Thanks! I just edited my comment.

[inexorabletash]

Wow, thanks for this awesome analysis! First pass of responses - I need to dig into the tests for the rest.

Added details around garbage collection of IDBDatabase objects. (bug #25223)
Not testable?

Not testable. :(

Added [Exposed=(Window,Worker)] annotations to interfaces.
Not testable? Or interfaces.{html,idl,worker.js}?

Tested by interfaces.worker.js

Added forced flag to the steps to close a database connection, described the firing of a "close" event, and onclose. (bug #22540)
Not testable?

Not testable, unfortunately - depends on implementation encountering an error or implementation-specific user action (E.g. clearing browsing data)

Converted specification to a more algorithmic style, and define abstract types such as key more rigorously. (bug #17681)
Not testable?

Not directly testable, although many edge cases discovered while doing this were covered with new tests.

Clarified open request / connection queue processing. (bug #9, bug #78, bug #79, bug #81)
Not testable?
Or maybe idbfactory-open-request-success.html, idbfactory-open-request-error.html, and idbrequest-onupgradeneeded.htm?

open-request-queue.html, delete-request-queue.html

Ensure event firing is done in the context of queued tasks. (bug #83)
Not testable?

Implicitly tested throughout, the spec was just not precise here.

Remove IDBEnvironment; expose globals with partial interface instead. (bug #94)
Not testable?

Not testable - this is just a change in IDL style, not web-observable.

Clarify when deleteDatabase() can fail. (bug #74)
Not testable, perhaps? How to test whether it returns "an appropriate error"?

Not testable, unfortunately. It would require the implementation to hit an internal I/O error.

Use [HTML]'s StructuredSerialize and StructuredDeserialize hooks. (bug #170)
Not testable?

Not testable.

[inexorabletash]

Correct source used for get(), getKey() and openKeyCursor() on IDBIndex.
Not testable?

This is testable; I do not see coverage for it. Behavior is not new here, it was just a spec copy/pasta. But we should still have a test!

Use [SameObject] / [NewObject] in IDL where appropriate. (issue #193, issue #194)
Not testable? Or interfaces.{html,idl,worker.js}?

The [SameObject] assertions should be testable. I see a couple cases covered but not all of them. Behavior here is not new, but we should still have tests!

I think that's it. I'll put a CL shortly with tests for these cases.

[xfq]

Thank you very much, @inexorabletash! I edited my comment according to your comments.

[inexorabletash]

Okie dokey:

Correct source used for get(), getKey() and openKeyCursor() on IDBIndex.

Now covered by *-request-source.html

Use [SameObject] / [NewObject] in IDL where appropriate. (issue #193, issue #194)

Now covered by *-SameObject.html

That should be everything?

[siusin]

@inexorabletash ++
@xfq ++
\o/

Yep, we can start the PR transition now. Thanks for your good work, as always.

[LJWatson]

The request to transition to PR has been accepted (publication will happen after the TPAC moratorium). Well done everyone!