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: enable require-error rule from errorlint linter on controller folder #18690

Merged
merged 2 commits into from
Jun 17, 2024
Merged

chore: enable require-error rule from errorlint linter on controller folder #18690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d2e15e
chore: enable require-error rule from errorlint linter on controller …
mmorel-35 Jun 15, 2024
67c20b1
Merge branch 'master' into testifylint/controller
pasha-codefresh Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ issues:
- SA1019
- SA5011
exclude-rules:
- path: "(cmpserver|controller|reposerver)/"
- path: "(cmpserver|reposerver)/"
text: "require-error:"
linters:
- testifylint
Expand Down
72 changes: 36 additions & 36 deletions controller/appcontroller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func TestAutoSync(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, app.Operation)
assert.NotNil(t, app.Operation.Sync)
assert.False(t, app.Operation.Sync.Prune)
Expand Down Expand Up @@ -501,7 +501,7 @@ func TestSkipAutoSync(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
})

Expand All @@ -516,7 +516,7 @@ func TestSkipAutoSync(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
})

Expand All @@ -532,7 +532,7 @@ func TestSkipAutoSync(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
})

Expand All @@ -549,7 +549,7 @@ func TestSkipAutoSync(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
})

Expand All @@ -575,7 +575,7 @@ func TestSkipAutoSync(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}})
assert.NotNil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
})

Expand All @@ -591,7 +591,7 @@ func TestSkipAutoSync(t *testing.T) {
})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
})
}
Expand Down Expand Up @@ -627,7 +627,7 @@ func TestAutoSyncIndicateError(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}})
assert.NotNil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, app.Operation)
}

Expand Down Expand Up @@ -670,7 +670,7 @@ func TestAutoSyncParameterOverrides(t *testing.T) {
cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}})
assert.Nil(t, cond)
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, app.Operation)
}

Expand Down Expand Up @@ -715,7 +715,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, patched)
})

Expand Down Expand Up @@ -766,11 +766,11 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, patched)
objsMap, err := ctrl.stateCache.GetManagedLiveObjs(app, []*unstructured.Unstructured{})
if err != nil {
assert.NoError(t, err)
require.NoError(t, err)
}
// Managed objects must be empty
assert.Empty(t, objsMap)
Expand Down Expand Up @@ -802,7 +802,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, patched)
})

Expand All @@ -826,7 +826,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
}

app1 := appTemplate.DeepCopy()
Expand Down Expand Up @@ -869,7 +869,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
// finalizer is not deleted
assert.False(t, patched)
// post-delete hook is created
Expand Down Expand Up @@ -907,7 +907,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
// finalizer is removed
assert.True(t, patched)
})
Expand Down Expand Up @@ -942,7 +942,7 @@ func TestFinalizeAppDeletion(t *testing.T) {
err := ctrl.finalizeApplicationDeletion(app, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
assert.NoError(t, err)
require.NoError(t, err)
// post-delete hook is deleted
require.Len(t, ctrl.kubectl.(*MockKubectl).DeletedResources, 1)
require.Equal(t, "post-delete-hook", ctrl.kubectl.(*MockKubectl).DeletedResources[0].Name)
Expand Down Expand Up @@ -1103,7 +1103,7 @@ func TestGetResourceTree_HasOrphanedResources(t *testing.T) {
TargetState: test.DeploymentManifest,
}})

assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, []v1alpha1.ResourceNode{managedDeploy}, tree.Nodes)
assert.Equal(t, []v1alpha1.ResourceNode{orphanedDeploy1, orphanedDeploy2}, tree.OrphanedNodes)
}
Expand Down Expand Up @@ -1463,7 +1463,7 @@ func TestUpdateReconciledAt(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand All @@ -1476,11 +1476,11 @@ func TestUpdateReconciledAt(t *testing.T) {
ctrl.processAppRefreshQueueItem()

_, updated, err := unstructured.NestedString(receivedPatch, "status", "reconciledAt")
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, updated)

_, updated, err = unstructured.NestedString(receivedPatch, "status", "observedAt")
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, updated)
})

Expand All @@ -1492,11 +1492,11 @@ func TestUpdateReconciledAt(t *testing.T) {
ctrl.processAppRefreshQueueItem()

_, updated, err := unstructured.NestedString(receivedPatch, "status", "reconciledAt")
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, updated)

_, updated, err = unstructured.NestedString(receivedPatch, "status", "observedAt")
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, updated)
})
}
Expand All @@ -1522,7 +1522,7 @@ func TestProjectErrorToCondition(t *testing.T) {

obj, ok, err := ctrl.appInformer.GetIndexer().GetByKey(key)
assert.True(t, ok)
assert.NoError(t, err)
require.NoError(t, err)
updatedApp := obj.(*v1alpha1.Application)
assert.Equal(t, v1alpha1.ApplicationConditionInvalidSpecError, updatedApp.Status.Conditions[0].Type)
assert.Equal(t, "Application referencing project wrong project which does not exist", updatedApp.Status.Conditions[0].Message)
Expand All @@ -1542,7 +1542,7 @@ func TestFinalizeProjectDeletion_HasApplications(t *testing.T) {
})

