Skip to content

bug: HaveAppStudioTestsFinished() has tautological nil check on line 709 flagged by nilness analyser #1497

@harshakumar25

Description

@harshakumar25

What the linter reports

Go's nilness static analyser flags a tautological nil check in HaveAppStudioTestsFinished() in gitops/snapshot.go:

tautological condition: non-nil != nil  nilness [Ln 709, Col 25]

Current code

func HaveAppStudioTestsFinished(snapshot *applicationapiv1alpha1.Snapshot) bool {
    statusCondition := meta.FindStatusCondition(snapshot.Status.Conditions, AppStudioTestSucceededCondition)
    if statusCondition == nil {
        statusCondition = meta.FindStatusCondition(snapshot.Status.Conditions, LegacyTestSucceededCondition)
        return statusCondition != nil && statusCondition.Status != metav1.ConditionUnknown
    }
    return statusCondition != nil && statusCondition.Status != metav1.ConditionUnknown  // ← always true: nil check is impossible here
}

The if statusCondition == nil block returns early, so the final return on line 709 is only reachable when statusCondition from FindStatusCondition(AppStudioTestSucceededCondition) is already non-nil. The statusCondition != nil guard there can never be false — the analyser can prove it statically.

Both branches also have identical return expressions, which is the clearest signal that the control flow can be simplified.

Expected behaviour (unchanged)

  • Neither condition set → false
  • LegacyTestSucceededCondition set with non-Unknown status → true
  • AppStudioTestSucceededCondition set with non-Unknown status → true

Fix

Collapse the duplicated return into a single one after the fallback assignment:

func HaveAppStudioTestsFinished(snapshot *applicationapiv1alpha1.Snapshot) bool {
    statusCondition := meta.FindStatusCondition(snapshot.Status.Conditions, AppStudioTestSucceededCondition)
    if statusCondition == nil {
        statusCondition = meta.FindStatusCondition(snapshot.Status.Conditions, LegacyTestSucceededCondition)
    }
    return statusCondition != nil && statusCondition.Status != metav1.ConditionUnknown
}

Additional context

The existing tests in gitops/snapshot_test.go exercise HaveAppStudioTestsFinished when no condition is set and when AppStudioTestSucceededCondition is set, but there is no test for the case where only LegacyTestSucceededCondition is set. A test for that path should be added alongside the fix (compare: HaveAppStudioTestsSucceeded already has a legacy-condition test at line 458).

HaveAppStudioTestsSucceeded (the function right below this one) does not have this problem — it correctly checks the legacy condition without duplicating the return.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions