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

Fix to remove browser downloads path #4288

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

inancgumus
Copy link
Member

What?

Each BrowserContext has a download path. We now clean them up when the Browser they are associated with closes.

Why?

To avoid inflating the user's temporary directory folder.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

Sorry, something went wrong.

@inancgumus inancgumus self-assigned this Jan 27, 2025
@inancgumus inancgumus marked this pull request as ready for review January 27, 2025 23:00
@inancgumus inancgumus requested a review from a team as a code owner January 27, 2025 23:00
@inancgumus inancgumus requested review from mstoykov and oleiade and removed request for a team January 27, 2025 23:00
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM and thank you 🙇

This does get my *browser* directories in /tmp from 866 to 33.

The remaining ones are

  • xk6-browser-data*
  • k6browser-artifacts*

Which seem to be completely different parts, although we likely should at least remove the xk6- part 😅

But we can do in a separate PR.

Each BrowserContext has a downloads path. We now clean them up when the
Browser they are associated with closes.
@inancgumus inancgumus force-pushed the fix/browser-downloadspath-removal branch from 3305ce4 to 1fa34e6 Compare January 28, 2025 14:22
@inancgumus inancgumus merged commit cf3aa58 into master Jan 28, 2025
53 of 55 checks passed
@inancgumus inancgumus deleted the fix/browser-downloadspath-removal branch January 28, 2025 15:01
@inancgumus inancgumus mentioned this pull request Jan 28, 2025
5 tasks
@inancgumus
Copy link
Member Author

inancgumus commented Jan 28, 2025

@mstoykov This PR renames the xk6-browser-data directory.

@inancgumus inancgumus added this to the v0.57.0 milestone Jan 28, 2025
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

3 participants