Skip to content

Commit

Permalink
chore: enable require-error rule from errorlint linter on controller …
Browse files Browse the repository at this point in the history 
…folder (argoproj#18690)

Signed-off-by: Matthieu MOREL <[email protected]>
Signed-off-by: pasha-codefresh <[email protected]>
Co-authored-by: pasha-codefresh <[email protected]>
  • Loading branch information
@mmorel-35 @pasha-codefresh @ggjulio
2 people authored and ggjulio committed Jul 21, 2024
1 parent 1bb7b55 commit 8f5c497
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 117 deletions.
2 changes: 1 addition & 1 deletion .golangci.yaml
View file
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
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
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

0 comments on commit 8f5c497

Please sign in to comment.