Merge pull request #80 from lukasjarosch/feat/github-action-test

Feat/GitHub action test

.github/workflows/test.yaml

@@ -1,39 +1,47 @@
 on: [push, pull_request]
-name: Test
+name: skipper-test
+
+env:
+  GO_VERSION: "1.21.0"
+  GO_VERSION_SHORT: "1.21"
+  GOLANGCI_VERSION: "v1.54.0"
+
+permissions:
+  contents: read
+
 jobs:
-  test:
-    strategy:
-      matrix:
-        go-version: [1.20.x, 1.21.x]
-        os: [ubuntu-latest, macos-latest]
-    runs-on: ${{ matrix.os }}
+  vet:
+    name: "go vet"
+    runs-on: ubuntu-latest
     steps:
-    - uses: actions/setup-go@v3
-      with:
-        go-version: ${{ matrix.go-version }}
-    - uses: actions/checkout@v3
-    - run: go test ./...
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+      - run: go vet $(go list ./... | grep -v /vendor/)
 
-  test-cache:
+  golangci-lint:
+    name: golangci-lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+      - uses: actions/setup-go@v5
+        with:
+          cache: false
+          go-version: ${{ env.GO_VERSION }}
+      - uses: golangci/golangci-lint-action@v4
+        with:
+          version: ${{ env.GOLANGCI_VERSION }}
+
+  test:
+    name: "go test"
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/setup-go@v3
-      with:
-        go-version: 1.19.x
-    - uses: actions/checkout@v3
-    - uses: actions/cache@v3
-      with:
-        # In order:
-        # * Module download cache
-        # * Build cache (Linux)
-        # * Build cache (Mac)
-        # * Build cache (Windows)
-        path: |
-          ~/go/pkg/mod
-          ~/.cache/go-build
-          ~/Library/Caches/go-build
-          ~\AppData\Local\go-build
-        key: ${{ runner.os }}-go-${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }}
-        restore-keys: |
-          ${{ runner.os }}-go-${{ matrix.go-version }}-
-    - run: go test ./...
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+      - uses: actions/checkout@v4
+      - run: go test $(go list ./... | grep -v /vendor/)

class.go

