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

core: add clearStorageTypes option #15508

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Conversation

vegerot
Copy link
Contributor

@vegerot vegerot commented Oct 3, 2023

Summary
This commit adds a new option to the config to allow users to specify which storage types to clear when resetting storage for a URL. This is useful for testing scenarios where you want to clear all storage except for a specific type.

Test plan:

  • Added a few unit tests to make sure the option works as expected

Related Issues/PRs

closes #15506

@vegerot vegerot requested a review from a team as a code owner October 3, 2023 00:55
@vegerot vegerot requested review from brendankenny and removed request for a team October 3, 2023 00:55
@vercel
Copy link

vercel bot commented Oct 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lighthouse ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2023 0:56am

@vegerot
Copy link
Contributor Author

vegerot commented Oct 3, 2023

question for reviewer: I don't like the name resetOriginStorage. Can we think of a better name please?

core/gather/driver/storage.js Outdated Show resolved Hide resolved
types/lhr/settings.d.ts Outdated Show resolved Hide resolved
Copy link
Member

@adamraine adamraine left a comment

Choose a reason for hiding this comment

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

This generally looks good! Might be some linting and other minor changes necessary for CI

core/config/constants.js Outdated Show resolved Hide resolved
types/lhr/settings.d.ts Outdated Show resolved Hide resolved
types/lhr/settings.d.ts Outdated Show resolved Hide resolved
@adamraine
Copy link
Member

adamraine commented Oct 19, 2023

You may need to run the following command to fix CI: yarn update:sample-json

The unit test failures appear to be unrelated which I'll work on separately (#15553).

Copy link
Member

@adamraine adamraine left a comment

Choose a reason for hiding this comment

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

Last couple nits but LGTM! Thanks for the contribution!

types/lhr/settings.d.ts Outdated Show resolved Hide resolved
types/lhr/settings.d.ts Outdated Show resolved Hide resolved
Summary:
This adds a new option to the config to allow users to specify which
storage types to clear when resetting storage for a URL. This is
useful for testing scenarios where you want to clear all storage
except for a specific type.

Test plan:
- Added a few unit tests to make sure the option works as expected
@adamraine adamraine changed the title core(gather): add clearStorageTypes option core: add clearStorageTypes option Oct 24, 2023
@adamraine adamraine merged commit c64437c into GoogleChrome:main Oct 24, 2023
26 checks passed
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.

Feature request: Add clearAllStorage option.
5 participants