这是indexloc提供的服务，不要输入任何密码
Skip to content

Update QuotaExceededError expectations #53645

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Update QuotaExceededError expectations #53645

wants to merge 6 commits into from

Conversation

domenic
Copy link
Member

@domenic domenic commented Jul 8, 2025

@domenic domenic requested a review from a team as a code owner July 8, 2025 04:11
@domenic domenic mentioned this pull request Jul 8, 2025
Open
15 tasks
This was referenced Jul 8, 2025
Open
Open
@domenic domenic force-pushed the quotaexceedederror-upgrade branch from aa7246b to b50f688 Compare July 8, 2025 04:36
@domenic domenic mentioned this pull request Jul 8, 2025
Open
2 tasks
annevk
annevk requested changes Jul 16, 2025
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

A lot of these things appear like regressions to me. If we don't throw for the method involved the test ends up passing.

Can we instead extend assert_throws_dom and promise_rejects_dom to account for more complex exceptions?

@domenic
Copy link
Member Author

domenic commented Jul 17, 2025

Thanks for the push, I introduced new assert_throws_quotaexceedederror and promise_rejects_quotaexceedederror. I think that's necessary instead of reusing the _dom versions for (a) clarity; (b) to let us test the requested and quota properties.

annevk
annevk approved these changes Jul 17, 2025
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Looks good to me. And let's copy @jgraham in case he's interested in reviewing the testharness.js changes.

foolip
foolip approved these changes Jul 18, 2025
View reviewed changes
@domenic @foolip
Apply suggestions from code review
05ac97c 
Co-authored-by: Philip Jägenstedt <philip@foolip.org>
twiss
twiss approved these changes Jul 21, 2025
View reviewed changes
Copy link
Member

@twiss twiss left a comment

Choose a reason for hiding this comment

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

LGTM for the WebCrypto part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@foolip foolip foolip approved these changes

@twiss twiss twiss approved these changes

@annevk annevk annevk approved these changes

@joeyparrish joeyparrish joeyparrish approved these changes

@ayuishii ayuishii Awaiting requested review from ayuishii

@beverloo beverloo Awaiting requested review from beverloo

@jakearchibald jakearchibald Awaiting requested review from jakearchibald

@jdm jdm Awaiting requested review from jdm

@jgraham jgraham Awaiting requested review from jgraham

@jrummell-chromium jrummell-chromium Awaiting requested review from jrummell-chromium

@mkruisselbrink mkruisselbrink Awaiting requested review from mkruisselbrink

@mnot mnot Awaiting requested review from mnot

@Ms2ger Ms2ger Awaiting requested review from Ms2ger

@siusin siusin Awaiting requested review from siusin

@titzer titzer Awaiting requested review from titzer

@wolenetz wolenetz Awaiting requested review from wolenetz

@youennf youennf Awaiting requested review from youennf

@yuki3 yuki3 Awaiting requested review from yuki3

@yutakahirano yutakahirano Awaiting requested review from yutakahirano

Assignees

@twiss twiss

Labels
ai background-fetch encrypted-media fetch fs infra media-source storage testharness.js wasm WebCryptoAPI webidl webstorage wg-cg_wicg wg-media wg-s_htmlwg wg-webcrypto
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@domenic @foolip @twiss @annevk @joeyparrish @wpt-pr-bot