Enable all default golangci-lint linters and fix issues related (#37)

* turn on default linters and fix warnings/errors as a result Signed-off-by: Jordan Dubrick <[email protected]> * cleanup forgotten commented out block Signed-off-by: Jordan Dubrick <[email protected]> * potential increase in code cov for resp body closing Signed-off-by: Jordan Dubrick <[email protected]> * fix lint error in test case Signed-off-by: Jordan Dubrick <[email protected]> * revert redundant code coverage changes Signed-off-by: Jordan Dubrick <[email protected]> * update defer statements and add test cases for them Signed-off-by: Jordan Dubrick <[email protected]> * add test cases for detector.go Signed-off-by: Jordan Dubrick <[email protected]> * add unit tests for DownloadDevfileTypesFromRegistry Signed-off-by: Jordan Dubrick <[email protected]> * consolidate closinghttp code to reduce redundancy Signed-off-by: Jordan Dubrick <[email protected]> * update testing for httpclose to use mock server Signed-off-by: Jordan Dubrick <[email protected]> * add more unit tests for code coverage Signed-off-by: Jordan Dubrick <[email protected]> * reintroduce err handling for closehttpresponsebody to combat warning Signed-off-by: Jordan Dubrick <[email protected]> * mock error for closing http body Signed-off-by: Jordan Dubrick <[email protected]> --------- Signed-off-by: Jordan Dubrick <[email protected]>
devfile · Dec 11, 2023 · 9ed7d09 · 9ed7d09
1 parent 719af19
commit 9ed7d09
Show file tree

Hide file tree

Showing 16 changed files with 239 additions and 51 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -18,7 +18,7 @@ linters:
   # Default: false
   # TODO(rm3l): all default linters are disabled in the context of https://github.com/devfile/api/issues/1257 (to just enforce that io/ioutil is not used anymore),
   #   but we should think about enabling all the default linters and fix the issues reported.
-  disable-all: true
+  disable-all: false
   enable:
     # Go linter that checks if package imports are in a list of acceptable packages
     - depguard

diff --git a/pkg/apis/enricher/docker_enricher.go b/pkg/apis/enricher/docker_enricher.go
@@ -32,19 +32,16 @@ func (d DockerEnricher) GetSupportedLanguages() []string {
 
 func (d DockerEnricher) DoEnrichLanguage(language *model.Language, _ *[]string) {
 	// The Dockerfile language does not contain frameworks
-	return
 }
 
 func (d DockerEnricher) DoEnrichComponent(component *model.Component, _ model.DetectionSettings, _ *context.Context) {
 	projectName := GetDefaultProjectName(component.Path)
 	component.Name = projectName
 
-	var ports []int
-	ports = GetPortsFromDockerFile(component.Path)
+	ports := GetPortsFromDockerFile(component.Path)
 	if len(ports) > 0 {
 		component.Ports = ports
 	}
-	return
 }
 
 func (d DockerEnricher) IsConfigValidForComponentDetection(language string, config string) bool {

diff --git a/pkg/apis/enricher/enricher.go b/pkg/apis/enricher/enricher.go
@@ -126,11 +126,10 @@ func GetPortsFromDockerFile(root string) []int {
 		cleanFilePath := filepath.Clean(filePath)
 		file, err := os.Open(cleanFilePath)
 		if err == nil {
-			defer func() error {
+			defer func(){
 				if err := file.Close(); err != nil {
-					return fmt.Errorf("error closing file: %s", err)
+					fmt.Printf("error closing file: %s", err)
 				}
-				return nil
 			}()
 			return utils.ReadPortsFromDockerfile(file)
 		}

diff --git a/pkg/apis/enricher/framework/dotnet/dotnet_detector.go b/pkg/apis/enricher/framework/dotnet/dotnet_detector.go
@@ -64,20 +64,23 @@ func getFrameworks(configFilePath string) string {
 	cleanConfigPath := filepath.Clean(configFilePath)
 	xmlFile, err := os.Open(cleanConfigPath)
 	if err != nil {
+		fmt.Printf("error opening file: %s", err)
+		return ""
+	}
+	byteValue, err := io.ReadAll(xmlFile)
+	if err != nil {
+		fmt.Printf("error reading file: %s", err)
 		return ""
 	}
-	byteValue, _ := io.ReadAll(xmlFile)
-
 	var proj schema.DotNetProject
 	err = xml.Unmarshal(byteValue, &proj)
 	if err != nil {
 		return ""
 	}
-	defer func() error {
+	defer func(){
 		if err := xmlFile.Close(); err != nil {
-			return fmt.Errorf("error closing file: %s", err)
+			fmt.Printf("error closing file: %s", err)
 		}
-		return nil
 	}()
 	if proj.PropertyGroup.TargetFramework != "" {
 		return proj.PropertyGroup.TargetFramework

diff --git a/pkg/apis/enricher/framework/javascript/nodejs/nuxt_detector.go b/pkg/apis/enricher/framework/javascript/nodejs/nuxt_detector.go
@@ -45,7 +45,7 @@ func (n NuxtDetector) DoFrameworkDetection(language *model.Language, config stri
 
 // DoPortsDetection searches for the port in package.json, and nuxt.config.js
 func (n NuxtDetector) DoPortsDetection(component *model.Component, ctx *context.Context) {
-	ports := []int{}
+	var ports []int
 	regexes := []string{`--port=(\d*)`}
 	// check if port is set in start script in package.json
 	port := getPortFromStartScript(component.Path, regexes)

diff --git a/pkg/apis/enricher/framework/javascript/nodejs/vue_detector.go b/pkg/apis/enricher/framework/javascript/nodejs/vue_detector.go
@@ -46,7 +46,7 @@ func (v VueDetector) DoFrameworkDetection(language *model.Language, config strin
 // DoPortsDetection searches for the port in package.json, .env file, and vue.config.js
 func (v VueDetector) DoPortsDetection(component *model.Component, ctx *context.Context) {
 	regexes := []string{`--port (\d*)`, `PORT=(\d*)`}
-	ports := []int{}
+	var ports []int
 	// check if --port or PORT is set in start script in package.json
 	port := getPortFromStartScript(component.Path, regexes)
 	if utils.IsValidPort(port) {

diff --git a/pkg/apis/recognizer/devfile_recognizer.go b/pkg/apis/recognizer/devfile_recognizer.go
@@ -242,12 +242,8 @@ var DownloadDevfileTypesFromRegistry = func(url string, filter model.DevfileFilt
 	if err != nil {
 		return []model.DevfileType{}, err
 	}
-	defer func() error {
-		if err := resp.Body.Close(); err != nil {
-			return fmt.Errorf("error closing file: %s", err)
-		}
-		return nil
-	}()
+
+	defer utils.CloseHttpResponseBody(resp)
 
 	// Check server response
 	if resp.StatusCode != http.StatusOK {

diff --git a/pkg/apis/recognizer/devfile_recognizer_test.go b/pkg/apis/recognizer/devfile_recognizer_test.go
@@ -16,6 +16,9 @@ import (
 	"reflect"
 	"runtime"
 	"testing"
+	"net/http"
+	"net/http/httptest"
+	"encoding/json"
 
 	"github.com/devfile/alizer/pkg/apis/model"
 	"github.com/stretchr/testify/assert"
@@ -137,6 +140,69 @@ func TestDetectLaravelDevfile(t *testing.T) {
 	detectDevfiles(t, "laravel", []string{"php-laravel"})
 }
 
+func TestDownloadDevfileTypesFromRegistry(t *testing.T){
+
+	server := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request){
+			//Verify url was set properly
+			if r.URL.Path != "/v2index" {
+				t.Errorf("URL was incorrect, expected /v2index and got %s", r.URL.Path)
+			}
+
+			response := []model.DevfileType{model.DevfileType{
+				Name:        "mocked-stack",
+				Language:    "python",
+				ProjectType: "python",
+				Tags:        []string{"python"},
+				Versions:    []model.Version{},
+			}}
+
+			responseJSON, _ := json.Marshal(response)
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			_, err := w.Write(responseJSON)
+			assert.NoError(t, err)
+		}))
+	defer server.Close()
+
+
+	tests := []struct {
+		name                string
+		filter              model.DevfileFilter
+		expectedDevfileType []model.DevfileType
+		url					string
+		expectingErr 		bool
+	}{
+		{
+			name:   "Successful Download",
+			filter: model.DevfileFilter{},
+			expectedDevfileType: []model.DevfileType{model.DevfileType{
+				Name:        "mocked-stack",
+				Language:    "python",
+				ProjectType: "python",
+				Tags:        []string{"python"},
+				Versions:    []model.Version{},
+			}},
+			url: server.URL,
+			expectingErr: false,
+		},
+		{
+			name:   "Unsuccessful Download",
+			filter: model.DevfileFilter{},
+			expectedDevfileType: []model.DevfileType{},
+			url: "fake-path",
+			expectingErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			devfileTypes, _ := DownloadDevfileTypesFromRegistry(tt.url, tt.filter)
+			assert.EqualValues(t, devfileTypes, tt.expectedDevfileType)
+		})
+	}
+}
+
 func TestGetUrlWithVersions(t *testing.T) {
 	tests := []struct {
 		name             string
@@ -790,4 +856,4 @@ func getTestProjectPath(folder string) string {
 	_, b, _, _ := runtime.Caller(0)
 	basepath := filepath.Dir(b)
 	return filepath.Join(basepath, "..", "..", "..", "resources/projects", folder)
-}
+}
diff --git a/pkg/utils/detector.go b/pkg/utils/detector.go
@@ -26,6 +26,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"net/http"
 
 	"github.com/devfile/alizer/pkg/apis/model"
 	"github.com/devfile/alizer/pkg/schema"
@@ -134,19 +135,18 @@ func GetPomFileContent(pomFilePath string) (schema.Pom, error) {
 	if err != nil {
 		return schema.Pom{}, err
 	}
-	byteValue, _ := io.ReadAll(xmlFile)
-
+	byteValue, err := io.ReadAll(xmlFile)
+	if err != nil {
+		return schema.Pom{}, err
+	}
+
 	var pom schema.Pom
 	err = xml.Unmarshal(byteValue, &pom)
 	if err != nil {
 		return schema.Pom{}, err
 	}
-	defer func() error {
-		if err := xmlFile.Close(); err != nil {
-			return fmt.Errorf("error closing file: %s", err)
-		}
-		return nil
-	}()
+
+	defer CloseFile(xmlFile)
 	return pom, nil
 }
 
@@ -353,12 +353,7 @@ func GetEnvVarsFromDockerFile(root string) ([]model.EnvVar, error) {
 		cleanFilePath := filepath.Clean(filePath)
 		file, err := os.Open(cleanFilePath)
 		if err == nil {
-			defer func() error {
-				if err := file.Close(); err != nil {
-					return fmt.Errorf("error closing file: %s", err)
-				}
-				return nil
-			}()
+			defer CloseFile(file)
 			return readEnvVarsFromDockerfile(file)
 		}
 	}
@@ -751,3 +746,15 @@ func NormalizeSplit(file string) (string, string) {
 	}
 	return dir, fileName
 }
+
+func CloseHttpResponseBody(resp *http.Response){
+	if err := resp.Body.Close(); err != nil {
+		fmt.Printf("error closing file: %s", err)
+	}
+}
+
+func CloseFile(file *os.File){
+	if err := file.Close(); err != nil {
+		fmt.Printf("error closing file: %s", err)
+	}
+}
diff --git a/pkg/utils/detector_test.go b/pkg/utils/detector_test.go
@@ -7,6 +7,10 @@ import (
 	"reflect"
 	"regexp"
 	"testing"
+	"net/http"
+	"net/http/httptest"
+	"io"
+	"fmt"
 
 	"github.com/devfile/alizer/pkg/apis/model"
 	"github.com/devfile/alizer/pkg/schema"
@@ -591,7 +595,8 @@ func TestIsTagInPomXMLFile(t *testing.T) {
 
 func TestGetPomFileContent(t *testing.T) {
 	missingFileErr := "no such file or directory"
-
+	badXmlFileErr := "XML syntax error on line 1: expected attribute name in element"
+
 	testCases := []struct {
 		name           string
 		filePath       string
@@ -638,6 +643,12 @@ func TestGetPomFileContent(t *testing.T) {
 			expectedResult: schema.Pom{},
 			expectedError:  &missingFileErr,
 		},
+		{
+			name:           "Case 3: File is unreadable (cannot unmarshal)",
+			filePath:       "testdata/bad-xml.xml",
+			expectedResult: schema.Pom{},
+			expectedError:  &badXmlFileErr,
+		},
 	}
 
 	for _, tt := range testCases {
@@ -1948,3 +1959,104 @@ func TestGetApplicationFileInfo(t *testing.T) {
 		})
 	}
 }
+
+type errorBodyCloser struct {
+	io.Reader
+}
+
+func (ebc *errorBodyCloser) Close() error {
+	return fmt.Errorf("mocked error closing body")
+}
+
+func TestCloseHttpResponseBody(t *testing.T){
+	server := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request){
+			w.WriteHeader(http.StatusOK)
+			_, err := w.Write([]byte(`{"test": "values"}`))
+			assert.NoError(t, err)
+		}))
+	defer server.Close()
+
+	tests := []struct {
+		name                string
+		url					string
+		expectErr			bool
+		expectedOut			string
+	}{
+		{
+			name:   "Case 1: Successful Closing of File",
+			url: server.URL,
+			expectErr: false,
+			expectedOut: "",
+		},
+		{
+			name:   "Case 2: Failure Closing File",
+			url: server.URL,
+			expectErr: true,
+			expectedOut: "error closing file: mocked error closing body",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resp, err := http.Get(tt.url)
+			assert.Empty(t, err)
+
+			// Below section handles the capturing of the fmt.Printf in the func being tested
+			captureStdout := os.Stdout
+			r, w, _ := os.Pipe()
+			os.Stdout = w
+			if tt.expectErr {
+				resp = &http.Response{
+					Body: &errorBodyCloser{},
+				}
+			}
+			CloseHttpResponseBody(resp)
+			w.Close()
+			out, _ := io.ReadAll(r)
+			os.Stdout = captureStdout
+			assert.EqualValues(t, tt.expectedOut, string(out))
+
+		})
+	}
+}
+
+func TestCloseFile(t *testing.T){
+	tests := []struct {
+		name                string
+		expectErr			bool
+		expectedOut			string
+	}{
+		{
+			name:   "Case 1: Filed closed",
+			expectErr: false,
+			expectedOut: "",
+		},
+		{
+			name: "Case 2: File not closed",
+			expectErr: true,
+			expectedOut: "error closing file: close testdata/pom-dependency.xml: file already closed",
+		},
+	}
+
+	file_path := "testdata/pom-dependency.xml"
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			open_file, _ := os.Open(file_path)
+			// Below section handles the capturing of the fmt.Printf in the func being tested
+			captureStdout := os.Stdout
+			r, w, _ := os.Pipe()
+			os.Stdout = w
+			// Mocking the hit of a close failure by preclosing the file
+			if tt.expectErr {
+				open_file.Close()
+			}
+			CloseFile(open_file)
+			w.Close()
+			out, _ := io.ReadAll(r)
+			os.Stdout = captureStdout
+			assert.EqualValues(t, tt.expectedOut, string(out))
+		})
+	}
+}