err := ctrl.finalizeProjectDeletion(proj)
assert.NoError(t, err)
require.NoError(t, err)
assert.False(t, patched)
}

Expand All @@ -1554,13 +1554,13 @@ func TestFinalizeProjectDeletion_DoesNotHaveApplications(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.AppProject{}, nil
})

err := ctrl.finalizeProjectDeletion(proj)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, map[string]interface{}{
"metadata": map[string]interface{}{
"finalizers": nil,
Expand All @@ -1579,7 +1579,7 @@ func TestProcessRequestedAppOperation_FailedNoRetries(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand Down Expand Up @@ -1607,7 +1607,7 @@ func TestProcessRequestedAppOperation_InvalidDestination(t *testing.T) {
defer fakeAppCs.Unlock()
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand All @@ -1633,7 +1633,7 @@ func TestProcessRequestedAppOperation_FailedHasRetries(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand Down Expand Up @@ -1676,7 +1676,7 @@ func TestProcessRequestedAppOperation_RunningPreviouslyFailed(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand Down Expand Up @@ -1709,7 +1709,7 @@ func TestProcessRequestedAppOperation_HasRetriesTerminated(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand All @@ -1736,7 +1736,7 @@ func TestProcessRequestedAppOperation_Successful(t *testing.T) {
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})
Expand Down Expand Up @@ -1799,7 +1799,7 @@ func TestGetAppHosts(t *testing.T) {
}},
}})

assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, []v1alpha1.HostInfo{{
Name: "minikube",
SystemInfo: corev1.NodeSystemInfo{OSImage: "debian"},
Expand Down Expand Up @@ -1926,7 +1926,7 @@ func TestAddControllerNamespace(t *testing.T) {
ctrl.processAppRefreshQueueItem()

updatedApp, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(ctrl.namespace).Get(context.Background(), app.Name, metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, test.FakeArgoCDNamespace, updatedApp.Status.ControllerNamespace)
})
t.Run("set controllerNamespace when the app is in another namespace than the controller", func(t *testing.T) {
Expand All @@ -1945,7 +1945,7 @@ func TestAddControllerNamespace(t *testing.T) {
ctrl.processAppRefreshQueueItem()

updatedApp, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(appNamespace).Get(context.Background(), app.Name, metav1.GetOptions{})
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, test.FakeArgoCDNamespace, updatedApp.Status.ControllerNamespace)
})
}
9 changes: 5 additions & 4 deletions controller/clusterinfoupdater_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

clustercache "github.com/argoproj/gitops-engine/pkg/cache"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/tools/cache"
)
Expand Down Expand Up @@ -76,7 +77,7 @@ func TestClusterSecretUpdater(t *testing.T) {

appCache := appstate.NewCache(cacheutil.NewCache(cacheutil.NewInMemoryCache(time.Minute)), time.Minute)
cluster, err := argoDB.CreateCluster(ctx, &v1alpha1.Cluster{Server: "http://minikube"})
assert.NoError(t, err, "Test prepare test data create cluster failed")
require.NoError(t, err, "Test prepare test data create cluster failed")

for _, test := range tests {
info := &clustercache.ClusterInfo{
Expand All @@ -90,11 +91,11 @@ func TestClusterSecretUpdater(t *testing.T) {
updater := NewClusterInfoUpdater(nil, argoDB, lister, appCache, nil, nil, fakeNamespace)

err = updater.updateClusterInfo(context.Background(), *cluster, info)
assert.NoError(t, err, "Invoking updateClusterInfo failed.")
require.NoError(t, err, "Invoking updateClusterInfo failed.")

var clusterInfo v1alpha1.ClusterInfo
err = appCache.GetClusterInfo(cluster.Server, &clusterInfo)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, updatedK8sVersion, clusterInfo.ServerVersion)
assert.Equal(t, test.ExpectedStatus, clusterInfo.ConnectionState.Status)
}
Expand All @@ -103,7 +104,7 @@ func TestClusterSecretUpdater(t *testing.T) {
func TestUpdateClusterLabels(t *testing.T) {
shouldNotBeInvoked := func(ctx context.Context, cluster *v1alpha1.Cluster) (*v1alpha1.Cluster, error) {
shouldNotHappen := errors.New("if an error happens here, something's wrong")
assert.NoError(t, shouldNotHappen)
require.NoError(t, shouldNotHappen)
return nil, shouldNotHappen
}
tests := []struct {
Expand Down
Loading