@@ -126,10 +126,7 @@ func (c Class) GetPath(path data.Path) (data.Value, error) {
 // HasPath returns true if the given path exists within the Class.
 func (c Class) HasPath(path data.Path) bool {
 	_, err := c.container.GetPath(path)
-	if err != nil {
-		return false
-	}
-	return true
+	return err == nil
 }
 
 // GetAll returns the whole data represented by this class.
@@ -227,7 +224,7 @@ func (c *Class) WalkValues(walkFunc func(data.Path, data.Value) error) error {
 func (c *Class) Values() map[string]data.Value {
 	valueMap := make(map[string]data.Value)
 
-	c.WalkValues(func(p data.Path, v data.Value) error {
+	_ = c.WalkValues(func(p data.Path, v data.Value) error {
 		valueMap[p.String()] = v
 		return nil
 	})

data/container.go

@@ -214,7 +214,7 @@ func (container *Container) HasPath(path Path) bool {
 // The function does not check whether the path is valid within the container.
 // This is to allow setting new paths, which do not yet exist.
 func (container *Container) AbsolutePath(path Path) (Path, error) {
-	if path == nil || len(path) == 0 {
+	if len(path) == 0 {
 		return nil, ErrEmptyPath
 	}
 

data/container_test.go

@@ -170,6 +170,7 @@ func TestContainer_Set(t *testing.T) {
 	assert.NotNil(t, container)
 
 	err = container.SetPath(NewPath("test.array.1.0"), []interface{}{1, 2, 3})
+	assert.NoError(t, err)
 }
 
 func TestContainer_Merge(t *testing.T) {

data/data.go

@@ -382,10 +382,7 @@ var SelectAllPaths PathSelectorFunc = func(_ interface{}, path Path, isLeaf bool
 }
 
 var SelectLeafPaths PathSelectorFunc = func(_ interface{}, path Path, isLeaf bool) bool {
-	if isLeaf {
-		return true
-	}
-	return false
+	return isLeaf
 }
 
 func Paths(data map[string]interface{}, selectorFn PathSelectorFunc) []Path {

data/data_test.go

@@ -4,8 +4,9 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/lukasjarosch/skipper/data"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/lukasjarosch/skipper/data"
 )
 
 func TestWalk(t *testing.T) {
@@ -186,7 +187,6 @@ func TestWalk(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-
 			// we need to store the paths in a map to prevent the 'append' behavior
 			// where the last element is overwritten by the next
 			pathMap := make(map[string]bool)
@@ -211,8 +211,6 @@ func TestWalk(t *testing.T) {
 			assert.NoError(t, err)
 		})
 	}
-
-	return
 }
 
 func TestGet(t *testing.T) {
@@ -224,7 +222,6 @@ func TestGet(t *testing.T) {
 		errExpected bool
 		err         error
 	}{
-
 		{
 			name:        "nil data",
 			data:        nil,
@@ -503,7 +500,6 @@ func TestSet(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-
 			returned, err := data.Set(tt.input, tt.key, tt.value)
 
 			if tt.errExpected {
@@ -824,7 +820,6 @@ func TestDeepSet(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-
 			returned, err := data.DeepSet(tt.input, tt.path, tt.value)
 
 			if tt.errExpected {
@@ -853,7 +848,8 @@ func TestPaths_SelectAll(t *testing.T) {
 			},
 			expected: []data.Path{
 				data.NewPath("foo"),
-				data.NewPath("baz")},
+				data.NewPath("baz"),
+			},
 		},
 		{
 			name: "NestedMap",
@@ -866,21 +862,24 @@ func TestPaths_SelectAll(t *testing.T) {
 			expected: []data.Path{
 				data.NewPath("parent"),
 				data.NewPath("parent.foo"),
-				data.NewPath("parent.baz")},
+				data.NewPath("parent.baz"),
+			},
 		},
 		{
 			name: "ArrayWithNestedMap",
 			input: map[string]interface{}{
 				"items": []interface{}{
 					map[string]interface{}{"name": "item1"},
-					map[string]interface{}{"name": "item2"}},
+					map[string]interface{}{"name": "item2"},
+				},
 			},
 			expected: []data.Path{
 				data.NewPath("items"),
 				data.NewPath("items.0"),
 				data.NewPath("items.0.name"),
 				data.NewPath("items.1"),
-				data.NewPath("items.1.name")},
+				data.NewPath("items.1.name"),
+			},
 		},
 		{
 			name: "SliceOfSlices",
@@ -945,7 +944,6 @@ func TestPaths_SelectAll(t *testing.T) {
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-
 			result := data.Paths(tt.input, data.SelectAllPaths)
 
 			data.SortPaths(result)
@@ -970,7 +968,8 @@ func TestPaths_SelectLeaves(t *testing.T) {
 			},
 			expected: []data.Path{
 				data.NewPath("foo"),
-				data.NewPath("baz")},
+				data.NewPath("baz"),
+			},
 		},
 		{
 			name: "NestedMap",
@@ -982,18 +981,21 @@ func TestPaths_SelectLeaves(t *testing.T) {
 			},
 			expected: []data.Path{
 				data.NewPath("parent.foo"),
-				data.NewPath("parent.baz")},
+				data.NewPath("parent.baz"),
+			},
 		},
 		{
 			name: "ArrayWithNestedMap",
 			input: map[string]interface{}{
 				"items": []interface{}{
 					map[string]interface{}{"name": "item1"},
-					map[string]interface{}{"name": "item2"}},
+					map[string]interface{}{"name": "item2"},
+				},
 			},
 			expected: []data.Path{
 				data.NewPath("items.0.name"),
-				data.NewPath("items.1.name")},
+				data.NewPath("items.1.name"),
+			},
 		},
 		{
 			name: "SliceOfSlices",
@@ -1051,7 +1053,6 @@ func TestPaths_SelectLeaves(t *testing.T) {
 	// Run test cases
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-
 			result := data.Paths(tt.input, data.SelectLeafPaths)
 
 			data.SortPaths(result)

fs.go

@@ -36,7 +36,7 @@ func DiscoverFiles(rootPath string, pathSelector *regexp.Regexp) ([]string, erro
 	}
 
 	var files []string
-	filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error {
+	err = filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error {
 		if info.IsDir() {
 			return nil
 		}
@@ -47,6 +47,9 @@ func DiscoverFiles(rootPath string, pathSelector *regexp.Regexp) ([]string, erro
 
 		return nil
 	})
+	if err != nil {
+		return nil, err
+	}
 
 	return files, nil
 }

inventory.go

@@ -165,10 +165,10 @@ func (inv *Inventory) WalkValues(walkFunc func(data.Path, data.Value) error) err
 // The context can be any path within the class to which the path is relative to.
 // In case the paths are empty or are not valid within the given context, an error is returned.
 func (inv *Inventory) AbsolutePath(path data.Path, context data.Path) (data.Path, error) {
-	if path == nil || len(path) == 0 {
+	if len(path) == 0 {
 		return nil, data.ErrEmptyPath
 	}
-	if context == nil || len(context) == 0 {
+	if len(context) == 0 {
 		return nil, fmt.Errorf("context path cannot be empty: %w", data.ErrEmptyPath)
 	}
 

reference/value.go

@@ -153,7 +153,7 @@ type ValueTarget interface {
 
 // ReplaceValues will replace all given references within the given ValueReferenceTarget.
 func ReplaceValues(target ValueTarget, references []ValueReference) error {
-	if references == nil || len(references) == 0 {
+	if len(references) == 0 {
 		return nil
 	}
 	if target == nil {
@@ -366,10 +366,10 @@ func ValueReplacementOrder(dependencyGraph graph.Graph[string, ValueReference])
 // If the order is empty/nil, then allReferences is just re-emitted.
 // If allReferences is empty/nil, then nil is returned.
 func ReorderValueReferences(order []ValueReference, allReferences []ValueReference) []ValueReference {
-	if order == nil || len(order) == 0 {
+	if len(order) == 0 {
 		return allReferences
 	}
-	if allReferences == nil || len(allReferences) == 0 {
+	if len(allReferences) == 0 {
 		return nil
 	}
 

reference_test.go

@@ -126,7 +126,7 @@ func TestValueManager_SetHooks_Registry(t *testing.T) {
 	})
 	t.Run("registering a new class with invalid references must fail", func(t *testing.T) {
 		registry := NewRegistry()
-		NewValueReferenceManager(registry)
+		_, _ = NewValueReferenceManager(registry)
 
 		// the common class is the only class, hence some references are invalid
 		common, err := NewClass("testdata/references/registry/common.yaml", codec.NewYamlCodec(), data.NewPath("common"))

registry.go

@@ -80,7 +80,7 @@ func (reg *Registry) RegisterClass(class *Class) error {
 
 	// Create a slice of all absolute paths defined by the class
 	var classPaths []string
-	class.Walk(func(path data.Path, _ data.Value, _ bool) error {
+	_ = class.Walk(func(path data.Path, _ data.Value, _ bool) error {
 		classPaths = append(classPaths, pathPrefix.AppendPath(path).String())
 		return nil
 	})
@@ -137,7 +137,7 @@ func (reg *Registry) classPreSetHook(class *Class) SetHookFunc {
 		// if the path does already exist and is owned by a *different* class, then
 		// we need to prevent the Set call as it would introduce a duplicate path.
 		if classIdentifier, exists := reg.paths[registryPath.String()]; exists {
-			existingPathClass, _ := reg.classes[classIdentifier]
+			existingPathClass := reg.classes[classIdentifier]
 			if !existingPathClass.Identifier.Equals(class.Identifier) {
 				return fmt.Errorf("path already owned by '%s': %w: %s", classIdentifier, ErrDuplicatePath, registryPath)
 			}
@@ -206,10 +206,7 @@ func (reg *Registry) GetPath(path data.Path) (data.Value, error) {
 // HasPath returns true if the path exists within the registry.
 func (reg *Registry) HasPath(path data.Path) bool {
 	_, err := reg.GetPath(path)
-	if err != nil {
-		return false
-	}
-	return true
+	return err == nil
 }
 
 // GetClassRelativePath attempts to resolve the target-class using the given classPath. Then it attempts to resolve the path (which might be class-local only).
@@ -265,7 +262,7 @@ func (reg *Registry) WalkValues(walkFunc func(data.Path, data.Value) error) erro
 func (reg *Registry) Values() map[string]data.Value {
 	valueMap := make(map[string]data.Value)
 
-	reg.WalkValues(func(p data.Path, v data.Value) error {
+	_ = reg.WalkValues(func(p data.Path, v data.Value) error {
 		valueMap[p.String()] = v
 		return nil
 	})
@@ -304,10 +301,10 @@ func (reg *Registry) ClassIdentifiers() []data.Path {
 // The context can be any path within the class to which the path is relative to, even just the classIdentifier.
 // In case the paths are empty or are not valid within the given context, an error is returned.
 func (reg *Registry) AbsolutePath(path data.Path, context data.Path) (data.Path, error) {
-	if path == nil || len(path) == 0 {
+	if len(path) == 0 {
 		return nil, data.ErrEmptyPath
 	}
-	if context == nil || len(context) == 0 {
+	if len(context) == 0 {
 		return nil, fmt.Errorf("context path cannot be empty: %w", data.ErrEmptyPath)
 	}
 

registry_test.go

@@ -20,8 +20,6 @@ var (
 	janeClassPath       = "testdata/classes/people/jane.yaml"
 	peopleClassPath     = "testdata/classes/people.yaml"
 
-	stripPrefix = data.NewPathFromOsPath("testdata/classes")
-
 	personClass,
 	pizzaClass,
 	foodCommonClass,