WIP: Addressing findings from linters/checkers/ci

application/application_db_calls_bench_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	_ "github.com/lib/pq"
+	"github.com/stretchr/testify/require"
 
 	"github.com/fabric8-services/fabric8-wit/application"
 	"github.com/fabric8-services/fabric8-wit/gormapplication"
@@ -125,7 +126,8 @@ func (s *BenchDbOperations) BenchmarkGormSelectSpaceNameRaw() {
 		defer result.Close()
 		for result.Next() {
 			var wit string
-			result.Scan(&wit)
+			err := result.Scan(&wit)
+			require.NoError(s.B(), err)
 			names = append(names, wit)
 		}
 	}
@@ -195,11 +197,12 @@ func (s *BenchDbOperations) BenchmarkGormSelectSpaceRaw() {
 		defer result.Close()
 		for result.Next() {
 			var sp space.Space
-			result.Scan(
+			err := result.Scan(
 				&sp.Version,
 				&sp.Name,
 				&sp.Description,
 				&sp.OwnerID)
+			require.NoError(s.B(), err)
 			sps = append(sps, sp)
 		}
 	}

application/transaction.go

@@ -1,6 +1,7 @@
 package application
 
 import (
+	"context"
 	"runtime/debug"
 	"time"
 
@@ -46,15 +47,27 @@ func Transactional(db DB, todo func(f Application) error) error {
 		case err := <-errorChan:
 			if err != nil {
 				log.Debug(nil, map[string]interface{}{"error": err}, "Rolling back the transaction...")
-				tx.Rollback()
+				errRollback := tx.Rollback()
+				if errRollback != nil {
+					log.Error(context.Background(), map[string]interface{}{
+						"errRollback": errors.WithStack(errRollback),
+						"err":         errors.WithStack(err),
+					}, "failed to rollback transaction: %+v", errRollback)
+				}
 				log.Error(nil, map[string]interface{}{
 					"err": err,
 				}, "database transaction failed!")
 				return errors.WithStack(err)
 			}
 
-			tx.Commit()
-			log.Debug(nil, nil, "Commit the transaction!")
+			log.Debug(nil, nil, "Committing the transaction!")
+			errCommit := tx.Commit()
+			if errCommit != nil {
+				log.Error(context.Background(), map[string]interface{}{
+					"errCommit": errors.WithStack(errCommit),
+				}, "failed to commit transaction: %+v", errCommit)
+			}
+
 			return nil
 		case <-txTimeout:
 			log.Debug(nil, nil, "Rolling back the transaction...")

application/transaction_bench_test.go

@@ -1,7 +1,6 @@
 package application_test
 
 import (
-	"database/sql"
 	"testing"
 
 	_ "github.com/lib/pq"
@@ -17,7 +16,6 @@ type BenchTransactional struct {
 	gormbench.DBBenchSuite
 	repo  space.Repository
 	appDB application.DB
-	dbPq  *sql.DB
 }
 
 func BenchmarkRunTransactional(b *testing.B) {

codebase/analytics-gemini/client_blackbox_test.go

@@ -28,7 +28,10 @@ func TestRegister(t *testing.T) {
 			testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"),
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{
 			Transport: r.Transport,
@@ -50,7 +53,10 @@ func TestRegister(t *testing.T) {
 			"../../test/data/gemini-scan/register-401",
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{
 			Transport: r.Transport,
@@ -102,7 +108,10 @@ func TestRegister(t *testing.T) {
 			testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"),
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{
 			Transport: r.Transport,
@@ -128,7 +137,10 @@ func TestRegister(t *testing.T) {
 			testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"),
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{
 			Transport: r.Transport,
@@ -165,7 +177,10 @@ func TestDeRegister(t *testing.T) {
 			"../../test/data/gemini-scan/deregister-200",
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{Transport: r.Transport}
 		cli := gemini.NewScanRepoClient(geminiURL, httpClient, codebaseURL, httpClient, false)
@@ -187,15 +202,21 @@ func TestDeRegister(t *testing.T) {
 			"../../test/data/gemini-scan/deregister-call-gemini-codebases-200",
 		)
 		require.NoError(t, err)
-		defer recordCodebases.Stop()
+		defer func() {
+			err := recordCodebases.Stop()
+			require.NoError(t, err)
+		}()
 		codebaseClient := &http.Client{Transport: recordCodebases.Transport}
 
 		recordGemini, err := testrecorder.New(
 			"../../test/data/gemini-scan/deregister-call-gemini-200",
 			testrecorder.WithJWTMatcher("../../test/jwt/public_key.pem"),
 		)
 		require.NoError(t, err)
-		defer recordGemini.Stop()
+		defer func() {
+			err := recordGemini.Stop()
+			require.NoError(t, err)
+		}()
 		geminiClient := &http.Client{Transport: recordGemini.Transport}
 
 		cli := gemini.NewScanRepoClient(geminiURL, geminiClient, codebaseURL, codebaseClient, false)
@@ -242,7 +263,10 @@ func TestDeRegister(t *testing.T) {
 			"../../test/data/gemini-scan/deregister-400",
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{Transport: r.Transport}
 		cli := gemini.NewScanRepoClient(geminiURL, httpClient, codebaseURL, httpClient, false)
@@ -263,7 +287,10 @@ func TestDeRegister(t *testing.T) {
 			"../../test/data/gemini-scan/deregister-bad-output",
 		)
 		require.NoError(t, err)
-		defer r.Stop()
+		defer func() {
+			err := r.Stop()
+			require.NoError(t, err)
+		}()
 
 		httpClient := &http.Client{Transport: r.Transport}
 		cli := gemini.NewScanRepoClient(geminiURL, httpClient, codebaseURL, httpClient, false)

codebase/che/client.go

@@ -448,7 +448,7 @@ func (w WorkspaceResponse) GetHrefByRelOfWorkspaceLink(rel string) string {
 
 // Following const define commonly used WorkspaceLink rel
 const (
-	IdeUrlRel   = "ide url"
+	IdeURLRel   = "ide url"
 	SelfLinkRel = "self link"
 )
 

codebase/codebase.go

@@ -283,7 +283,10 @@ func (m *GormCodebaseRepository) List(ctx context.Context, spaceID uuid.UUID, st
 
 	for rows.Next() {
 		value := Codebase{}
-		db.ScanRows(rows, &value)
+		err := db.ScanRows(rows, &value)
+		if err != nil {
+			return nil, 0, errs.Wrapf(err, "failed to scan codebase rows")
+		}
 		if first {
 			first = false
 			if err = rows.Scan(columnValues...); err != nil {
@@ -300,10 +303,14 @@ func (m *GormCodebaseRepository) List(ctx context.Context, spaceID uuid.UUID, st
 		rows2, err := orgDB.Rows()
 		defer closeable.Close(ctx, rows2)
 		if err != nil {
-			return nil, 0, err
+			return nil, 0, errs.WithStack(err)
 		}
 		rows2.Next() // count(*) will always return a row
-		rows2.Scan(&count)
+		err = rows2.Scan(&count)
+		if err != nil {
+			return nil, 0, errs.WithStack(err)
+		}
+
 	}
 	return result, count, nil
 }
@@ -384,7 +391,10 @@ func (m *GormCodebaseRepository) SearchByURL(ctx context.Context, url string, st
 	first := true
 	for rows.Next() {
 		value := Codebase{}
-		db.ScanRows(rows, &value)
+		err := db.ScanRows(rows, &value)
+		if err != nil {
+			return nil, 0, errors.NewInternalError(ctx, err)
+		}
 		if first {
 			first = false
 			if err = rows.Scan(columnValues...); err != nil {

comment/comment_repository.go

@@ -175,7 +175,10 @@ func (m *GormCommentRepository) List(ctx context.Context, parentID uuid.UUID, st
 
 	for rows.Next() {
 		value := &Comment{}
-		db.ScanRows(rows, value)
+		err := db.ScanRows(rows, value)
+		if err != nil {
+			return nil, 0, errors.NewInternalError(ctx, err)
+		}
 		if first {
 			first = false
 			if err = rows.Scan(columnValues...); err != nil {
@@ -191,10 +194,13 @@ func (m *GormCommentRepository) List(ctx context.Context, parentID uuid.UUID, st
 		rows2, err := orgDB.Rows()
 		defer closeable.Close(ctx, rows2)
 		if err != nil {
-			return nil, 0, err
+			return nil, 0, errs.WithStack(err)
 		}
 		rows2.Next() // count(*) will always return a row
-		rows2.Scan(&count)
+		err = rows2.Scan(&count)
+		if err != nil {
+			return nil, 0, errors.NewInternalError(ctx, err)
+		}
 	}
 	return result, count, nil
 }

comment/comment_repository_blackbox_test.go

@@ -56,16 +56,18 @@ func (s *TestCommentRepository) TestCreateCommentWithParentComment() {
 	// parent comment
 	fxt := tf.NewTestFixture(s.T(), s.DB, tf.Identities(1))
 	parentComment := newComment(uuid.NewV4(), "Test A", rendering.SystemMarkupMarkdown)
-	s.repo.Create(s.Ctx, parentComment, fxt.Identities[0].ID)
+	err := s.repo.Create(s.Ctx, parentComment, fxt.Identities[0].ID)
+	require.NoError(s.T(), err)
 	// child comments
 	childComment := newComment(uuid.NewV4(), "Test Child A", rendering.SystemMarkupMarkdown)
 	childComment.ParentCommentID = id.NullUUID{
 		UUID:  parentComment.ID,
 		Valid: true,
 	}
 	// when
-	s.repo.Create(s.Ctx, childComment, fxt.Identities[0].ID)
+	err = s.repo.Create(s.Ctx, childComment, fxt.Identities[0].ID)
 	// then
+	require.NoError(s.T(), err)
 	require.NotNil(s.T(), childComment.ID, "Comment was not created, ID nil")
 	require.NotNil(s.T(), childComment.CreatedAt, "Comment was not created")
 	require.False(s.T(), childComment.CreatedAt.After(time.Now()), "Comment was not created, CreatedAt after Now()")
@@ -74,7 +76,7 @@ func (s *TestCommentRepository) TestCreateCommentWithParentComment() {
 	// now retrieving the stored child comment again and see if the parent reference was stored
 	var resultComment *comment.Comment
 	// when
-	resultComment, err := s.repo.Load(s.Ctx, childComment.ID)
+	resultComment, err = s.repo.Load(s.Ctx, childComment.ID)
 	// then
 	require.NoError(s.T(), err)
 	require.NotNil(s.T(), resultComment.ParentCommentID, "Parent comment id was not set, ID nil")
@@ -86,8 +88,9 @@ func (s *TestCommentRepository) TestCreateCommentWithMarkup() {
 	fxt := tf.NewTestFixture(s.T(), s.DB, tf.Identities(1))
 	comment := newComment(uuid.NewV4(), "Test A", rendering.SystemMarkupMarkdown)
 	// when
-	s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID)
+	err := s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID)
 	// then
+	require.NoError(s.T(), err)
 	assert.NotNil(s.T(), comment.ID, "Comment was not created, ID nil")
 	require.NotNil(s.T(), comment.CreatedAt, "Comment was not created?")
 	assert.False(s.T(), comment.CreatedAt.After(time.Now()), "Comment was not created, CreatedAt after Now()?")
@@ -98,8 +101,9 @@ func (s *TestCommentRepository) TestCreateCommentWithoutMarkup() {
 	fxt := tf.NewTestFixture(s.T(), s.DB, tf.Identities(1))
 	comment := newComment(uuid.NewV4(), "Test A", "")
 	// when
-	s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID)
+	err := s.repo.Create(s.Ctx, comment, fxt.Identities[0].ID)
 	// then
+	require.NoError(s.T(), err)
 	assert.NotNil(s.T(), comment.ID, "Comment was not created, ID nil")
 	require.NotNil(s.T(), comment.CreatedAt, "Comment was not created?")
 	assert.False(s.T(), comment.CreatedAt.After(time.Now()), "CreatedAt after Now()?")
@@ -116,7 +120,8 @@ func (s *TestCommentRepository) TestSaveCommentWithMarkup() {
 	// when
 	comment.Body = "Test AB"
 	comment.Markup = rendering.SystemMarkupMarkdown
-	s.repo.Save(s.Ctx, comment, comment.Creator)
+	err := s.repo.Save(s.Ctx, comment, comment.Creator)
+	require.NoError(s.T(), err)
 	offset := 0
 	limit := 1
 	comments, _, err := s.repo.List(s.Ctx, comment.ParentID, &offset, &limit)
@@ -137,7 +142,8 @@ func (s *TestCommentRepository) TestSaveCommentWithoutMarkup() {
 	// when
 	comment.Body = "Test AB"
 	comment.Markup = ""
-	s.repo.Save(s.Ctx, comment, comment.Creator)
+	err := s.repo.Save(s.Ctx, comment, comment.Creator)
+	require.NoError(s.T(), err)
 	offset := 0
 	limit := 1
 	comments, _, err := s.repo.List(s.Ctx, comment.ParentID, &offset, &limit)

configuration/configuration_blackbox_test.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 	"strconv"
-	"strings"
 	"testing"
 
 	"net/http"
@@ -13,7 +12,6 @@ import (
 
 	"github.com/fabric8-services/fabric8-wit/configuration"
 	"github.com/fabric8-services/fabric8-wit/resource"
-	"github.com/satori/go.uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -183,30 +181,6 @@ func TestGetMaxHeaderSizeSetByEnvVaribaleOK(t *testing.T) {
 	assert.Equal(t, envValue, viperValue)
 }
 
-func generateEnvKey(yamlKey string) string {
-	return "F8_" + strings.ToUpper(strings.Replace(yamlKey, ".", "_", -1))
-}
-
-func checkGetKeycloakEndpointSetByEnvVaribaleOK(t *testing.T, envName string, getEndpoint func(req *http.Request) (string, error)) {
-	envValue := uuid.NewV4().String()
-	env := os.Getenv(envName)
-	defer func() {
-		os.Setenv(envName, env)
-		resetConfiguration(defaultValuesConfigFilePath)
-	}()
-
-	os.Setenv(envName, envValue)
-	resetConfiguration(defaultValuesConfigFilePath)
-
-	url, err := getEndpoint(reqLong)
-	require.NoError(t, err)
-	require.Equal(t, envValue, url)
-
-	url, err = getEndpoint(reqShort)
-	require.NoError(t, err)
-	require.Equal(t, envValue, url)
-}
-
 func TestGetDeploymentsTimeoutDefault(t *testing.T) {
 	resource.Require(t, resource.UnitTest)
 	viperValue := config.GetDeploymentsHTTPTimeoutSeconds()