Skip to content

Commit

Permalink
all: use T.TempDir to create temporary test directories (ethereum#24633)
Browse files Browse the repository at this point in the history
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
had 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 <engzerjun@gmail.com>
  • Loading branch information
Juneezee authored and sadoci committed Jan 27, 2023
1 parent fc15a03 commit f64247b
Show file tree
Hide file tree
Showing 34 changed files with 140 additions and 372 deletions.
8 changes: 2 additions & 6 deletions accounts/abi/bind/bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1970,14 +1970,10 @@ func TestGolangBindings(t *testing.T) {
t.Skip("go sdk not found for testing")
}
// Create a temporary workspace for the test suite
ws, err := ioutil.TempDir("", "binding-test")
if err != nil {
t.Fatalf("failed to create temporary workspace: %v", err)
}
//defer os.RemoveAll(ws)
ws := t.TempDir()

pkg := filepath.Join(ws, "bindtest")
if err = os.MkdirAll(pkg, 0700); err != nil {
if err := os.MkdirAll(pkg, 0700); err != nil {
t.Fatalf("failed to create package: %v", err)
}
// Generate the test suite for all the contracts
Expand Down
1 change: 0 additions & 1 deletion accounts/keystore/account_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func TestWatchNewFile(t *testing.T) {
t.Parallel()

dir, ks := tmpKeyStore(t, false)
defer os.RemoveAll(dir)

// Ensure the watcher is started before adding any files.
ks.Accounts()
Expand Down
43 changes: 13 additions & 30 deletions accounts/keystore/keystore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package keystore

import (
"io/ioutil"
"math/rand"
"os"
"runtime"
Expand All @@ -38,7 +37,6 @@ var testSigData = make([]byte, 32)

func TestKeyStore(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)

a, err := ks.NewAccount("foo")
if err != nil {
Expand Down Expand Up @@ -72,8 +70,7 @@ func TestKeyStore(t *testing.T) {
}

func TestSign(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, true)

pass := "" // not used but required by API
a1, err := ks.NewAccount(pass)
Expand All @@ -89,8 +86,7 @@ func TestSign(t *testing.T) {
}

func TestSignWithPassphrase(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, true)

pass := "passwd"
acc, err := ks.NewAccount(pass)
Expand All @@ -117,8 +113,7 @@ func TestSignWithPassphrase(t *testing.T) {
}

func TestTimedUnlock(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, true)

pass := "foo"
a1, err := ks.NewAccount(pass)
Expand Down Expand Up @@ -152,8 +147,7 @@ func TestTimedUnlock(t *testing.T) {
}

func TestOverrideUnlock(t *testing.T) {
dir, ks := tmpKeyStore(t, false)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, false)

pass := "foo"
a1, err := ks.NewAccount(pass)
Expand Down Expand Up @@ -193,8 +187,7 @@ func TestOverrideUnlock(t *testing.T) {

// This test should fail under -race if signing races the expiration goroutine.
func TestSignRace(t *testing.T) {
dir, ks := tmpKeyStore(t, false)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, false)

// Create a test account.
a1, err := ks.NewAccount("")
Expand Down Expand Up @@ -226,8 +219,7 @@ func TestWalletNotifierLifecycle(t *testing.T) {
}

// Create a temporary kesytore to test with
dir, ks := tmpKeyStore(t, false)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, false)

// Ensure that the notification updater is not running yet
time.Sleep(250 * time.Millisecond)
Expand Down Expand Up @@ -287,8 +279,7 @@ type walletEvent struct {
// Tests that wallet notifications and correctly fired when accounts are added
// or deleted from the keystore.
func TestWalletNotifications(t *testing.T) {
dir, ks := tmpKeyStore(t, false)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, false)

// Subscribe to the wallet feed and collect events.
var (
Expand Down Expand Up @@ -349,8 +340,7 @@ func TestWalletNotifications(t *testing.T) {

// TestImportExport tests the import functionality of a keystore.
func TestImportECDSA(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, true)
key, err := crypto.GenerateKey()
if err != nil {
t.Fatalf("failed to generate key: %v", key)
Expand All @@ -368,8 +358,7 @@ func TestImportECDSA(t *testing.T) {

// TestImportECDSA tests the import and export functionality of a keystore.
func TestImportExport(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, true)
acc, err := ks.NewAccount("old")
if err != nil {
t.Fatalf("failed to create account: %v", acc)
Expand All @@ -378,8 +367,7 @@ func TestImportExport(t *testing.T) {
if err != nil {
t.Fatalf("failed to export account: %v", acc)
}
dir2, ks2 := tmpKeyStore(t, true)
defer os.RemoveAll(dir2)
_, ks2 := tmpKeyStore(t, true)
if _, err = ks2.Import(json, "old", "old"); err == nil {
t.Errorf("importing with invalid password succeeded")
}
Expand All @@ -399,8 +387,7 @@ func TestImportExport(t *testing.T) {
// TestImportRace tests the keystore on races.
// This test should fail under -race if importing races.
func TestImportRace(t *testing.T) {
dir, ks := tmpKeyStore(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStore(t, true)
acc, err := ks.NewAccount("old")
if err != nil {
t.Fatalf("failed to create account: %v", acc)
Expand All @@ -409,8 +396,7 @@ func TestImportRace(t *testing.T) {
if err != nil {
t.Fatalf("failed to export account: %v", acc)
}
dir2, ks2 := tmpKeyStore(t, true)
defer os.RemoveAll(dir2)
_, ks2 := tmpKeyStore(t, true)
var atom uint32
var wg sync.WaitGroup
wg.Add(2)
Expand Down Expand Up @@ -466,10 +452,7 @@ func checkEvents(t *testing.T, want []walletEvent, have []walletEvent) {
}

func tmpKeyStore(t *testing.T, encrypted bool) (string, *KeyStore) {
d, err := ioutil.TempDir("", "eth-keystore-test")
if err != nil {
t.Fatal(err)
}
d := t.TempDir()
newKs := NewPlaintextKeyStore
if encrypted {
newKs = func(kd string) *KeyStore { return NewKeyStore(kd, veryLightScryptN, veryLightScryptP) }
Expand Down
17 changes: 4 additions & 13 deletions accounts/keystore/plain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"crypto/rand"
"encoding/hex"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"reflect"
"strings"
Expand All @@ -32,10 +30,7 @@ import (
)

func tmpKeyStoreIface(t *testing.T, encrypted bool) (dir string, ks keyStore) {
d, err := ioutil.TempDir("", "geth-keystore-test")
if err != nil {
t.Fatal(err)
}
d := t.TempDir()
if encrypted {
ks = &keyStorePassphrase{d, veryLightScryptN, veryLightScryptP, true}
} else {
Expand All @@ -45,8 +40,7 @@ func tmpKeyStoreIface(t *testing.T, encrypted bool) (dir string, ks keyStore) {
}

func TestKeyStorePlain(t *testing.T) {
dir, ks := tmpKeyStoreIface(t, false)
defer os.RemoveAll(dir)
_, ks := tmpKeyStoreIface(t, false)

pass := "" // not used but required by API
k1, account, err := storeNewKey(ks, rand.Reader, pass)
Expand All @@ -66,8 +60,7 @@ func TestKeyStorePlain(t *testing.T) {
}

func TestKeyStorePassphrase(t *testing.T) {
dir, ks := tmpKeyStoreIface(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStoreIface(t, true)

pass := "foo"
k1, account, err := storeNewKey(ks, rand.Reader, pass)
Expand All @@ -87,8 +80,7 @@ func TestKeyStorePassphrase(t *testing.T) {
}

func TestKeyStorePassphraseDecryptionFail(t *testing.T) {
dir, ks := tmpKeyStoreIface(t, true)
defer os.RemoveAll(dir)
_, ks := tmpKeyStoreIface(t, true)

pass := "foo"
k1, account, err := storeNewKey(ks, rand.Reader, pass)
Expand All @@ -102,7 +94,6 @@ func TestKeyStorePassphraseDecryptionFail(t *testing.T) {

func TestImportPreSaleKey(t *testing.T) {
dir, ks := tmpKeyStoreIface(t, true)
defer os.RemoveAll(dir)

// file content of a presale key file generated with:
// python pyethsaletool.py genwallet
Expand Down
8 changes: 1 addition & 7 deletions cmd/ethkey/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,12 @@
package main

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

func TestMessageSignVerify(t *testing.T) {
tmpdir, err := ioutil.TempDir("", "ethkey-test")
if err != nil {
t.Fatal("Can't create temporary directory:", err)
}
defer os.RemoveAll(tmpdir)
tmpdir := t.TempDir()

keyfile := filepath.Join(tmpdir, "the-keyfile")
message := "test message"
Expand Down
4 changes: 2 additions & 2 deletions cmd/geth/accountcmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
// are copied into a temporary keystore directory.

func tmpDatadirWithKeystore(t *testing.T) string {
datadir := tmpdir(t)
datadir := t.TempDir()
keystore := filepath.Join(datadir, "keystore")
source := filepath.Join("..", "..", "accounts", "keystore", "testdata", "keystore")
if err := cp.CopyAll(keystore, source); err != nil {
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestAccountImport(t *testing.T) {
}

func importAccountWithExpect(t *testing.T, key string, expected string) {
dir := tmpdir(t)
dir := t.TempDir()
keyfile := filepath.Join(dir, "key.prv")
if err := ioutil.WriteFile(keyfile, []byte(key), 0600); err != nil {
t.Error(err)
Expand Down
6 changes: 2 additions & 4 deletions cmd/geth/consolecmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"crypto/rand"
"math/big"
"os"
"path/filepath"
"runtime"
"strconv"
Expand Down Expand Up @@ -92,9 +91,7 @@ func TestAttachWelcome(t *testing.T) {
if runtime.GOOS == "windows" {
ipc = `\\.\pipe\geth` + strconv.Itoa(trulyRandInt(100000, 999999))
} else {
ws := tmpdir(t)
defer os.RemoveAll(ws)
ipc = filepath.Join(ws, "gwemix.ipc")
ipc = filepath.Join(t.TempDir(), "gwemix.ipc")
}
// And HTTP + WS attachment
p := trulyRandInt(1024, 65533) // Yeah, sometimes this will fail, sorry :P
Expand All @@ -118,6 +115,7 @@ func TestAttachWelcome(t *testing.T) {
waitForEndpoint(t, endpoint, 3*time.Second)
testAttachWelcome(t, geth, endpoint, httpAPIs)
})
geth.ExpectExit()
}

func testAttachWelcome(t *testing.T, geth *testgeth, endpoint, apis string) {
Expand Down
4 changes: 1 addition & 3 deletions cmd/geth/dao_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"io/ioutil"
"math/big"
"os"
"path/filepath"
"testing"

Expand Down Expand Up @@ -111,8 +110,7 @@ func testDAOForkBlockNewChain(t *testing.T, test int, genesis string, expectBloc
}

// Create a temporary data directory to use and inspect later
datadir := tmpdir(t)
defer os.RemoveAll(datadir)
datadir := t.TempDir()

// Start a Geth instance with the requested flags set and immediately terminate
if genesis != "" {
Expand Down
4 changes: 1 addition & 3 deletions cmd/geth/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package main

import (
"io/ioutil"
"os"
"path/filepath"
"testing"
)
Expand Down Expand Up @@ -73,8 +72,7 @@ var customGenesisTests = []struct {
func TestCustomGenesis(t *testing.T) {
for i, tt := range customGenesisTests {
// Create a temporary data directory to use and inspect later
datadir := tmpdir(t)
defer os.RemoveAll(datadir)
datadir := t.TempDir()

// Initialize the data directory with the custom genesis block
json := filepath.Join(datadir, "genesis.json")
Expand Down
19 changes: 2 additions & 17 deletions cmd/geth/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"context"
"fmt"
"io/ioutil"
"os"
"testing"
"time"
Expand All @@ -29,14 +28,6 @@ import (
"github.com/ethereum/go-ethereum/rpc"
)

func tmpdir(t *testing.T) string {
dir, err := ioutil.TempDir("", "geth-test")
if err != nil {
t.Fatal(err)
}
return dir
}

type testgeth struct {
*cmdtest.TestCmd

Expand Down Expand Up @@ -82,15 +73,9 @@ func runGeth(t *testing.T, args ...string) *testgeth {
}
}
if tt.Datadir == "" {
tt.Datadir = tmpdir(t)
tt.Cleanup = func() { os.RemoveAll(tt.Datadir) }
// The temporary datadir will be removed automatically if something fails below.
tt.Datadir = t.TempDir()
args = append([]string{"--datadir", tt.Datadir}, args...)
// Remove the temporary datadir if something fails below.
defer func() {
if t.Failed() {
tt.Cleanup()
}
}()
}

// Boot "geth". This actually runs the test binary but the TestMain
Expand Down
8 changes: 3 additions & 5 deletions consensus/ethash/algorithm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,8 @@ func TestHashimoto(t *testing.T) {
// Tests that caches generated on disk may be done concurrently.
func TestConcurrentDiskCacheGeneration(t *testing.T) {
// Create a temp folder to generate the caches into
// TODO: t.TempDir fails to remove the directory on Windows
// \AppData\Local\Temp\1\TestConcurrentDiskCacheGeneration2382060137\001\cache-R23-1dca8a85e74aa763: Access is denied.
cachedir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatalf("Failed to create temporary cache dir: %v", err)
Expand Down Expand Up @@ -794,11 +796,7 @@ func BenchmarkHashimotoFullSmall(b *testing.B) {

func benchmarkHashimotoFullMmap(b *testing.B, name string, lock bool) {
b.Run(name, func(b *testing.B) {
tmpdir, err := ioutil.TempDir("", "ethash-test")
if err != nil {
b.Fatal(err)
}
defer os.RemoveAll(tmpdir)
tmpdir := b.TempDir()

d := &dataset{epoch: 0}
d.generate(tmpdir, 1, lock, false)
Expand Down
Loading

0 comments on commit f64247b

Please sign in to comment.