Skip to content

[testify-expert] Improve Test Quality: pkg/cli/semver_precise_test.go #48958

Description

@github-actions

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Current State

  • File: pkg/cli/semver_precise_test.go (89 lines, 2 test functions)
  • Source pair: pkg/cli/semver.go (wraps pkg/semverutil for parseVersion/isSemanticVersionTag); the actual IsPreciseVersion()/IsNewer() logic lives in pkg/semverutil/semverutil.go
  • Tests: TestIsPreciseVersion (table-driven, 8 cases), TestPreciseVersionPreference (single scenario, manual assertions)

Strengths

  • TestIsPreciseVersion is already table-driven with descriptive subtests and good edge cases (no-prefix version, single digit major, etc.).

Prioritized Improvements

1. Testify assertion upgrades (highest priority — zero testify usage today)

This file uses raw if/t.Errorf/t.Fatalf while the rest of pkg/cli (275+ files) uses testify/assert and testify/require. parseVersion returning nil should use require.NotNil to stop the subtest immediately instead of manual nil-check + Fatalf.

Before/after

Before:

v := parseVersion(tt.version)
if v == nil {
    t.Fatalf("Failed to parse version: %s", tt.version)
}

result := v.IsPreciseVersion()
if result != tt.expected {
    t.Errorf("isPreciseVersion() for %q = %v, want %v", tt.version, result, tt.expected)
}

After:

v := parseVersion(tt.version)
require.NotNil(t, v, "failed to parse version: %s", tt.version)
assert.Equal(t, tt.expected, v.IsPreciseVersion(), "isPreciseVersion() for %q", tt.version)

Similarly in TestPreciseVersionPreference:

require.NotNil(t, v6)
require.NotNil(t, v600)

assert.Equal(t, v6.Major, v600.Major)
assert.Equal(t, v6.Minor, v600.Minor)
assert.Equal(t, v6.Patch, v600.Patch)

assert.True(t, v600.IsPreciseVersion(), "v6.0.0 should be precise")
assert.False(t, v6.IsPreciseVersion(), "v6 should not be precise")

assert.False(t, v6.IsNewer(v600), "v6 should not be newer than v6.0.0")
assert.False(t, v600.IsNewer(v6), "v6.0.0 should not be newer than v6")
2. Missing/high-value test cases
  • No case for an invalid/unparseable version string (e.g. "not-a-version", "") exercising the parseVersion returning nil path directly instead of only asserting it inline.
  • No case with a pre-release/build-metadata suffix (e.g. v1.2.3-rc.1), which IsPreciseVersion/IsNewer may treat differently.
  • IsNewer is only exercised for the equal-version case in TestPreciseVersionPreference; add a dedicated table-driven TestIsNewer covering: strictly newer major, newer minor, newer patch, and older versions in both directions.
3. Table-driven refactor for TestPreciseVersionPreference

Convert the "v6 vs v6.0.0" comparison into a data-driven case (or fold it into a broader IsNewer/IsPreciseVersion table) so additional equal-version pairs (e.g. v6.0 vs v6.0.0, 6.1 vs v6.1.0) can be added without new boilerplate.

4. Organization/readability
  • Add a package-level comment or t.Run grouping to clarify that this file only tests the pkg/cli wrapper behavior (parseVersion) and not the full semverutil parsing logic (which has its own more extensive test file).

Acceptance Checklist

  • Convert existing assertions to testify/assert and require
  • Add test case for invalid/unparseable version input
  • Add test case for pre-release/build-metadata version
  • Add dedicated TestIsNewer table covering major/minor/patch differences in both directions
  • Refactor equal-version comparison into table-driven case
  • make test-unit passes for pkg/cli

Generated by 🧪 Daily Testify Uber Super Expert · aut00 · 22.3 AIC · ⊞ 7.2K ·

  • expires on Jul 31, 2026, 10:21 AM UTC-08:00

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions