Operator-sdk: Enable `dupl ` in the linter and fix issues

Created on 18 Nov 2019  路  4Comments  路  Source: operator-framework/operator-sdk

Feature Request

This issue is for we track the need to enable --enable= dupl. See: https://github.com/operator-framework/operator-sdk/blob/master/hack/go-linter.sh#L13

We should do a PR with the check enabled and its fixes. Following the current erro:

 $ make test-linter
./hack/go-linter.sh
Checking if golangci-lint is installed
Running golangci-lint
internal/scorecard/plugins/olm_tests.go:383: 383-419 lines are duplicate of `internal/scorecard/plugins/olm_tests.go:422-458` (dupl)
func (t *StatusDescriptorsTest) Run(ctx context.Context) *schelpers.TestResult {
    res := &schelpers.TestResult{Test: t}
    err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR)
    if err != nil {
        res.Errors = append(res.Errors, err)
        res.State = scapiv1alpha1.ErrorState
        return res
    }
    if t.CR.Object["status"] == nil {
        return res
    }
    statusBlock := t.CR.Object["status"].(map[string]interface{})
    res.MaximumPoints = len(statusBlock)
    var crd *olmapiv1alpha1.CRDDescription
    for _, owned := range t.CSV.Spec.CustomResourceDefinitions.Owned {
        if owned.Kind == t.CR.GetKind() {
            crd = &owned
            break
        }
    }
    if crd == nil {
        return res
    }
    for key := range statusBlock {
        for _, statDesc := range crd.StatusDescriptors {
            if statDesc.Path == key {
                res.EarnedPoints++
                delete(statusBlock, key)
                break
            }
        }
    }
    for key := range statusBlock {
        res.Suggestions = append(res.Suggestions, "Add a status descriptor for "+key)
    }
    return res
}
internal/scorecard/plugins/olm_tests.go:422: 422-458 lines are duplicate of `internal/scorecard/plugins/olm_tests.go:383-419` (dupl)
func (t *SpecDescriptorsTest) Run(ctx context.Context) *schelpers.TestResult {
    res := &schelpers.TestResult{Test: t}
    err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR)
    if err != nil {
        res.Errors = append(res.Errors, err)
        res.State = scapiv1alpha1.ErrorState
        return res
    }
    if t.CR.Object["spec"] == nil {
        return res
    }
    specBlock := t.CR.Object["spec"].(map[string]interface{})
    res.MaximumPoints = len(specBlock)
    var crd *olmapiv1alpha1.CRDDescription
    for _, owned := range t.CSV.Spec.CustomResourceDefinitions.Owned {
        if owned.Kind == t.CR.GetKind() {
            crd = &owned
            break
        }
    }
    if crd == nil {
        return res
    }
    for key := range specBlock {
        for _, statDesc := range crd.SpecDescriptors {
            if statDesc.Path == key {
                res.EarnedPoints++
                delete(specBlock, key)
                break
            }
        }
    }
    for key := range specBlock {
        res.Suggestions = append(res.Suggestions, "Add a spec descriptor for "+key)
    }
    return res
}
make: *** [test-linter] Error 1
aretesting good first issue help wanted kinflake

All 4 comments

@camilamacedo86

I would like to give this a go.

Hi @camilamacedo86

I have updated the dupl errors in internal/scaffold/role.go and internal/scorecard/plugins/olm_tests.go and I can't find any unit test covering this code. Is this correct or am I missing something?

HI @jesperhansen17,

The project is very good covered by tests, however, many of them are e2e and are not written in go. We would like to start to cover more with unit tests as well.

Did you make the PR? If yes, please add the Closes: #numberissue for it be linked and closed with the merge.

Hi @camilamacedo86

Ok, I see.

Haven't opened a PR yet. Still doing some manual testing. Will open ASAP.

Was this page helpful?
0 / 5 - 0 ratings