Skip to content

Commit

Permalink
[test] use T.TempDir to create temporary test directory (open-telem…
Browse files Browse the repository at this point in the history
…etry#10419)

* [test] Use `T.TempDir` to create temporary test directory

This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.

Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
	defer func() {
		if err := os.RemoveAll(dir); err != nil {
			t.Fatal(err)
		}
	}
is also tedious, but `t.TempDir` handles this for us nicely.

Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <[email protected]>

* [test] Fix failing tests on Windows

We must explicitly close the underlying BoltDB, otherwise the tests will
fail on Windows with the error "The process cannot access the file
because it is being used by another process".

Signed-off-by: Eng Zer Jun <[email protected]>
  • Loading branch information
Juneezee committed Jun 6, 2022
1 parent 5ebba60 commit 2f9afce
Show file tree
Hide file tree
Showing 16 changed files with 98 additions and 147 deletions.
9 changes: 2 additions & 7 deletions cmd/mdatagen/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package main

import (
"io/ioutil"
"os"
"path/filepath"
"testing"

Expand Down Expand Up @@ -82,16 +81,12 @@ func Test_runContents(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tmpdir, err := ioutil.TempDir("", "metadata-test-*")
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, os.RemoveAll(tmpdir))
})
tmpdir := t.TempDir()

metadataFile := filepath.Join(tmpdir, "metadata.yaml")
require.NoError(t, ioutil.WriteFile(metadataFile, []byte(tt.args.yml), 0600))

err = run(metadataFile, tt.args.useExpGen)
err := run(metadataFile, tt.args.useExpGen)

if tt.wantErr != "" {
require.Regexp(t, tt.wantErr, err)
Expand Down
4 changes: 1 addition & 3 deletions extension/storage/dbstorage/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package dbstorage
import (
"context"
"fmt"
"io/ioutil"
"sync"
"testing"

Expand Down Expand Up @@ -108,8 +107,7 @@ func TestExtensionIntegrity(t *testing.T) {
}

func newTestExtension(t *testing.T) storage.Extension {
tempDir, err := ioutil.TempDir("", "")
require.NoError(t, err)
tempDir := t.TempDir()

f := NewFactory()
cfg := f.CreateDefaultConfig().(*Config)
Expand Down
1 change: 1 addition & 0 deletions extension/storage/filestorage/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func newClient(filePath string, timeout time.Duration) (*fileStorageClient, erro
return err
}
if err := db.Update(initBucket); err != nil {
_ = db.Close()
return nil, err
}

Expand Down
39 changes: 19 additions & 20 deletions extension/storage/filestorage/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package filestorage
import (
"context"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"testing"
"time"
Expand All @@ -30,11 +28,14 @@ import (
)

func TestClientOperations(t *testing.T) {
tempDir := newTempDir(t)
tempDir := t.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client.Close(context.TODO()))
})

ctx := context.Background()
testKey := "testKey"
Expand Down Expand Up @@ -65,11 +66,14 @@ func TestClientOperations(t *testing.T) {
}

func TestClientBatchOperations(t *testing.T) {
tempDir := newTempDir(t)
tempDir := t.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client.Close(context.TODO()))
})

ctx := context.Background()
testSetEntries := []storage.Operation{
Expand Down Expand Up @@ -183,7 +187,7 @@ func TestNewClientTransactionErrors(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

tempDir := newTempDir(t)
tempDir := t.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, timeout)
Expand All @@ -194,6 +198,8 @@ func TestNewClientTransactionErrors(t *testing.T) {

// Validate expected behavior
tc.validate(t, client)

require.NoError(t, client.db.Close())
})
}
}
Expand All @@ -202,7 +208,7 @@ func TestNewClientErrorsOnInvalidBucket(t *testing.T) {
temp := defaultBucket
defaultBucket = nil

tempDir := newTempDir(t)
tempDir := t.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -213,7 +219,7 @@ func TestNewClientErrorsOnInvalidBucket(t *testing.T) {
}

func BenchmarkClientGet(b *testing.B) {
tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -229,7 +235,7 @@ func BenchmarkClientGet(b *testing.B) {
}

func BenchmarkClientGet100(b *testing.B) {
tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -249,7 +255,7 @@ func BenchmarkClientGet100(b *testing.B) {
}

func BenchmarkClientSet(b *testing.B) {
tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -266,7 +272,7 @@ func BenchmarkClientSet(b *testing.B) {
}

func BenchmarkClientSet100(b *testing.B) {
tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -286,7 +292,7 @@ func BenchmarkClientSet100(b *testing.B) {
}

func BenchmarkClientDelete(b *testing.B) {
tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -309,7 +315,7 @@ func BenchmarkClientSetLargeDB(b *testing.B) {
entry := make([]byte, entrySizeInBytes)
var testKey string

tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand Down Expand Up @@ -343,7 +349,7 @@ func BenchmarkClientInitLargeDB(b *testing.B) {
entryCount := 2000
var testKey string

tempDir := newTempDir(b)
tempDir := b.TempDir()
dbFile := filepath.Join(tempDir, "my_db")

client, err := newClient(dbFile, time.Second)
Expand All @@ -370,10 +376,3 @@ func BenchmarkClientInitLargeDB(b *testing.B) {
b.StartTimer()
}
}

func newTempDir(tb testing.TB) string {
tempDir, err := ioutil.TempDir("", "")
require.NoError(tb, err)
tb.Cleanup(func() { os.RemoveAll(tempDir) })
return tempDir
}
62 changes: 41 additions & 21 deletions extension/storage/filestorage/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ func TestExtensionIntegrity(t *testing.T) {
for _, c := range components {
client, err := se.GetClient(ctx, c.kind, c.name, "")
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client.Close(ctx))
})

clients[c.name] = client
}

Expand Down Expand Up @@ -116,6 +120,9 @@ func TestClientHandlesSimpleCases(t *testing.T) {

myBytes := []byte("value")
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client.Close(ctx))
})

// Set the data
err = client.Set(ctx, "key", myBytes)
Expand Down Expand Up @@ -156,6 +163,9 @@ func TestTwoClientsWithDifferentNames(t *testing.T) {
"foo",
)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client1.Close(ctx))
})

