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

[invidian]

Proposal

Use case. Why is this important?

Instead of writing the code below in many many unit tests:

      dir, err := ioutil.TempDir("", "wal_fuzz_live")
      require.NoError(t, err)
      defer func() {
        require.NoError(t, os.RemoveAll(dir))
      }()

Write this instead:

    dir := t.TempDir()

Advantages:

• Error handling is implicit.
• Cleanup is implicit.
• Naming of directory derives from name of test, again, implicit.

Current occurences:

0 ✓ (40.6ms) 11:11:22 invidian@dellxps15mateusz ~/repos/prometheus (⎇  main)$ git show --no-patch
commit a6e6011d55ed913cb8e53968cead9a6a6fd84911 (HEAD -> main, upstream/main, upstream/HEAD)
Author: Furkan Türkal <[email protected]>
Date:   Mon Oct 25 00:45:31 2021 +0300

    Add scrape_body_size_bytes metric (#9569)

    Fixes #9520

    Signed-off-by: Furkan <[email protected]>
0 ✓ (37.3ms) 11:11:29 invidian@dellxps15mateusz ~/repos/prometheus (⎇  main)$ git grep ioutil.TempDir *_test.go | wc -l
129

[roidelapluie]

:+1:

[praveenghuge]

Hi @invidian i would like to work on this

[invidian]

Sure. Just be aware that t.TempDir() use t.Cleanup() for triggering, which might execute at different time than currently used defer. It also always checks for errors, so you may find some tests on Windows to fail. My commits like https://github.com/prometheus/prometheus/pull/9583/commits/81aec2f2d9c8a9dbfa7dcbce05a0cffd488aa152 and https://github.com/prometheus/prometheus/pull/9583/commits/a2009e0fe54f46b4276180e5172a31647f6e20de tries to address it in #9583, so feel free to cherry-pick them if needed, so your PR passes the CI!

[roidelapluie]

Please do this in multiple pull requests as well, not a big pull request. There are a lot of occurences.

[palash25]

i was looking through this wanting to contribute but I didn't find any ioutil.TempDir calls in any of the test files, i think this can be closed

[invidian]

I guess this is because ioutil.TempDir got deprecated and is now replaced by os.MkdirTemp:

$ git describe --always
v0.39.1-270-g6dd4e907a
$ git grep os.MkdirTemp
cmd/promtool/tsdb.go:           dir, err := os.MkdirTemp("", "tsdb_bench")
promql/engine_test.go:  dir, err := os.MkdirTemp("", "test_concurrency")
tsdb/blockwriter.go:    chunkDir, err := os.MkdirTemp(os.TempDir(), "head")
tsdb/example_test.go:   dir, err := os.MkdirTemp("", "tsdb-test")
tsdb/tsdbutil/dir_locker_testutil.go:                   tmpdir, err := os.MkdirTemp("", "test")
util/teststorage/storage.go:    dir, err := os.MkdirTemp("", "test_storage")
util/testutil/directory.go:     directory, err = os.MkdirTemp(defaultDirectory, name)
web/api/v1/api_test.go: dbDir, err := os.MkdirTemp("", "tsdb-api-ready")

[palash25]

thanks i changed one occurrence of os.MkdirTemp in engine_test.go the other two test files that had os.MkdirTemp could not be changed because both of the calls were outside test functions

[GuillermoMajano]

this is close?

[invidian]

Doesn't seem like it. Example: https://github.com/prometheus/prometheus/blob/6e2905a4d4ff9b47b1f6d201333f5bd53633f921/web/api/v1/api_test.go#L2567

[beorn7]

There is #9641 , which appears to be abandoned. If anyone wants to pick up that work, that would be great.

[milencium]

Heey @invidian and @beorn7 , I would love to be assigned on this task if possible, thank you guys in advance

[beorn7]

@milencium Please, go ahead. PRs welcome.

[milencium]

@beorn7, thanks for this amazing opportunity, will do !

[miledxz]

@beorn7 , I will proceed as you suggested

[bboreham]

Reviewed at the bug-scrub. This issue seems still valid; ioutil.TempDir has been replaced by os.MkdirTemp, and there are still a handful of calls to that in tests.

[miledxz]

@bboreham I remember this breaking ci cd after my pr, maybe my pr was not correct but file system is not same on windows and linux and some tests becomes flaky

[bboreham]

Thanks for the reminder. Trail at #13344