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

WPT: Upstream and add some transaction scheduling tests #22027

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Feb 29, 2020

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}

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.

In service of w3c/IndexedDB#253 move some
transaction scheduling tests from Blink to WPT.

This involved converting them from js-test.js to testharness.js, but
the overall logic of each test was retained.

This also adds one new test which verifies the change described
in w3c/IndexedDB#319 - all browsers implicitly
block R-O transactions behind overlapping R/W transactions.

Change-Id: I596aaa75b79bf3bf3e17a2553abb4e11329d59ab
Bug: 921193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081237
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746553}
@inexorabletash
Copy link
Contributor

wpt-firefox-nightly-stability

/IndexedDB/transaction-lifetime-blocked.htm
Blocked event | FAIL: 4/10, PASS: 6/10 | assert_unreached: open.error (VersionError) Reached unreachable code

/IndexedDB/transaction-lifetime.htm
Test events opening a second database when one connection is open already | FAIL: 5/10, PASS: 5/10 | assert_unreached: open.error (VersionError) Reached unreachable code

Pre-existing?

@inexorabletash
Copy link
Contributor

I can repro the failures of those tests in FF locally by reloading the files a few times.

@inexorabletash
Copy link
Contributor

FWIW, both of those tests will fail if another test leaves a database named 'db' with a version higher than 3; it doesn't proactively delete the database before trying to use it.

Tests that use 'db' as a name look like they all have add_completion_callback() handlers which attempt to delete the database. Maybe if FF doesn't let that run and/or aborts the delete if the page unloads it would be left behind.

A good fix would be to use a unique database name per test, as is done in most places.

@inexorabletash
Copy link
Contributor

Putting up a Chrome CL to address that flakiness. https://chromium-review.googlesource.com/c/chromium/src/+/2086452/

@stephenmcgruer
Copy link
Contributor

Thanks for proactively looking at this! Since there's a fix in flight, I'm going to admin-merge this.

@stephenmcgruer stephenmcgruer merged commit 5e1160e into master Mar 4, 2020
@stephenmcgruer stephenmcgruer deleted the chromium-export-cl-2081237 branch March 4, 2020 17:21
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.

None yet

4 participants