client2, err := se.GetClient(
ctx,
Expand All @@ -164,6 +174,9 @@ func TestTwoClientsWithDifferentNames(t *testing.T) {
"bar",
)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client2.Close(ctx))
})

myBytes1 := []byte("value1")
myBytes2 := []byte("value2")
Expand All @@ -188,8 +201,7 @@ func TestTwoClientsWithDifferentNames(t *testing.T) {
func TestGetClientErrorsOnDeletedDirectory(t *testing.T) {
ctx := context.Background()

tempDir, err := ioutil.TempDir("", "")
require.NoError(t, err)
tempDir := t.TempDir()

f := NewFactory()
cfg := f.CreateDefaultConfig().(*Config)
Expand Down Expand Up @@ -217,8 +229,7 @@ func TestGetClientErrorsOnDeletedDirectory(t *testing.T) {
}

func newTestExtension(t *testing.T) storage.Extension {
tempDir, err := ioutil.TempDir("", "")
require.NoError(t, err)
tempDir := t.TempDir()

f := NewFactory()
cfg := f.CreateDefaultConfig().(*Config)
Expand All @@ -240,8 +251,7 @@ func newTestEntity(name string) config.ComponentID {
func TestCompaction(t *testing.T) {
ctx := context.Background()

tempDir, err := ioutil.TempDir("", "")
require.NoError(t, err)
tempDir := t.TempDir()

f := NewFactory()
cfg := f.CreateDefaultConfig().(*Config)
Expand All @@ -259,8 +269,10 @@ func TestCompaction(t *testing.T) {
newTestEntity("my_component"),
"",
)

require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client.Close(ctx))
})

files, err := ioutil.ReadDir(tempDir)
require.NoError(t, err)
Expand Down Expand Up @@ -290,8 +302,11 @@ func TestCompaction(t *testing.T) {
// compact the db
c, ok := client.(*fileStorageClient)
require.True(t, ok)
client, err = c.Compact(ctx, tempDir, cfg.Timeout, 1)
fsClient1, err := c.Compact(ctx, tempDir, cfg.Timeout, 1)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, fsClient1.Close(ctx))
})

// check size after compaction
newStats, err := os.Stat(path)
Expand All @@ -301,15 +316,16 @@ func TestCompaction(t *testing.T) {
// remove data from database
for i = 0; i < numEntries; i++ {
key = fmt.Sprintf("key_%d", i)
err = client.Delete(ctx, key)
err = fsClient1.Delete(ctx, key)
require.NoError(t, err)
}

// compact after data removal
c, ok = client.(*fileStorageClient)
require.True(t, ok)
_, err = c.Compact(ctx, tempDir, cfg.Timeout, 1)
fsClient2, err := fsClient1.Compact(ctx, tempDir, cfg.Timeout, 1)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, fsClient2.Close(ctx))
})

// check size
stats = newStats
Expand All @@ -323,8 +339,7 @@ func TestCompaction(t *testing.T) {
func TestCompactionRemoveTemp(t *testing.T) {
ctx := context.Background()

tempDir, err := ioutil.TempDir("", "")
require.NoError(t, err)
tempDir := t.TempDir()

f := NewFactory()
cfg := f.CreateDefaultConfig().(*Config)
Expand All @@ -342,8 +357,10 @@ func TestCompactionRemoveTemp(t *testing.T) {
newTestEntity("my_component"),
"",
)

require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, client.Close(ctx))
})

// check if only db exists in tempDir
files, err := ioutil.ReadDir(tempDir)
Expand All @@ -354,8 +371,11 @@ func TestCompactionRemoveTemp(t *testing.T) {
// perform compaction in the same directory
c, ok := client.(*fileStorageClient)
require.True(t, ok)
client, err = c.Compact(ctx, tempDir, cfg.Timeout, 1)
fsClient1, err := c.Compact(ctx, tempDir, cfg.Timeout, 1)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, fsClient1.Close(ctx))
})

// check if only db exists in tempDir
files, err = ioutil.ReadDir(tempDir)
Expand All @@ -364,13 +384,13 @@ func TestCompactionRemoveTemp(t *testing.T) {
require.Equal(t, fileName, files[0].Name())

// perform compaction in different directory
emptyTempDir, err := ioutil.TempDir("", "")
require.NoError(t, err)
emptyTempDir := t.TempDir()

c, ok = client.(*fileStorageClient)
require.True(t, ok)
_, err = c.Compact(ctx, emptyTempDir, cfg.Timeout, 1)
fsClient2, err := fsClient1.Compact(ctx, emptyTempDir, cfg.Timeout, 1)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, fsClient2.Close(ctx))
})

// check if emptyTempDir is empty after compaction
files, err = ioutil.ReadDir(emptyTempDir)
Expand Down
4 changes: 1 addition & 3 deletions extension/storage/filestorage/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package filestorage

import (
"context"
"io/ioutil"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -53,9 +52,8 @@ func TestFactory(t *testing.T) {
{
name: "Default",
config: func() *Config {
tempDir, _ := ioutil.TempDir("", "")
return &Config{
Directory: tempDir,
Directory: t.TempDir(),
Compaction: &CompactionConfig{},
}
}(),
Expand Down
Loading

0 comments on commit 2f9afce

Please sign in to comment.