Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deduplicate some WPT. #42700

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Oct 23, 2023

storagemanager-estimate and estimate-indexeddb are more or less the
same tests, the latter having been ported from the former to use
async/await. The former probably should have been deleted when the
latter was introduced.

Since some of the tests are related to IndexedDB and some are not, this
change keeps the IndexedDB tests in the file called estimate-indexeddb
and keeps the basic tests in storagemanager-estimate (with minor
updates).

One wrinkle from the Chromium side is that the behavior of
storageManager.estimate() is not actually specced, and as the Chromium
implementation uses LevelDB, which behaves in mysterious ways, adding
things to the database does NOT always increase the reported usage size.
Both of these tests operate on large things, which typically do increase
usage, however we noticed that for one reason or another, the test that
adds an uninitialized ArrayBuffer starts failing on Windows if
durability is set to relaxed. There are other ways to make the test fail
as well: using a shorter name for the database, or putting small
values, does not reliably increase the reported usage. This is all fine
in the sense that it isn't defined behavior, but it does suggest that
working in this area or on tests of this ilk is a bit of a minefield.
These tests probably should not exist as WPT, at least not until quota
behavior is specced (see whatwg/storage#110).
But it would also be sort of a shame to delete WPT that are passing, so
I've left them in place for now.

Bug: 1489517
Change-Id: I6619f504ce92e428054691ac6bf54a0e14e3ce5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4968659
Commit-Queue: Evan Stade <[email protected]>
Reviewed-by: Ayu Ishii <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1214324}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

storagemanager-estimate and estimate-indexeddb are more or less the
same tests, the latter having been ported from the former to use
`async`/`await`. The former probably should have been deleted when the
latter was introduced.

Since some of the tests are related to IndexedDB and some are not, this
change keeps the IndexedDB tests in the file called `estimate-indexeddb`
and keeps the basic tests in `storagemanager-estimate` (with minor
updates).

One wrinkle from the Chromium side is that the behavior of
`storageManager.estimate()` is not actually specced, and as the Chromium
implementation uses LevelDB, which behaves in mysterious ways, adding
things to the database does NOT always increase the reported usage size.
Both of these tests operate on large things, which typically do increase
usage, however we noticed that for one reason or another, the test that
adds an *uninitialized* ArrayBuffer starts failing on Windows if
durability is set to relaxed. There are other ways to make the test fail
as well: using a shorter name for the database, or putting small
values, does not reliably increase the reported usage. This is all fine
in the sense that it isn't defined behavior, but it does suggest that
working in this area or on tests of this ilk is a bit of a minefield.
These tests probably should not exist as WPT, at least not until quota
behavior is specced (see whatwg/storage#110).
But it would also be sort of a shame to delete WPT that are passing, so
I've left them in place for now.

Bug: 1489517
Change-Id: I6619f504ce92e428054691ac6bf54a0e14e3ce5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4968659
Commit-Queue: Evan Stade <[email protected]>
Reviewed-by: Ayu Ishii <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1214324}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 446b9bc into master Oct 24, 2023
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-4968659 branch October 24, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants