Skip to content

fix: Replaced os.MkdirTemp with t.TempDir#15860

Merged
bwplotka merged 2 commits intoprometheus:mainfrom
suryaaprakassh:IssueFix
Jan 31, 2025
Merged

fix: Replaced os.MkdirTemp with t.TempDir#15860
bwplotka merged 2 commits intoprometheus:mainfrom
suryaaprakassh:IssueFix

Conversation

@suryaaprakassh
Copy link
Copy Markdown
Contributor

@suryaaprakassh suryaaprakassh commented Jan 23, 2025

Fixes #9594

  • Replace os.MkdirTemp with t.TempDir.
  • Removed the cleanup logic for os.MkdirTemp.

@suryaaprakassh
fix: os.MkdirTemp with t.TempDir
6bda88b 
Signed-off-by: Surya Prakash <surya0prakash@proton.me>
@bwplotka
Copy link
Copy Markdown
Member

bwplotka commented Jan 27, 2025

I see more places than just this one.

Do you mind fixing them all while you are on it? (:

image

@suryaaprakassh
Copy link
Copy Markdown
Contributor Author

suryaaprakassh commented Jan 27, 2025

I see more places than just this one.

Do you mind fixing them all while you are on it? (:

image

Sure I will fix it 👍

@suryaaprakassh suryaaprakassh marked this pull request as draft January 30, 2025 02:41
dgl
dgl reviewed Jan 30, 2025
View reviewed changes
@suryaaprakassh suryaaprakassh force-pushed the IssueFix branch 2 times, most recently from c93938b to ea5078d Compare January 31, 2025 02:35
@suryaaprakassh
Copy link
Copy Markdown
Contributor Author

suryaaprakassh commented Jan 31, 2025

The current changes involve modifying every os.MkdirTemp() call to t.TempDir();. However, some of these os.MkdirTemp() calls are not located within test environments. Additionally, os.MkdirTemp() in api_test.go cannot be modified, as previously mentioned by @dgl.

@suryaaprakassh suryaaprakassh marked this pull request as ready for review January 31, 2025 03:04
bwplotka
bwplotka approved these changes Jan 31, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

@bwplotka bwplotka merged commit cb3b17a into prometheus:main Jan 31, 2025
63 checks passed
@suryaaprakassh suryaaprakassh deleted the IssueFix branch February 3, 2025 05:18
@charleskorn charleskorn mentioned this pull request Mar 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dgl dgl dgl left review comments

@bwplotka bwplotka bwplotka approved these changes

@jesusvazquez jesusvazquez Awaiting requested review from jesusvazquez jesusvazquez is a code owner

@roidelapluie roidelapluie Awaiting requested review from roidelapluie

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use testing.T.TempDir() instead of ioutil.TempDir() in unit tests

3 participants

@suryaaprakassh @bwplotka @dgl