From 21b3d6a6c90d7ee3ea743a441c383cc69a83abd1 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Thu, 28 Apr 2022 10:01:15 +0800 Subject: [PATCH] test: use `T.TempDir` to create temporary test directory (#934) * 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 * test: fix TestEncryptTwiceReusesSaltButNewCipher on Windows Signed-off-by: Eng Zer Jun * test: fix TestCheckConnection on Windows Signed-off-by: Eng Zer Jun * test: fix TestRPCServer_GetTicker_LastUpdatedNanos on Windows Signed-off-by: Eng Zer Jun * test: cleanup TestGenerateReport Signed-off-by: Eng Zer Jun --- backtester/config/config_test.go | 13 ++------ backtester/report/report_test.go | 37 ++++++++-------------- common/file/archive/zip_test.go | 24 ++------------- common/file/file_test.go | 14 ++------- config/config_encryption_test.go | 30 ++++++++---------- config/config_test.go | 6 +--- engine/database_connection_test.go | 49 ++++++++++++------------------ engine/engine_test.go | 32 ++----------------- engine/rpcserver_test.go | 14 +++++---- 9 files changed, 62 insertions(+), 157 deletions(-) diff --git a/backtester/config/config_test.go b/backtester/config/config_test.go index e4f27bf3..c1f36a80 100644 --- a/backtester/config/config_test.go +++ b/backtester/config/config_test.go @@ -62,18 +62,9 @@ func TestLoadConfig(t *testing.T) { } func TestReadConfigFromFile(t *testing.T) { - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer func() { - err = os.RemoveAll(tempDir) - if err != nil { - t.Error(err) - } - }() + tempDir := t.TempDir() var passFile *os.File - passFile, err = ioutil.TempFile(tempDir, "*.start") + passFile, err := ioutil.TempFile(tempDir, "*.start") if err != nil { t.Fatalf("Problem creating temp file at %v: %s\n", passFile, err) } diff --git a/backtester/report/report_test.go b/backtester/report/report_test.go index 407b1f34..85f4a09c 100644 --- a/backtester/report/report_test.go +++ b/backtester/report/report_test.go @@ -2,9 +2,6 @@ package report import ( "errors" - "io/ioutil" - "os" - "path/filepath" "testing" "time" @@ -27,19 +24,13 @@ func TestGenerateReport(t *testing.T) { e := testExchange a := asset.Spot p := currency.NewPair(currency.BTC, currency.USDT) - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer func(path string) { - err = os.RemoveAll(path) - if err != nil { - t.Error(err) - } - }(tempDir) d := Data{ - Config: &config.Config{}, - OutputPath: filepath.Join("..", "results"), + Config: &config.Config{ + StrategySettings: config.StrategySettings{ + DisableUSDTracking: true, + }, + }, + OutputPath: t.TempDir(), TemplatePath: "tpl.gohtml", OriginalCandles: []*gctkline.Item{ { @@ -312,18 +303,14 @@ func TestGenerateReport(t *testing.T) { }, CurrencyPairStatistics: nil, WasAnyDataMissing: false, - FundingStatistics: nil, + FundingStatistics: &statistics.FundingStatistics{ + Report: &funding.Report{ + DisableUSDTracking: true, + }, + }, }, } - d.OutputPath = tempDir - d.Config.StrategySettings.DisableUSDTracking = true - d.Statistics.FundingStatistics = &statistics.FundingStatistics{ - Report: &funding.Report{ - DisableUSDTracking: true, - }, - } - err = d.GenerateReport() - if err != nil { + if err := d.GenerateReport(); err != nil { t.Error(err) } } diff --git a/common/file/archive/zip_test.go b/common/file/archive/zip_test.go index 050b67d7..3df5da9c 100644 --- a/common/file/archive/zip_test.go +++ b/common/file/archive/zip_test.go @@ -3,33 +3,12 @@ package archive import ( "archive/zip" "errors" - "fmt" - "io/ioutil" - "os" "path/filepath" "testing" ) -var ( - tempDir string -) - -func TestMain(m *testing.M) { - var err error - tempDir, err = ioutil.TempDir("", "gct-temp") - if err != nil { - fmt.Printf("failed to create tempDir: %v", err) - os.Exit(1) - } - t := m.Run() - err = os.RemoveAll(tempDir) - if err != nil { - fmt.Printf("Failed to remove tempDir %v", err) - } - os.Exit(t) -} - func TestUnZip(t *testing.T) { + tempDir := t.TempDir() zipFile := filepath.Join("..", "..", "..", "testdata", "testdata.zip") files, err := UnZip(zipFile, tempDir) if err != nil { @@ -53,6 +32,7 @@ func TestUnZip(t *testing.T) { } func TestZip(t *testing.T) { + tempDir := t.TempDir() singleFile := filepath.Join("..", "..", "..", "testdata", "configtest.json") outFile := filepath.Join(tempDir, "out.zip") err := Zip(singleFile, outFile) diff --git a/common/file/file_test.go b/common/file/file_test.go index 58a0e653..9671245a 100644 --- a/common/file/file_test.go +++ b/common/file/file_test.go @@ -199,11 +199,7 @@ func TestWriter(t *testing.T) { type args struct { file string } - tmp, err := ioutil.TempDir("", "") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmp) + tmp := t.TempDir() testData := `data` @@ -269,12 +265,8 @@ func TestWriterNoPermissionFails(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Skip file permissions") } - temp, err := ioutil.TempDir("", "") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(temp) - err = os.Chmod(temp, 0o555) + temp := t.TempDir() + err := os.Chmod(temp, 0o555) if err != nil { t.Fatal(err) } diff --git a/config/config_encryption_test.go b/config/config_encryption_test.go index a0f00996..0547d76d 100644 --- a/config/config_encryption_test.go +++ b/config/config_encryption_test.go @@ -139,11 +139,7 @@ func TestEncryptTwiceReusesSaltButNewCipher(t *testing.T) { c := &Config{ EncryptConfig: 1, } - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer os.RemoveAll(tempDir) + tempDir := t.TempDir() // Prepare input passFile, err := ioutil.TempFile(tempDir, "*.pw") @@ -161,11 +157,17 @@ func TestEncryptTwiceReusesSaltButNewCipher(t *testing.T) { // Temporarily replace Stdin with a custom input oldIn := os.Stdin - defer func() { os.Stdin = oldIn }() + t.Cleanup(func() { os.Stdin = oldIn }) os.Stdin, err = os.Open(passFile.Name()) if err != nil { t.Fatalf("Problem opening temp file at %s: %s\n", passFile.Name(), err) } + t.Cleanup(func() { + err = os.Stdin.Close() + if err != nil { + t.Error(err) + } + }) // Save encrypted config enc1 := filepath.Join(tempDir, "encrypted.dat") @@ -203,15 +205,11 @@ func TestSaveAndReopenEncryptedConfig(t *testing.T) { c := &Config{} c.Name = "myCustomName" c.EncryptConfig = 1 - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer os.RemoveAll(tempDir) + tempDir := t.TempDir() // Save encrypted config enc := filepath.Join(tempDir, "encrypted.dat") - err = withInteractiveResponse(t, "pass\npass\n", func() error { + err := withInteractiveResponse(t, "pass\npass\n", func() error { return c.SaveConfigToFile(enc) }) if err != nil { @@ -253,11 +251,7 @@ func setAnswersFile(t *testing.T, answerFile string) func() { func TestReadConfigWithPrompt(t *testing.T) { // Prepare temp dir - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer os.RemoveAll(tempDir) + tempDir := t.TempDir() // Ensure we'll get the prompt when loading c := &Config{ @@ -266,7 +260,7 @@ func TestReadConfigWithPrompt(t *testing.T) { // Save config testConfigFile := filepath.Join(tempDir, "config.json") - err = c.SaveConfigToFile(testConfigFile) + err := c.SaveConfigToFile(testConfigFile) if err != nil { t.Fatalf("Problem saving config file in %s: %s\n", tempDir, err) } diff --git a/config/config_test.go b/config/config_test.go index 4fad7bd8..b8cefd8d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -2176,11 +2176,7 @@ func TestMigrateConfig(t *testing.T) { targetDir string } - dir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() tests := []struct { name string diff --git a/engine/database_connection_test.go b/engine/database_connection_test.go index 168a39c5..9418aa4f 100644 --- a/engine/database_connection_test.go +++ b/engine/database_connection_test.go @@ -2,9 +2,7 @@ package engine import ( "errors" - "io/ioutil" "log" - "os" "sync" "testing" @@ -12,28 +10,19 @@ import ( "github.com/thrasher-corp/gocryptotrader/database/drivers" ) -func CreateDatabase(t *testing.T) string { +func CreateDatabase(t *testing.T) { t.Helper() // fun workarounds to globals ruining testing - tmpDir, err := ioutil.TempDir("", "") - if err != nil { - log.Fatal(err) - } - database.DB.DataPath = tmpDir - return tmpDir -} + database.DB.DataPath = t.TempDir() -func Cleanup(tmpDir string) { - if database.DB.IsConnected() { - err := database.DB.CloseConnection() - if err != nil { - log.Fatal(err) + t.Cleanup(func() { + if database.DB.IsConnected() { + err := database.DB.CloseConnection() + if err != nil { + log.Fatal(err) + } } - err = os.RemoveAll(tmpDir) - if err != nil { - log.Fatal(err) - } - } + }) } func TestSetupDatabaseConnectionManager(t *testing.T) { @@ -52,8 +41,7 @@ func TestSetupDatabaseConnectionManager(t *testing.T) { } func TestStartSQLite(t *testing.T) { - tmpDir := CreateDatabase(t) - defer Cleanup(tmpDir) + CreateDatabase(t) m, err := SetupDatabaseConnectionManager(&database.Config{}) if !errors.Is(err, nil) { t.Errorf("error '%v', expected '%v'", err, nil) @@ -113,8 +101,7 @@ func TestStartPostgres(t *testing.T) { } func TestDatabaseConnectionManagerIsRunning(t *testing.T) { - tmpDir := CreateDatabase(t) - defer Cleanup(tmpDir) + CreateDatabase(t) m, err := SetupDatabaseConnectionManager(&database.Config{ Enabled: true, Driver: database.DBSQLite, @@ -144,8 +131,7 @@ func TestDatabaseConnectionManagerIsRunning(t *testing.T) { } func TestDatabaseConnectionManagerStop(t *testing.T) { - tmpDir := CreateDatabase(t) - defer Cleanup(tmpDir) + CreateDatabase(t) m, err := SetupDatabaseConnectionManager(&database.Config{ Enabled: true, Driver: database.DBSQLite, @@ -181,8 +167,7 @@ func TestDatabaseConnectionManagerStop(t *testing.T) { } func TestCheckConnection(t *testing.T) { - tmpDir := CreateDatabase(t) - defer Cleanup(tmpDir) + CreateDatabase(t) var m *DatabaseConnectionManager err := m.checkConnection() if !errors.Is(err, ErrNilSubsystem) { @@ -236,11 +221,15 @@ func TestCheckConnection(t *testing.T) { if !errors.Is(err, database.ErrDatabaseNotConnected) { t.Errorf("error '%v', expected '%v'", err, database.ErrDatabaseNotConnected) } + + err = m.Stop() + if !errors.Is(err, nil) { + t.Errorf("error '%v', expected '%v'", err, nil) + } } func TestGetInstance(t *testing.T) { - tmpDir := CreateDatabase(t) - defer Cleanup(tmpDir) + CreateDatabase(t) m, err := SetupDatabaseConnectionManager(&database.Config{ Enabled: true, Driver: database.DBSQLite, diff --git a/engine/engine_test.go b/engine/engine_test.go index 1e4f0cf9..8aa707b4 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -2,7 +2,6 @@ package engine import ( "errors" - "io/ioutil" "os" "testing" "time" @@ -77,16 +76,7 @@ func TestLoadConfigWithSettings(t *testing.T) { func TestStartStopDoesNotCausePanic(t *testing.T) { t.Parallel() - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer func() { - err = os.RemoveAll(tempDir) - if err != nil { - t.Error(err) - } - }() + tempDir := t.TempDir() botOne, err := NewFromSettings(&Settings{ ConfigFile: config.TestFile, EnableDryRun: true, @@ -116,24 +106,8 @@ func TestStartStopTwoDoesNotCausePanic(t *testing.T) { if !enableExperimentalTest { t.Skip("test is functional, however does not need to be included in go test runs") } - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - tempDir2, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Problem creating temp dir at %s: %s\n", tempDir, err) - } - defer func() { - err = os.RemoveAll(tempDir) - if err != nil { - t.Error(err) - } - err = os.RemoveAll(tempDir2) - if err != nil { - t.Error(err) - } - }() + tempDir := t.TempDir() + tempDir2 := t.TempDir() botOne, err := NewFromSettings(&Settings{ ConfigFile: config.TestFile, EnableDryRun: true, diff --git a/engine/rpcserver_test.go b/engine/rpcserver_test.go index 2a5e8a12..eaa6a506 100644 --- a/engine/rpcserver_test.go +++ b/engine/rpcserver_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "io/ioutil" "log" "os" "path/filepath" @@ -266,17 +265,20 @@ func RPCTestSetup(t *testing.T) *Engine { if err != nil { t.Fatal(err) } - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatal(err) - } - dbm.dbConn.DataPath = tempDir + dbm.dbConn.DataPath = t.TempDir() engerino.DatabaseManager = dbm var wg sync.WaitGroup err = dbm.Start(&wg) if err != nil { t.Fatal(err) } + t.Cleanup(func() { + err = dbm.Stop() + if err != nil { + t.Fatal(err) + } + }) + engerino.Config = &config.Config{} em := SetupExchangeManager() exch, err := em.NewExchangeByName(testExchange)