From 0262785da13a1fd65060fca6c09014c346e96b98 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:15:27 +0100 Subject: [PATCH 01/10] feat: test marketplace test action --- .github/workflows/test.yaml | 37 ++++++++----------------------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1d22ad4..4ab807f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -8,32 +8,11 @@ jobs: os: [ubuntu-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - - uses: actions/setup-go@v3 - with: - go-version: ${{ matrix.go-version }} - - uses: actions/checkout@v3 - - run: go test ./... - - test-cache: - 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 ./... + - name: Setup Go + uses: actions/setup-go@v3 + with: + go-version: ${{ matrix.go-version }} + - name: Checkout + uses: actions/checkout@v3 + - name: Run tests + uses: robherley/go-test-action@v0.1.0 From b30068b5e304fee1be20407ac9f13dea7427624f Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:21:13 +0100 Subject: [PATCH 02/10] feat: bump action version --- .github/workflows/test.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 4ab807f..a200718 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -9,10 +9,10 @@ jobs: runs-on: ${{ matrix.os }} steps: - name: Setup Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: go-version: ${{ matrix.go-version }} - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Run tests uses: robherley/go-test-action@v0.1.0 From e8ca92e6d7de0b3302f5c026536f35f01fb472a4 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:24:14 +0100 Subject: [PATCH 03/10] feat: bump setup-go --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a200718..0b70426 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -9,7 +9,7 @@ jobs: runs-on: ${{ matrix.os }} steps: - name: Setup Go - uses: actions/setup-go@v4 + uses: actions/setup-go@v4.1.0 with: go-version: ${{ matrix.go-version }} - name: Checkout From d58d9452bb9c4f600ec0acaaef954a3478aeb52f Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:30:01 +0100 Subject: [PATCH 04/10] feat: add golangci-lint --- .github/workflows/golangci-lint.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .github/workflows/golangci-lint.yaml diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml new file mode 100644 index 0000000..26be6d8 --- /dev/null +++ b/.github/workflows/golangci-lint.yaml @@ -0,0 +1,20 @@ +name: golangci-lint +on: [push, pull_request] + +permissions: + contents: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: "1.21" + cache: false + - name: golangci-lint + uses: golangci/golangci-lint-action@v4 + with: + version: v1.54 From fd7e2f696fb94ac65dc40f973001e147fa843c40 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:37:55 +0100 Subject: [PATCH 05/10] fix: satisfy golangci-lint --- class.go | 7 ++----- data/container.go | 2 +- data/container_test.go | 1 + data/data.go | 5 +---- data/data_test.go | 35 ++++++++++++++++++----------------- fs.go | 5 ++++- inventory.go | 4 ++-- reference/value.go | 6 +++--- reference_test.go | 2 +- registry.go | 15 ++++++--------- registry_test.go | 2 -- 11 files changed, 39 insertions(+), 45 deletions(-) diff --git a/class.go b/class.go index 3d3bf72..99c0f88 100644 --- a/class.go +++ b/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 }) diff --git a/data/container.go b/data/container.go index f17660d..c4b2934 100644 --- a/data/container.go +++ b/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 } diff --git a/data/container_test.go b/data/container_test.go index d68122d..ef4daf0 100644 --- a/data/container_test.go +++ b/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) { diff --git a/data/data.go b/data/data.go index 2c807be..40a53b4 100644 --- a/data/data.go +++ b/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 { diff --git a/data/data_test.go b/data/data_test.go index e1855ef..9aafc23 100644 --- a/data/data_test.go +++ b/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) diff --git a/fs.go b/fs.go index 5b40258..69371f4 100644 --- a/fs.go +++ b/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 } diff --git a/inventory.go b/inventory.go index 0ba0659..eb58572 100644 --- a/inventory.go +++ b/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) } diff --git a/reference/value.go b/reference/value.go index dffd745..4ac5b30 100644 --- a/reference/value.go +++ b/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 } diff --git a/reference_test.go b/reference_test.go index 69f4fd6..9b116fc 100644 --- a/reference_test.go +++ b/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")) diff --git a/registry.go b/registry.go index 8eb9c6c..c2d7641 100644 --- a/registry.go +++ b/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) } diff --git a/registry_test.go b/registry_test.go index 59d3ab7..f4cb6f5 100644 --- a/registry_test.go +++ b/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, From da848c6f030dd1f82abf626de2b40d41fcea387d Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:46:15 +0100 Subject: [PATCH 06/10] feat: update test workflow --- .github/workflows/golangci-lint.yaml | 20 ----------- .github/workflows/test.yaml | 52 +++++++++++++++++++++------- 2 files changed, 39 insertions(+), 33 deletions(-) delete mode 100644 .github/workflows/golangci-lint.yaml diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml deleted file mode 100644 index 26be6d8..0000000 --- a/.github/workflows/golangci-lint.yaml +++ /dev/null @@ -1,20 +0,0 @@ -name: golangci-lint -on: [push, pull_request] - -permissions: - contents: read - -jobs: - golangci: - name: lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: "1.21" - cache: false - - name: golangci-lint - uses: golangci/golangci-lint-action@v4 - with: - version: v1.54 diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 0b70426..2c22b9a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -1,18 +1,44 @@ 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: + vet: + name: "go vet" + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - uses: actions/checkout@v4 + - run: go vet $(go list ./... | grep -v /vendor/) + + golangci-lint: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: golangci-lint + uses: golangci/golangci-lint-action@v4 + with: + version: v1.54 + test: - strategy: - matrix: - go-version: [1.20.x, 1.21.x] - os: [ubuntu-latest, macos-latest] - runs-on: ${{ matrix.os }} + name: "test" + runs-on: ubuntu-latest steps: - - name: Setup Go - uses: actions/setup-go@v4.1.0 + - uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} - - name: Checkout - uses: actions/checkout@v4 - - name: Run tests - uses: robherley/go-test-action@v0.1.0 + go-version: ${{ env.GO_VERSION }} + - uses: actions/checkout@v4 + - run: go test $(go list ./... | grep -v /vendor/) From 58aeb2821c5b3b98702904e3079988e237f9f453 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:46:55 +0100 Subject: [PATCH 07/10] feat: adjust job names --- .github/workflows/test.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 2c22b9a..2d4c069 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -21,7 +21,7 @@ jobs: - run: go vet $(go list ./... | grep -v /vendor/) golangci-lint: - name: lint + name: golangci-lint runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -34,7 +34,7 @@ jobs: version: v1.54 test: - name: "test" + name: "go test" runs-on: ubuntu-latest steps: - uses: actions/setup-go@v5 From 6544738ecc5baceb5732ca923ef8000b61be213f Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:50:14 +0100 Subject: [PATCH 08/10] feat: gosh, an update --- .github/workflows/test.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 2d4c069..d84e95f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -24,14 +24,15 @@ jobs: name: golangci-lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: go-version: ${{ env.GO_VERSION }} - - name: golangci-lint - uses: golangci/golangci-lint-action@v4 + - uses: actions/checkout@v4 + with: + fetch-depth: 1 + - uses: golangci/golangci-lint-action@v4 with: - version: v1.54 + version: ${{ env.GOLANGCI_VERSION }} test: name: "go test" From 244a5862086023c4a5bc3e022b8aadb994449498 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:55:11 +0100 Subject: [PATCH 09/10] feat: rearrange checkout action --- .github/workflows/test.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index d84e95f..60102da 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -14,22 +14,22 @@ jobs: name: "go vet" runs-on: ubuntu-latest steps: + - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: go-version: ${{ env.GO_VERSION }} - - uses: actions/checkout@v4 - run: go vet $(go list ./... | grep -v /vendor/) golangci-lint: name: golangci-lint runs-on: ubuntu-latest steps: - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.GO_VERSION }} - uses: actions/checkout@v4 with: fetch-depth: 1 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} - uses: golangci/golangci-lint-action@v4 with: version: ${{ env.GOLANGCI_VERSION }} @@ -38,6 +38,7 @@ jobs: name: "go test" runs-on: ubuntu-latest steps: + - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: go-version: ${{ env.GO_VERSION }} From ac64f9e05b697b859afca84647d15a10c0fa880b Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:58:31 +0100 Subject: [PATCH 10/10] feat: disable go-cache on golangci-lint --- .github/workflows/test.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 60102da..f4c3c54 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -29,6 +29,7 @@ jobs: fetch-depth: 1 - uses: actions/setup-go@v5 with: + cache: false go-version: ${{ env.GO_VERSION }} - uses: golangci/golangci-lint-action@v4 with: