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

Use tempdir when testing static files, so temporary directories get dropped automatically #6824

Closed
shekhirin opened this issue Feb 27, 2024 · 4 comments · Fixed by #6962
Closed
Assignees
Labels
A-static-files Related to static files C-test A change that impacts how or what we test D-good-first-issue Nice and easy! A great choice to get started

Comments

@shekhirin
Copy link
Collaborator

shekhirin commented Feb 27, 2024

Describe the feature

// TODO: replace with `tempdir` usage, so the temp directory is removed
// automatically when the variable goes out of scope
reth_primitives::fs::remove_dir_all(static_files_dir)
.expect("Failed to remove static files directory");

Here and in other places where create_test_static_files_dir is used, we need to return both a guard (TempDir) and an actual path to the directory, so the guard gets dropped when goes out of scope and the directory gets deleted.

Additional context

Blocked by #6444

@shekhirin shekhirin added D-good-first-issue Nice and easy! A great choice to get started C-test A change that impacts how or what we test A-static-files Related to static files labels Feb 27, 2024
@AbnerZheng
Copy link
Contributor

Can I give it a try?

@shekhirin shekhirin assigned AbnerZheng and unassigned AbnerZheng Feb 27, 2024
@shekhirin
Copy link
Collaborator Author

@AbnerZheng hey, I assigned you, but we want to merge #6444 into main first, and then accept new PRs on top of these changes.

@AbnerZheng
Copy link
Contributor

NP, I can take a look first and create the PR after the PR has been merged.

@shekhirin
Copy link
Collaborator Author

hey @AbnerZheng, we just merged the PR into main! So feel free to start working on this 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-static-files Related to static files C-test A change that impacts how or what we test D-good-first-issue Nice and easy! A great choice to get started
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants