Make configFetcher fallback work for gitea (#357)

#299 who closed #133 did not take into account, that that gitea (and eventually) other forges do return 200 and empty string if file was not found - this make configFetcher more resilient

server/shared/configFetcher.go

@@ -1,6 +1,7 @@
 package shared
 
 import (
+	"fmt"
 	"strings"
 	"time"
 
@@ -31,13 +32,13 @@ func (cf *configFetcher) Fetch() (files []*remote.FileMeta, err error) {
 	for i := 0; i < 5; i++ {
 		select {
 		case <-time.After(time.Second * time.Duration(i)):
-			if len(cf.repo.Config) > 0 {
+			if len(config) > 0 {
 				// either a file
 				if !strings.HasSuffix(config, "/") {
 					file, err = cf.remote_.File(cf.user, cf.repo, cf.build, config)
-					if err == nil {
+					if err == nil && len(file) != 0 {
 						return []*remote.FileMeta{{
-							Name: cf.repo.Config,
+							Name: config,
 							Data: file,
 						}}, nil
 					}
@@ -54,12 +55,13 @@ func (cf *configFetcher) Fetch() (files []*remote.FileMeta, err error) {
 				// test .woodpecker/ folder
 				// if folder is not supported we will get a "Not implemented" error and continue
 				files, err = cf.remote_.Dir(cf.user, cf.repo, cf.build, ".woodpecker")
-				if err == nil {
-					return filterPipelineFiles(files), nil
+				files = filterPipelineFiles(files)
+				if err == nil && len(files) != 0 {
+					return files, nil
 				}
 
 				file, err = cf.remote_.File(cf.user, cf.repo, cf.build, ".woodpecker.yml")
-				if err == nil {
+				if err == nil && len(file) != 0 {
 					return []*remote.FileMeta{{
 						Name: ".woodpecker.yml",
 						Data: file,
@@ -67,14 +69,19 @@ func (cf *configFetcher) Fetch() (files []*remote.FileMeta, err error) {
 				}
 
 				file, err = cf.remote_.File(cf.user, cf.repo, cf.build, ".drone.yml")
-				if err == nil {
+				if err == nil && len(file) != 0 {
 					return []*remote.FileMeta{{
 						Name: ".drone.yml",
 						Data: file,
 					}}, nil
 				}
+
+				if err == nil && len(files) == 0 {
+					return nil, fmt.Errorf("ConfigFetcher: Fallback did not found config")
+				}
 			}
 
+			// TODO: retry loop is inactive and could maybe be fixed/deleted
 			return nil, err
 		}
 	}

server/shared/configFetcher_test.go

@@ -5,31 +5,45 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/stretchr/testify/mock"
 	"github.com/woodpecker-ci/woodpecker/model"
 	"github.com/woodpecker-ci/woodpecker/server/remote"
 	"github.com/woodpecker-ci/woodpecker/server/remote/mocks"
 	"github.com/woodpecker-ci/woodpecker/server/shared"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 )
 
 func TestFetch(t *testing.T) {
 	t.Parallel()
 
+	type file struct {
+		name string
+		data []byte
+	}
+
+	dummyData := []byte("TEST")
+
 	testTable := []struct {
 		name              string
 		repoConfig        string
-		files             []string
+		files             []file
 		expectedFileNames []string
 		expectedError     bool
 	}{
 		{
 			name:       "Default config - .woodpecker/",
 			repoConfig: "",
-			files: []string{
-				".woodpecker/text.txt",
-				".woodpecker/release.yml",
-				".woodpecker/image.png",
-			},
+			files: []file{{
+				name: ".woodpecker/text.txt",
+				data: dummyData,
+			}, {
+				name: ".woodpecker/release.yml",
+				data: dummyData,
+			}, {
+				name: ".woodpecker/image.png",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".woodpecker/release.yml",
 			},
@@ -38,9 +52,10 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Default config - .woodpecker.yml",
 			repoConfig: "",
-			files: []string{
-				".woodpecker.yml",
-			},
+			files: []file{{
+				name: ".woodpecker.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".woodpecker.yml",
 			},
@@ -49,9 +64,10 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Default config - .drone.yml",
 			repoConfig: "",
-			files: []string{
-				".drone.yml",
-			},
+			files: []file{{
+				name: ".drone.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".drone.yml",
 			},
@@ -60,17 +76,20 @@ func TestFetch(t *testing.T) {
 		{
 			name:              "Default config - Empty repo",
 			repoConfig:        "",
-			files:             []string{},
+			files:             []file{},
 			expectedFileNames: []string{},
 			expectedError:     true,
 		},
 		{
 			name:       "Default config - Additional sub-folders",
 			repoConfig: "",
-			files: []string{
-				".woodpecker/test.yml",
-				".woodpecker/sub-folder/config.yml",
-			},
+			files: []file{{
+				name: ".woodpecker/test.yml",
+				data: dummyData,
+			}, {
+				name: ".woodpecker/sub-folder/config.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".woodpecker/test.yml",
 			},
@@ -79,25 +98,55 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Default config - Additional none .yml files",
 			repoConfig: "",
-			files: []string{
-				".woodpecker/notes.txt",
-				".woodpecker/image.png",
-				".woodpecker/test.yml",
-			},
+			files: []file{{
+				name: ".woodpecker/notes.txt",
+				data: dummyData,
+			}, {
+				name: ".woodpecker/image.png",
+				data: dummyData,
+			}, {
+				name: ".woodpecker/test.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".woodpecker/test.yml",
 			},
 			expectedError: false,
 		},
+		{
+			name:       "Default config - Empty Folder",
+			repoConfig: " ",
+			files: []file{{
+				name: ".woodpecker/.keep",
+				data: dummyData,
+			}, {
+				name: ".woodpecker.yml",
+				data: nil,
+			}, {
+				name: ".drone.yml",
+				data: dummyData,
+			}},
+			expectedFileNames: []string{
+				".drone.yml",
+			},
+			expectedError: false,
+		},
 		{
 			name:       "Special config - folder (ignoring default files)",
 			repoConfig: ".my-ci-folder/",
-			files: []string{
-				".woodpecker/test.yml",
-				".woodpecker.yml",
-				".drone.yml",
-				".my-ci-folder/test.yml",
-			},
+			files: []file{{
+				name: ".woodpecker/test.yml",
+				data: dummyData,
+			}, {
+				name: ".woodpecker.yml",
+				data: dummyData,
+			}, {
+				name: ".drone.yml",
+				data: dummyData,
+			}, {
+				name: ".my-ci-folder/test.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".my-ci-folder/test.yml",
 			},
@@ -106,9 +155,10 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Special config - folder",
 			repoConfig: ".my-ci-folder/",
-			files: []string{
-				".my-ci-folder/test.yml",
-			},
+			files: []file{{
+				name: ".my-ci-folder/test.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".my-ci-folder/test.yml",
 			},
@@ -117,9 +167,10 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Special config - subfolder",
 			repoConfig: ".my-ci-folder/my-config/",
-			files: []string{
-				".my-ci-folder/my-config/test.yml",
-			},
+			files: []file{{
+				name: ".my-ci-folder/my-config/test.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".my-ci-folder/my-config/test.yml",
 			},
@@ -128,9 +179,10 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Special config - file",
 			repoConfig: ".config.yml",
-			files: []string{
-				".config.yml",
-			},
+			files: []file{{
+				name: ".config.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".config.yml",
 			},
@@ -139,9 +191,10 @@ func TestFetch(t *testing.T) {
 		{
 			name:       "Special config - file inside subfolder",
 			repoConfig: ".my-ci-folder/sub-folder/config.yml",
-			files: []string{
-				".my-ci-folder/sub-folder/config.yml",
-			},
+			files: []file{{
+				name: ".my-ci-folder/sub-folder/config.yml",
+				data: dummyData,
+			}},
 			expectedFileNames: []string{
 				".my-ci-folder/sub-folder/config.yml",
 			},
@@ -150,7 +203,7 @@ func TestFetch(t *testing.T) {
 		{
 			name:              "Special config - empty repo",
 			repoConfig:        ".config.yml",
-			files:             []string{},
+			files:             []file{},
 			expectedFileNames: []string{},
 			expectedError:     true,
 		},
@@ -163,12 +216,14 @@ func TestFetch(t *testing.T) {
 			r := new(mocks.Remote)
 			dirs := map[string][]*remote.FileMeta{}
 			for _, file := range tt.files {
-				r.On("File", mock.Anything, mock.Anything, mock.Anything, file).Return([]byte{}, nil)
-				path := filepath.Dir(file)
-				dirs[path] = append(dirs[path], &remote.FileMeta{
-					Name: file,
-					Data: []byte{},
-				})
+				r.On("File", mock.Anything, mock.Anything, mock.Anything, file.name).Return(file.data, nil)
+				path := filepath.Dir(file.name)
+				if path != "." {
+					dirs[path] = append(dirs[path], &remote.FileMeta{
+						Name: file.name,
+						Data: file.data,
+					})
+				}
 			}
 
 			for path, files := range dirs {
@@ -192,22 +247,11 @@ func TestFetch(t *testing.T) {
 				t.Fatal("error fetching config:", err)
 			}
 
-			matchingFiles := 0
-			for _, expectedFileName := range tt.expectedFileNames {
-				for _, file := range files {
-					if file.Name == expectedFileName {
-						matchingFiles += 1
-					}
-				}
-			}
-
-			if matchingFiles != len(tt.expectedFileNames) {
-				var receivedFileNames []string
-				for _, file := range files {
-					receivedFileNames = append(receivedFileNames, file.Name)
-				}
-				t.Fatal("expected some other pipeline files", tt.expectedFileNames, receivedFileNames)
+			matchingFiles := make([]string, len(files))
+			for i := range files {
+				matchingFiles[i] = files[i].Name
 			}
+			assert.ElementsMatch(t, tt.expectedFileNames, matchingFiles, "expected some other pipeline files")
 		})
 	}
 }