Skip to content

Commit

Permalink
Don't use err when it's nil (#1046)
Browse files Browse the repository at this point in the history
  • Loading branch information
rfay committed Aug 14, 2018
1 parent 8d570e7 commit 4d7d345
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 25 deletions.
8 changes: 6 additions & 2 deletions pkg/appimport/appimport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,17 @@ func TestValidateAsset(t *testing.T) {
// db no sql
gofilePath := filepath.Join(testdata, "junk.go")
_, isArchive, err = appimport.ValidateAsset(gofilePath, "db")
assert.Contains(err.Error(), "provided path is not a .sql file or archive")
if err != nil {
assert.Contains(err.Error(), "provided path is not a .sql file or archive")
}
assert.Error(err)
assert.False(isArchive)

// files not a directory
_, isArchive, err = appimport.ValidateAsset(gofilePath, "files")
assert.False(isArchive)
assert.Error(err)
assert.Contains(err.Error(), "provided path is not a directory or archive")
if err != nil {
assert.Contains(err.Error(), "provided path is not a directory or archive")
}
}
28 changes: 21 additions & 7 deletions pkg/ddevapp/ddevapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,17 @@ func TestDdevStart(t *testing.T) {

err = app.Init(another.Dir)
assert.Error(err)
assert.Contains(err.Error(), fmt.Sprintf("a project (web container) in running state already exists for %s that was created at %s", TestSites[0].Name, TestSites[0].Dir))
if err != nil {
assert.Contains(err.Error(), fmt.Sprintf("a project (web container) in running state already exists for %s that was created at %s", TestSites[0].Name, TestSites[0].Dir))
}

// Make sure that GetActiveApp() also fails when trying to start app of duplicate name in current directory.
switchDir := another.Chdir()
_, err = ddevapp.GetActiveApp("")
assert.Error(err)
assert.Contains(err.Error(), fmt.Sprintf("a project (web container) in running state already exists for %s that was created at %s", TestSites[0].Name, TestSites[0].Dir))
if err != nil {
assert.Contains(err.Error(), fmt.Sprintf("a project (web container) in running state already exists for %s that was created at %s", TestSites[0].Name, TestSites[0].Dir))
}
testcommon.CleanupDir(another.Dir)
switchDir()
}
Expand Down Expand Up @@ -416,7 +420,9 @@ func TestStartWithoutDdevConfig(t *testing.T) {

_, err = ddevapp.GetActiveApp("")
assert.Error(err)
assert.Contains(err.Error(), "Could not find a project")
if err != nil {
assert.Contains(err.Error(), "Could not find a project")
}
}

// TestGetApps tests the GetApps function to ensure it accurately returns a list of running applications.
Expand Down Expand Up @@ -1283,23 +1289,29 @@ func TestMultipleComposeFiles(t *testing.T) {

_, err = app.ComposeFiles()
assert.Error(err)
assert.Contains(err.Error(), "there are more than one docker-compose.y*l")
if err != nil {
assert.Contains(err.Error(), "there are more than one docker-compose.y*l")
}

// Make sure that some docker-compose.override.yml and docker-compose.override.yaml conflict gets noted properly
app, err = ddevapp.NewApp("./testdata/testConflictingOverrideYaml", "")
assert.NoError(err)

_, err = app.ComposeFiles()
assert.Error(err)
assert.Contains(err.Error(), "there are more than one docker-compose.override.y*l")
if err != nil {
assert.Contains(err.Error(), "there are more than one docker-compose.override.y*l")
}

// Make sure the error gets pointed out of there's no main docker-compose.yaml
app, err = ddevapp.NewApp("./testdata/testNoDockerCompose", "")
assert.NoError(err)

_, err = app.ComposeFiles()
assert.Error(err)
assert.Contains(err.Error(), "failed to find a docker-compose.yml or docker-compose.yaml")
if err != nil {
assert.Contains(err.Error(), "failed to find a docker-compose.yml or docker-compose.yaml")
}

// Catch if we have no docker files at all.
// This should also fail if the docker-compose.yaml.bak gets loaded.
Expand All @@ -1308,7 +1320,9 @@ func TestMultipleComposeFiles(t *testing.T) {

_, err = app.ComposeFiles()
assert.Error(err)
assert.Contains(err.Error(), "failed to load any docker-compose.*y*l files")
if err != nil {
assert.Contains(err.Error(), "failed to load any docker-compose.*y*l files")
}
}

// TestGetAllURLs ensures the GetAllURLs function returns the expected number of URLs,
Expand Down
24 changes: 18 additions & 6 deletions pkg/ddevapp/providerDrudS3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ func TestDrudS3ConfigCommand(t *testing.T) {
restoreOutput = testcommon.CaptureUserOut()
err = app.PromptForConfig()
assert.Error(err)
assert.Contains(err.Error(), "NoSuchBucket")
if err != nil {
assert.Contains(err.Error(), "NoSuchBucket")
}
_ = restoreOutput()

// Now try with an invalid environment name, should fail
Expand All @@ -116,7 +118,9 @@ func TestDrudS3ConfigCommand(t *testing.T) {
assert.NoError(err)
err = provider.Validate()
assert.Error(err)
assert.Contains(err.Error(), "could not find an environment with backups")
if err != nil {
assert.Contains(err.Error(), "could not find an environment with backups")
}
}

// assertEqualProviderValues is just a helper function to avoid repeating assertions.
Expand Down Expand Up @@ -193,26 +197,34 @@ func TestDrudS3ValidDownloadObjects(t *testing.T) {
provider.AWSAccessKey = "AKIAIBSTOTALLYINVALID"
_, _, err = provider.GetBackup("database")
assert.Error(err)
assert.Contains(err.Error(), "InvalidAccessKeyId")
if err != nil {
assert.Contains(err.Error(), "InvalidAccessKeyId")
}

// Make sure invalid secret key gets correct behavior
provider.AWSAccessKey = accessKeyID
provider.AWSSecretKey = "rweeHGZ5totallyinvalidsecretkey"
_, _, err = provider.GetBackup("database")
assert.Error(err)
assert.Contains(err.Error(), "SignatureDoesNotMatch")
if err != nil {
assert.Contains(err.Error(), "SignatureDoesNotMatch")
}

// Make sure bad environment gets correct behavior.
provider.AWSSecretKey = secretAccessKey
provider.EnvironmentName = "someInvalidUnknownEnvironment"
_, _, err = provider.GetBackup("database")
assert.Error(err)
assert.Contains(err.Error(), "could not find an environment")
if err != nil {
assert.Contains(err.Error(), "could not find an environment")
}

// Make sure bad bucket gets correct behavior.
provider.S3Bucket = drudS3TestBucket
provider.S3Bucket = "someInvalidUnknownBucket"
_, _, err = provider.GetBackup("database")
assert.Error(err)
assert.Contains(err.Error(), "NoSuchBucket")
if err != nil {
assert.Contains(err.Error(), "NoSuchBucket")
}
}
16 changes: 12 additions & 4 deletions pkg/dockerutil/dockerutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,15 @@ func TestContainerWait(t *testing.T) {

err := ContainerWait(0, labels)
assert.Error(err)
assert.Contains(err.Error(), "health check timed out")
if err != nil {
assert.Contains(err.Error(), "health check timed out")
}

err = ContainerWait(5, labels)
assert.Error(err)
assert.Contains(err.Error(), "failed to query container")
if err != nil {
assert.Contains(err.Error(), "failed to query container")
}
}

// TestComposeCmd tests execution of docker-compose commands.
Expand Down Expand Up @@ -181,10 +185,14 @@ func TestRunSimpleContainer(t *testing.T) {
// Try the case of running nonexistent script
_, err = RunSimpleContainer("busybox", "TestRunSimpleContainer"+basename, []string{"nocommandbythatname"}, nil, []string{"TEMPENV=someenv"}, []string{testdata + ":/tempmount"}, "25")
assert.Error(err)
assert.Contains(err.Error(), "failed to StartContainer")
if err != nil {
assert.Contains(err.Error(), "failed to StartContainer")
}

// Try the case of running a script that fails
_, err = RunSimpleContainer("busybox", "TestRunSimpleContainer"+basename, []string{"/tempmount/simplescript.sh"}, nil, []string{"TEMPENV=someenv", "ERROROUT=true"}, []string{testdata + ":/tempmount"}, "25")
assert.Error(err)
assert.Contains(err.Error(), "container run failed with exit code 5")
if err != nil {
assert.Contains(err.Error(), "container run failed with exit code 5")
}
}
8 changes: 6 additions & 2 deletions pkg/fileutil/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@ func TestCopyDir(t *testing.T) {
// test source not a directory
err = fileutil.CopyDir(testFileLocation, sourceDir)
assert.Error(err)
assert.Contains(err.Error(), "source is not a directory")
if err != nil {
assert.Contains(err.Error(), "source is not a directory")
}

// test destination exists
err = fileutil.CopyDir(sourceDir, targetDir)
assert.Error(err)
assert.Contains(err.Error(), "destination already exists")
if err != nil {
assert.Contains(err.Error(), "destination already exists")
}
err = os.RemoveAll(subdir)
assert.NoError(err)

Expand Down
13 changes: 9 additions & 4 deletions pkg/testcommon/testcommon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ func TestTmpDir(t *testing.T) {
CleanupDir(testDir)
_, err = os.Stat(testDir)
assert.Error(err, "Could not stat temporary directory")
assert.True(os.IsNotExist(err), "Error is of type IsNotExists")
if err != nil {
assert.True(os.IsNotExist(err), "Error is of type IsNotExists")
}
}

// TestChdir tests the Chdir function and ensures it will change to a temporary directory and then properly return
Expand Down Expand Up @@ -139,7 +141,6 @@ func TestValidTestSite(t *testing.T) {
site.Cleanup()
_, err = os.Stat(site.Dir)
assert.Error(err, "Could not stat temporary directory after cleanup")

}

// TestGetLocalHTTPResponse() brings up a project and hits a URL to get the response
Expand Down Expand Up @@ -191,15 +192,19 @@ func TestGetCachedArchive(t *testing.T) {
sourceURL := "https://raw.githubusercontent.com/drud/ddev/master/.gitignore"
exPath, archPath, err := GetCachedArchive("TestInvalidArchive", "test", "", sourceURL)
assert.Error(err)
assert.Contains(err.Error(), fmt.Sprintf("archive extraction of %s failed", archPath))
if err != nil {
assert.Contains(err.Error(), fmt.Sprintf("archive extraction of %s failed", archPath))
}

err = os.RemoveAll(filepath.Dir(exPath))
assert.NoError(err)

sourceURL = "http://invalid_domain/somefilethatdoesnotexists"
exPath, archPath, err = GetCachedArchive("TestInvalidDownloadURL", "test", "", sourceURL)
assert.Error(err)
assert.Contains(err.Error(), fmt.Sprintf("Failed to download url=%s into %s", sourceURL, archPath))
if err != nil {
assert.Contains(err.Error(), fmt.Sprintf("Failed to download url=%s into %s", sourceURL, archPath))
}

err = os.RemoveAll(filepath.Dir(exPath))
assert.NoError(err)
Expand Down

0 comments on commit 4d7d345

Please sign in to comment.