Skip to content

Commit

Permalink
Check errors when removing test directories and files
Browse files Browse the repository at this point in the history 
Currently in tests, we have calls to os.Remove and os.RemoveAll where we
don't check the returned error. This hides useful error messages when
tests fail to run, such as "too many open files".

This change checks for more filesystem related errors and calls t.Fatal
if there is an error.
  • Loading branch information
@nsurfer
nsurfer committed Apr 7, 2021
1 parent 3e4f6b8 commit d929ee1
Show file tree
Hide file tree
Showing 38 changed files with 688 additions and 667 deletions.
15 changes: 11 additions & 4 deletions logger/log_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestStdLoggerTraceWithOutDebug(t *testing.T) {

func TestFileLogger(t *testing.T) {
tmpDir := createDir(t, "_nats-server")
defer os.RemoveAll(tmpDir)
defer removeDir(t, tmpDir)

file := createFileAtDir(t, tmpDir, "nats-server:log_")
file.Close()
Expand Down Expand Up @@ -178,7 +178,7 @@ func TestFileLoggerSizeLimit(t *testing.T) {
logger.Close()

tmpDir := createDir(t, "nats-server")
defer os.RemoveAll(tmpDir)
defer removeDir(t, tmpDir)

file := createFileAtDir(t, tmpDir, "log_")
file.Close()
Expand Down Expand Up @@ -212,9 +212,9 @@ func TestFileLoggerSizeLimit(t *testing.T) {
}

// Remove all files
os.RemoveAll(tmpDir)
removeDir(t, tmpDir)
tmpDir = createDir(t, "nats-server")
defer os.RemoveAll(tmpDir)
defer removeDir(t, tmpDir)

// Recreate logger and don't set a limit
file = createFileAtDir(t, tmpDir, "log_")
Expand Down Expand Up @@ -346,3 +346,10 @@ func createFileAtDir(t *testing.T, dir, prefix string) *os.File {
}
return f
}

func removeDir(t *testing.T, dir string) {
t.Helper()
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
48 changes: 24 additions & 24 deletions server/accounts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"encoding/json"
"fmt"
"net/http"
"os"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -282,7 +281,7 @@ func TestAccountIsolationExportImport(t *testing.T) {
`,
c.exp, c.imp,
)))
defer os.Remove(cf)
defer removeFile(t, cf)
s, _ := RunServerWithConfig(cf)
defer s.Shutdown()

Expand Down Expand Up @@ -486,7 +485,7 @@ func accountNameExists(name string, accounts []*Account) bool {

func TestAccountSimpleConfig(t *testing.T) {
confFileName := createConfFile(t, []byte(`accounts = [foo, bar]`))
defer os.Remove(confFileName)
defer removeFile(t, confFileName)
opts, err := ProcessConfigFile(confFileName)
if err != nil {
t.Fatalf("Received an error processing config file: %v", err)
Expand All @@ -503,7 +502,7 @@ func TestAccountSimpleConfig(t *testing.T) {

// Make sure double entries is an error.
confFileName = createConfFile(t, []byte(`accounts = [foo, foo]`))
defer os.Remove(confFileName)
defer removeFile(t, confFileName)
_, err = ProcessConfigFile(confFileName)
if err == nil {
t.Fatalf("Expected an error with double account entries")
Expand All @@ -527,7 +526,7 @@ func TestAccountParseConfig(t *testing.T) {
}
}
`))
defer os.Remove(confFileName)
defer removeFile(t, confFileName)
opts, err := ProcessConfigFile(confFileName)
if err != nil {
t.Fatalf("Received an error processing config file: %v", err)
Expand Down Expand Up @@ -577,7 +576,7 @@ func TestAccountParseConfigDuplicateUsers(t *testing.T) {
}
}
`))
defer os.Remove(confFileName)
defer removeFile(t, confFileName)
_, err := ProcessConfigFile(confFileName)
if err == nil {
t.Fatalf("Expected an error with double user entries")
Expand Down Expand Up @@ -685,7 +684,7 @@ func TestImportExportConfigFailures(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with import from unknown account")
}
Expand All @@ -697,7 +696,7 @@ func TestImportExportConfigFailures(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with import of a service with no account")
}
Expand All @@ -709,7 +708,7 @@ func TestImportExportConfigFailures(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with import of a service with wildcard subject")
}
Expand All @@ -721,7 +720,7 @@ func TestImportExportConfigFailures(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with export with unknown keyword")
}
Expand All @@ -733,7 +732,7 @@ func TestImportExportConfigFailures(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with import with unknown keyword")
}
Expand All @@ -745,7 +744,7 @@ func TestImportExportConfigFailures(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with export with account")
}
Expand Down Expand Up @@ -973,7 +972,7 @@ func TestStreamImportLengthBug(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)
if _, err := ProcessConfigFile(cf); err == nil {
t.Fatalf("Expected an error with import with wildcard prefix")
}
Expand Down Expand Up @@ -1810,7 +1809,8 @@ func TestAccountRequestReplyTrackLatency(t *testing.T) {

// This will test for leaks in the remote latency tracking via client.rrTracking
func TestAccountTrackLatencyRemoteLeaks(t *testing.T) {
optsA, _ := ProcessConfigFile("./configs/seed.conf")
optsA, err := ProcessConfigFile("./configs/seed.conf")
require_NoError(t, err)
optsA.NoSigs, optsA.NoLog = true, true
optsA.ServerName = "A"
srvA := RunServer(optsA)
Expand Down Expand Up @@ -2144,7 +2144,7 @@ func TestAccountMapsUsers(t *testing.T) {
}
}
`))
defer os.Remove(confFileName)
defer removeFile(t, confFileName)
opts, err := ProcessConfigFile(confFileName)
if err != nil {
t.Fatalf("Unexpected error parsing config file: %v", err)
Expand Down Expand Up @@ -2254,7 +2254,7 @@ func TestAccountGlobalDefault(t *testing.T) {

// Make sure we can not define one in a config file either.
confFileName := createConfFile(t, []byte(`accounts { $G {} }`))
defer os.Remove(confFileName)
defer removeFile(t, confFileName)

if _, err := ProcessConfigFile(confFileName); err == nil {
t.Fatalf("Expected an error parsing config file with reserved account")
Expand Down Expand Up @@ -2788,7 +2788,7 @@ func TestGlobalAccountRouteMappingsConfiguration(t *testing.T) {
bar.*.*: RAB.$2.$1
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, _ := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -2843,7 +2843,7 @@ func TestAccountRouteMappingsConfiguration(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, _ := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -2873,7 +2873,7 @@ func TestAccountRouteMappingsWithLossInjection(t *testing.T) {
bar: { dest: bar, weight: 0% }
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, _ := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -2910,7 +2910,7 @@ func TestAccountRouteMappingsWithOriginClusterFilter(t *testing.T) {
foo: { dest: bar, cluster: SYN, weight: 100% }
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, _ := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -2955,7 +2955,7 @@ func TestAccountServiceImportWithRouteMappings(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, opts := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -3006,7 +3006,7 @@ func TestAccountImportsWithWildcardSupport(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, opts := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -3099,7 +3099,7 @@ func TestAccountImportsWithWildcardSupportStreamAndService(t *testing.T) {
}
}
`))
defer os.Remove(cf)
defer removeFile(t, cf)

s, opts := RunServerWithConfig(cf)
defer s.Shutdown()
Expand Down Expand Up @@ -3275,7 +3275,7 @@ func TestAccountSystemPermsWithGlobalAccess(t *testing.T) {
$SYS { users = [ { user: "admin", pass: "s3cr3t!" } ] }
}
`))
defer os.Remove(conf)
defer removeFile(t, conf)

s, _ := RunServerWithConfig(conf)

Expand Down
3 changes: 1 addition & 2 deletions server/config_check_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package server
import (
"errors"
"fmt"
"os"
"strings"
"testing"
)
Expand Down Expand Up @@ -1488,7 +1487,7 @@ func TestConfigCheck(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
conf := createConfFile(t, []byte(test.config))
defer os.Remove(conf)
defer removeFile(t, conf)
err := checkConfig(conf)
var expectedErr error

Expand Down
2 changes: 1 addition & 1 deletion server/dirstore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ func TestReload(t *testing.T) {
for k := range files {
hash = dirStore.Hash()
require_False(t, bytes.Equal(hash[:], emptyHash[:]))
os.Remove(k)
removeFile(t, k)
err = dirStore.Reload()
require_NoError(t, err)
assertStoreSize(t, dirStore, len(files)-1)
Expand Down
11 changes: 5 additions & 6 deletions server/events_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -464,11 +463,11 @@ func TestSystemAccountingWithLeafNodes(t *testing.T) {
}
seed, _ := kp.Seed()
mycreds := genCredsFile(t, ujwt, seed)
defer os.Remove(mycreds)
defer removeFile(t, mycreds)

// Create a server that solicits a leafnode connection.
sl, slopts, lnconf := runSolicitWithCredentials(t, opts, mycreds)
defer os.Remove(lnconf)
defer removeFile(t, lnconf)
defer sl.Shutdown()

checkLeafNodeConnected(t, s)
Expand Down Expand Up @@ -1159,7 +1158,7 @@ func TestSystemAccountFromConfig(t *testing.T) {
`

conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, opub, apub, ts.URL)))
defer os.Remove(conf)
defer removeFile(t, conf)

s, _ := RunServerWithConfig(conf)
defer s.Shutdown()
Expand Down Expand Up @@ -2229,7 +2228,7 @@ func TestServerEventsFilteredByTag(t *testing.T) {
}
no_auth_user: b
`))
defer os.Remove(confA)
defer removeFile(t, confA)
sA, _ := RunServerWithConfig(confA)
defer sA.Shutdown()
confB := createConfFile(t, []byte(fmt.Sprintf(`
Expand All @@ -2254,7 +2253,7 @@ func TestServerEventsFilteredByTag(t *testing.T) {
}
no_auth_user: b
`, sA.opts.Cluster.Port)))
defer os.Remove(confB)
defer removeFile(t, confB)
sB, _ := RunServerWithConfig(confB)
defer sB.Shutdown()
checkClusterFormed(t, sA, sB)
Expand Down
Loading

0 comments on commit d929ee1

Please sign in to comment.