Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: move removetest.go to the internal package #5495

Merged
merged 3 commits into from
Dec 22, 2023
Merged

chore: move removetest.go to the internal package #5495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f814039
chore: move removetest.go to the internal package
stormqueen1990 Dec 18, 2023
42d5870
chore: move removetest_testutils.go to internal/testutils
stormqueen1990 Dec 20, 2023
27ae069
fix: rename helper structs to use more specific names
stormqueen1990 Dec 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 13 additions & 13 deletions kustomize/commands/edit/remove/removeresource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,22 @@ import (
"errors"
"testing"

"sigs.k8s.io/kustomize/kustomize/v5/commands/edit/remove_test"
testutils_test "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/testutils"
)

func TestRemoveResources(t *testing.T) {
testCases := []remove_test.Case{
testCases := []testutils_test.RemoveTestCase{
{
Description: "remove resources",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"resource1.yaml",
"resource2.yaml",
"resource3.yaml",
},
RemoveArgs: []string{"resource1.yaml"},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"resource2.yaml",
"resource3.yaml",
Expand All @@ -34,7 +34,7 @@ func TestRemoveResources(t *testing.T) {
},
{
Description: "remove resource with pattern",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"foo/resource1.yaml",
"foo/resource2.yaml",
Expand All @@ -43,7 +43,7 @@ func TestRemoveResources(t *testing.T) {
},
RemoveArgs: []string{"foo/resource*.yaml"},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"do/not/deleteme/please.yaml",
},
Expand All @@ -56,15 +56,15 @@ func TestRemoveResources(t *testing.T) {
},
{
Description: "nothing found to remove",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"resource1.yaml",
"resource2.yaml",
"resource3.yaml",
},
RemoveArgs: []string{"foo"},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"resource2.yaml",
"resource3.yaml",
Expand All @@ -74,14 +74,14 @@ func TestRemoveResources(t *testing.T) {
},
{
Description: "no arguments",
Given: remove_test.Given{},
Expected: remove_test.Expected{
Given: testutils_test.RemoveTestGivenValues{},
Expected: testutils_test.RemoveTestExpectedValues{
Err: errors.New("must specify a resource file"),
},
},
{
Description: "remove with multiple pattern arguments",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"foo/foo.yaml",
"bar/bar.yaml",
Expand All @@ -94,7 +94,7 @@ func TestRemoveResources(t *testing.T) {
"res*.yaml",
},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"do/not/deleteme/please.yaml",
},
Expand All @@ -107,5 +107,5 @@ func TestRemoveResources(t *testing.T) {
},
}

remove_test.ExecuteTestCases(t, testCases, "resources", newCmdRemoveResource)
testutils_test.ExecuteRemoveTestCases(t, testCases, "resources", newCmdRemoveResource)
}
26 changes: 13 additions & 13 deletions kustomize/commands/edit/remove/removetransformer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@ package remove
import (
"testing"

"sigs.k8s.io/kustomize/kustomize/v5/commands/edit/remove_test"
testutils_test "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/testutils"
"sigs.k8s.io/kustomize/kyaml/errors"
)

func TestRemoveTransformer(t *testing.T) {
testCases := []remove_test.Case{
testCases := []testutils_test.RemoveTestCase{
{
Description: "remove transformers",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"transformer1.yaml",
"transformer2.yaml",
"transformer3.yaml",
},
RemoveArgs: []string{"transformer1.yaml"},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"transformer2.yaml",
"transformer3.yaml",
Expand All @@ -34,7 +34,7 @@ func TestRemoveTransformer(t *testing.T) {
},
{
Description: "remove transformer with pattern",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"foo/transformer1.yaml",
"foo/transformer2.yaml",
Expand All @@ -43,7 +43,7 @@ func TestRemoveTransformer(t *testing.T) {
},
RemoveArgs: []string{"foo/transformer*.yaml"},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"do/not/deleteme/please.yaml",
},
Expand All @@ -56,15 +56,15 @@ func TestRemoveTransformer(t *testing.T) {
},
{
Description: "nothing found to remove",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"transformer1.yaml",
"transformer2.yaml",
"transformer3.yaml",
},
RemoveArgs: []string{"foo"},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"transformer2.yaml",
"transformer3.yaml",
Expand All @@ -74,14 +74,14 @@ func TestRemoveTransformer(t *testing.T) {
},
{
Description: "no arguments",
Given: remove_test.Given{},
Expected: remove_test.Expected{
Given: testutils_test.RemoveTestGivenValues{},
Expected: testutils_test.RemoveTestExpectedValues{
Err: errors.Errorf("must specify a transformer file"),
},
},
{
Description: "remove with multiple pattern arguments",
Given: remove_test.Given{
Given: testutils_test.RemoveTestGivenValues{
Items: []string{
"foo/foo.yaml",
"bar/bar.yaml",
Expand All @@ -94,7 +94,7 @@ func TestRemoveTransformer(t *testing.T) {
"tra*.yaml",
},
},
Expected: remove_test.Expected{
Expected: testutils_test.RemoveTestExpectedValues{
Items: []string{
"do/not/deleteme/please.yaml",
},
Expand All @@ -107,5 +107,5 @@ func TestRemoveTransformer(t *testing.T) {
},
}

remove_test.ExecuteTestCases(t, testCases, "transformers", newCmdRemoveTransformer)
testutils_test.ExecuteRemoveTestCases(t, testCases, "transformers", newCmdRemoveTransformer)
}
37 changes: 20 additions & 17 deletions ...e/commands/edit/remove_test/removetest.go → ...ds/internal/testutils/remove_testutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,56 +1,59 @@
// Copyright 2022 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0

package remove_test
package testutils_test

import (
"fmt"
"strings"
"testing"

"github.com/spf13/cobra"
testutils_test "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/testutils"
"sigs.k8s.io/kustomize/kyaml/filesys"
)

// Given represents the provided inputs for the test case.
type Given struct {
// RemoveTestGivenValues represents the provided inputs for the test case.
type RemoveTestGivenValues struct {
// Items is the given input items.
Items []string
// RemoveArgs are the arguments to pass to the remove command.
RemoveArgs []string
}

// Expected represents the expected outputs of the test case.
type Expected struct {
// Expected is the collection of expected output items.
// RemoveTestExpectedValues represents the expected outputs of the test case.
type RemoveTestExpectedValues struct {
// RemoveTestExpectedValues is the collection of expected output items.
Items []string
// Deleted is the collection of expected Deleted items (if any).
Deleted []string
// Err represents the error that is expected in the output (if any).
Err error
}

// Case represents a test case to execute.
type Case struct {
// RemoveTestCase represents a test case to execute.
type RemoveTestCase struct {
// Description is the description of the test case.
Description string
// Given is the provided inputs for the test case.
Given Given
Given RemoveTestGivenValues
// Expected is the expected outputs for the test case.
Expected Expected
Expected RemoveTestExpectedValues
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, Maybe those global struct names are a little generic more than real functions.
If you can, Could you rename those to a more small scope name? (ex, add the Remove prefix, It's only my idea.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Maybe RemoveTestCase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koba1t updated! Could you PTAL?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


// ExecuteTestCases executes the provided test cases against the specified command
// ExecuteRemoveTestCases executes the provided test cases against the specified command
// for a particular collection (e.g. ) tests a command defined by the provided
// collection Name (e.g. transformers or resources) and newRemoveCmdToTest function.
func ExecuteTestCases(t *testing.T, testCases []Case, collectionName string,
newRemoveCmdToTest func(filesys.FileSystem) *cobra.Command) {
func ExecuteRemoveTestCases(
t *testing.T,
testCases []RemoveTestCase,
collectionName string,
newRemoveCmdToTest func(filesys.FileSystem) *cobra.Command,
) {
t.Helper()
for _, tc := range testCases {
t.Run(tc.Description, func(t *testing.T) {
fSys := filesys.MakeFsInMemory()
testutils_test.WriteTestKustomizationWith(
WriteTestKustomizationWith(
fSys,
[]byte(fmt.Sprintf("%s:\n - %s",
collectionName,
Expand All @@ -61,13 +64,13 @@ func ExecuteTestCases(t *testing.T, testCases []Case, collectionName string,
t.Errorf("unexpected cmd error: %v", err)
} else if tc.Expected.Err != nil {
if err.Error() != tc.Expected.Err.Error() {
t.Errorf("expected error did not occurred. Expected: %v. Actual: %v",
t.Errorf("expected error did not occur. Expected: %v. Actual: %v",
tc.Expected.Err,
err)
}
return
}
content, err := testutils_test.ReadTestKustomization(fSys)
content, err := ReadTestKustomization(fSys)
if err != nil {
t.Errorf("unexpected read error: %v", err)
}
Expand Down