Merge pull request #1654 from jodh-intel/fix-test-constraint-version-numbers

Fix test constraint version numbers

pkg/katatestutils/constraints.go

@@ -184,13 +184,13 @@ func (tc *TestConstraint) handleDistroName(name string, op Operator) (result Res
 // handleDistroVersion checks that the current distro version is compatible with
 // the constraint specified by the arguments.
 func (tc *TestConstraint) handleDistroVersion(version string, op Operator) (result Result, err error) {
-	return handleVersionType("distro", version, op, tc.DistroVersion)
+	return handleVersionType("distro", tc.DistroVersion, op, version)
 }
 
 // handleKernelVersion checks that the current kernel version is compatible with
 // the constraint specified by the arguments.
 func (tc *TestConstraint) handleKernelVersion(version string, op Operator) (result Result, err error) {
-	return handleVersionType("kernel", version, op, tc.KernelVersion)
+	return handleVersionType("kernel", tc.KernelVersion, op, version)
 }
 
 // handleVersionType checks that the current and new versions are compatible with
@@ -250,7 +250,7 @@ func handleVersionType(versionName, newVersion string, op Operator, currentVersi
 	}
 
 	descr := fmt.Sprintf("need %s version %s %q, got version %q",
-		versionName, op, newVersion, currentVersion)
+		versionName, op, currentVersion, newVersion)
 
 	result = Result{
 		Description: descr,
@@ -432,9 +432,9 @@ func (tc *TestConstraint) handleResults(result Result, err error) {
 	}
 }
 
-// constraintInvalid handles the specified constraint, returning true if the
-// constraint fails, else false.
-func (tc *TestConstraint) constraintInvalid(fn Constraint) bool {
+// constraintValid handles the specified constraint, returning true if the
+// constraint is valid, else false.
+func (tc *TestConstraint) constraintValid(fn Constraint) bool {
 	c := Constraints{}
 
 	// Call the constraint function that sets the Constraints values
@@ -449,7 +449,7 @@ func (tc *TestConstraint) constraintInvalid(fn Constraint) bool {
 		result, err := tc.handleUID(c.UID, c.Operator)
 		tc.handleResults(result, err)
 		if !result.Success {
-			return true
+			return false
 		}
 	}
 
@@ -457,7 +457,7 @@ func (tc *TestConstraint) constraintInvalid(fn Constraint) bool {
 		result, err := tc.handleDistroName(c.DistroName, c.Operator)
 		tc.handleResults(result, err)
 		if !result.Success {
-			return true
+			return false
 		}
 	}
 
@@ -465,7 +465,7 @@ func (tc *TestConstraint) constraintInvalid(fn Constraint) bool {
 		result, err := tc.handleDistroVersion(c.DistroVersion, c.Operator)
 		tc.handleResults(result, err)
 		if !result.Success {
-			return true
+			return false
 		}
 
 	}
@@ -474,11 +474,11 @@ func (tc *TestConstraint) constraintInvalid(fn Constraint) bool {
 		result, err := tc.handleKernelVersion(c.KernelVersion, c.Operator)
 		tc.handleResults(result, err)
 		if !result.Success {
-			return true
+			return false
 		}
 
 	}
 
 	// Constraint is valid
-	return false
+	return true
 }

pkg/katatestutils/constraints_api.go

@@ -112,9 +112,14 @@ func (tc *TestConstraint) NotValid(constraints ...Constraint) bool {
 		panic("need atleast one constraint")
 	}
 
+	// Reset in case of a previous call
+	tc.Passed = nil
+	tc.Failed = nil
+	tc.Issue = ""
+
 	for _, c := range constraints {
-		invalid := tc.constraintInvalid(c)
-		if invalid {
+		valid := tc.constraintValid(c)
+		if !valid {
 			return true
 		}
 	}
@@ -177,14 +182,30 @@ func NeedDistroVersionWithOp(version string, op Operator) Constraint {
 	}
 }
 
-// NeedDistroVersionLT will skip the test unless the distro version is older
-// than the specified version.
+// NeedDistroVersionEquals will skip the test unless the distro version is the
+// same as the specified version.
 //
 // Note: distro versions vary in format.
 func NeedDistroVersionEquals(version string) Constraint {
 	return NeedDistroVersionWithOp(version, eqOperator)
 }
 
+// NeedDistroVersionNotEquals will skip the test unless the distro version is
+// different to the specified version.
+//
+// Note: distro versions vary in format.
+func NeedDistroVersionNotEquals(version string) Constraint {
+	return NeedDistroVersionWithOp(version, neOperator)
+}
+
+// NeedDistroVersionLE will skip the test unless the distro version is older
+// than or the same as the specified version.
+//
+// Note: distro versions vary in format.
+func NeedDistroVersionLE(version string) Constraint {
+	return NeedDistroVersionWithOp(version, leOperator)
+}
+
 // NeedDistroVersionLT will skip the test unless the distro version is older
 // than the specified version.
 //
@@ -193,6 +214,14 @@ func NeedDistroVersionLT(version string) Constraint {
 	return NeedDistroVersionWithOp(version, ltOperator)
 }
 
+// NeedDistroVersionGE will skip the test unless the distro version is newer
+// than or the same as the specified version.
+//
+// Note: distro versions vary in format.
+func NeedDistroVersionGE(version string) Constraint {
+	return NeedDistroVersionWithOp(version, geOperator)
+}
+
 // NeedDistroVersionGT will skip the test unless the distro version is newer
 // than the specified version.
 //

pkg/katatestutils/constraints_test.go

@@ -196,19 +196,34 @@ func semverBumpVersion(ver semver.Version, bumpMajor, bumpMinor, bumpPatch bool)
 	return ver.String(), nil
 }
 
-// incrementVersion increments the specified version and returns the
-// string representation.
-func incrementVersion(version string) (string, error) {
+// changeVersion modifies the specified version and returns the
+// string representation. If decrement is true the returned version is smaller
+// than the specified version, else it is larger.
+func changeVersion(version string, decrement bool) (string, error) {
+	operand := int64(1)
+
+	if decrement {
+		operand = -1
+	}
+
 	// Is it an integer?
 	intResult, err := strconv.ParseUint(version, 10, 0)
 	if err == nil {
-		return fmt.Sprintf("%d", intResult+1), nil
+		if intResult == 0 && decrement {
+			return "", fmt.Errorf("cannot decrement integer version with value zero")
+		}
+
+		return fmt.Sprintf("%d", uint64(int64(intResult)+operand)), nil
 	}
 
 	// Is it a float?
 	floatResult, err := strconv.ParseFloat(version, 32)
 	if err == nil {
-		return fmt.Sprintf("%f", floatResult+1), nil
+		if int(floatResult) == 0 && decrement {
+			return "", fmt.Errorf("cannot decrement integer part of floating point version with value zero: %v", version)
+		}
+
+		return fmt.Sprintf("%f", floatResult+float64(operand)), nil
 	}
 
 	// Not an int nor a float, so it must be a semantic version
@@ -218,14 +233,36 @@ func incrementVersion(version string) (string, error) {
 		return "", err
 	}
 
-	err = ver.IncrementMajor()
-	if err != nil {
-		return "", err
+	if decrement {
+		// the semver package only provides increment operations, so
+		// handle decrement ourselves.
+		major := ver.Major
+
+		if major == 0 {
+			return "", fmt.Errorf("cannot decrement semver with zero major version: %+v", version)
+		}
+
+		major--
+
+		ver.Major = major
+	} else {
+		err = ver.IncrementMajor()
+		if err != nil {
+			return "", err
+		}
 	}
 
 	return ver.String(), nil
 }
 
+func incrementVersion(version string) (string, error) {
+	return changeVersion(version, false)
+}
+
+func decrementVersion(version string) (string, error) {
+	return changeVersion(version, true)
+}
+
 // testGetDistro is an alternative implementation of getDistroDetails() used
 // for testing.
 func testGetDistro() (name, version string, err error) {
@@ -550,16 +587,16 @@ func TestConstraintHandleDistroVersion(t *testing.T) {
 		{higherVersion, eqOperator, Result{Success: false}, false},
 
 		{distroVersion, gtOperator, Result{Success: false}, false},
-		{higherVersion, gtOperator, Result{Success: true}, false},
+		{higherVersion, gtOperator, Result{Success: false}, false},
 
 		{distroVersion, geOperator, Result{Success: true}, false},
-		{higherVersion, geOperator, Result{Success: true}, false},
+		{higherVersion, geOperator, Result{Success: false}, false},
 
 		{distroVersion, ltOperator, Result{Success: false}, false},
-		{higherVersion, ltOperator, Result{Success: false}, false},
+		{higherVersion, ltOperator, Result{Success: true}, false},
 
 		{distroVersion, leOperator, Result{Success: true}, false},
-		{higherVersion, leOperator, Result{Success: false}, false},
+		{higherVersion, leOperator, Result{Success: true}, false},
 
 		{distroVersion, neOperator, Result{Success: false}, false},
 		{higherVersion, neOperator, Result{Success: true}, false},
@@ -773,24 +810,24 @@ func TestConstraintHandleKernelVersion(t *testing.T) {
 		{kernelVersion, neOperator, Result{Success: false}, false},
 
 		{newerMajor, eqOperator, Result{Success: false}, false},
-		{newerMajor, geOperator, Result{Success: true}, false},
-		{newerMajor, gtOperator, Result{Success: true}, false},
-		{newerMajor, ltOperator, Result{Success: false}, false},
-		{newerMajor, leOperator, Result{Success: false}, false},
+		{newerMajor, geOperator, Result{Success: false}, false},
+		{newerMajor, gtOperator, Result{Success: false}, false},
+		{newerMajor, ltOperator, Result{Success: true}, false},
+		{newerMajor, leOperator, Result{Success: true}, false},
 		{newerMajor, neOperator, Result{Success: true}, false},
 
 		{newerMinor, eqOperator, Result{Success: false}, false},
-		{newerMinor, geOperator, Result{Success: true}, false},
-		{newerMinor, gtOperator, Result{Success: true}, false},
-		{newerMinor, ltOperator, Result{Success: false}, false},
-		{newerMinor, leOperator, Result{Success: false}, false},
+		{newerMinor, geOperator, Result{Success: false}, false},
+		{newerMinor, gtOperator, Result{Success: false}, false},
+		{newerMinor, ltOperator, Result{Success: true}, false},
+		{newerMinor, leOperator, Result{Success: true}, false},
 		{newerMinor, neOperator, Result{Success: true}, false},
 
 		{newerPatch, eqOperator, Result{Success: false}, false},
-		{newerPatch, geOperator, Result{Success: true}, false},
-		{newerPatch, gtOperator, Result{Success: true}, false},
-		{newerPatch, ltOperator, Result{Success: false}, false},
-		{newerPatch, leOperator, Result{Success: false}, false},
+		{newerPatch, geOperator, Result{Success: false}, false},
+		{newerPatch, gtOperator, Result{Success: false}, false},
+		{newerPatch, ltOperator, Result{Success: true}, false},
+		{newerPatch, leOperator, Result{Success: true}, false},
 		{newerPatch, neOperator, Result{Success: true}, false},
 	}
 
@@ -1145,7 +1182,152 @@ func TestConstraintNotValid(t *testing.T) {
 
 }
 
-func TestConstraintConstraintInvalid(t *testing.T) {
+func TestConstraintNotValidKernelVersion(t *testing.T) {
+	assert := assert.New(t)
+
+	assert.NotNil(kernelVersion)
+
+	// Generate new kernel versions for testing purposes based on the
+	// current kernel version.
+	higherVersion, err := incrementVersion(kernelVersion)
+	assert.NoError(err)
+	assert.NotEqual(kernelVersion, higherVersion)
+
+	lowerVersion, err := decrementVersion(kernelVersion)
+	assert.NoError(err)
+	assert.NotEqual(kernelVersion, lowerVersion)
+
+	// Antique kernel version numbers.
+	//
+	// Note: Not all are actually real kernel releases - we're just trying
+	// to do a thorough test.
+	lowKernelVersions := []string{
+		"0.0.0",
+		"0.0.1",
+		"1.0.0",
+		"1.0.6-1.1.0",
+		"2.0.0",
+		"2.6.0",
+		lowerVersion,
+	}
+
+	// Host kernel is expected to be newer than all the low kernel versions
+	for _, debug := range []bool{true, false} {
+		tc := NewTestConstraint(debug)
+
+		for _, ver := range lowKernelVersions {
+			result := tc.NotValid(NeedKernelVersionEquals(ver))
+			assert.True(result)
+
+			result = tc.NotValid(NeedKernelVersionLE(ver))
+			assert.True(result)
+
+			result = tc.NotValid(NeedKernelVersionLT(ver))
+			assert.True(result)
+
+			result = tc.NotValid(NeedKernelVersionGT(ver))
+			assert.False(result)
+
+			result = tc.NotValid(NeedKernelVersionGE(ver))
+			assert.False(result)
+
+			result = tc.NotValid(NeedKernelVersionNotEquals(ver))
+			assert.False(result)
+		}
+	}
+
+	// Ridiculously high kernel version numbers. The host kernel is
+	// expected to never reach these values.
+	highKernelVersions := []string{
+		higherVersion,
+		"999.0.0",
+		"999.0.999",
+		"999.999.999",
+		"1024.0.0",
+	}
+
+	for _, debug := range []bool{true, false} {
+		tc := NewTestConstraint(debug)
+
+		for _, ver := range highKernelVersions {
+			result := tc.NotValid(NeedKernelVersionEquals(ver))
+			assert.True(result)
+
+			result = tc.NotValid(NeedKernelVersionGE(ver))
+			assert.True(result)
+
+			result = tc.NotValid(NeedKernelVersionGT(ver))
+			assert.True(result)
+
+			result = tc.NotValid(NeedKernelVersionLE(ver))
+			assert.False(result)
+
+			result = tc.NotValid(NeedKernelVersionLT(ver))
+			assert.False(result)
+
+			result = tc.NotValid(NeedKernelVersionNotEquals(ver))
+			assert.False(result)
+		}
+	}
+}
+
+func TestConstraintNotValidDistroVersion(t *testing.T) {
+	assert := assert.New(t)
+
+	assert.NotNil(distroVersion)
+
+	// Generate new distro versions for testing purposes based on the
+	// current kernel version.
+	higherVersion, err := incrementVersion(distroVersion)
+	assert.NoError(err)
+	assert.NotEqual(distroVersion, higherVersion)
+
+	lowerVersion, err := decrementVersion(distroVersion)
+	assert.NoError(err)
+	assert.NotEqual(distroVersion, lowerVersion)
+
+	for _, debug := range []bool{true, false} {
+		tc := NewTestConstraint(debug)
+
+		result := tc.NotValid(NeedDistroVersionEquals(higherVersion))
+		assert.True(result)
+
+		result = tc.NotValid(NeedDistroVersionEquals(distroVersion))
+		assert.False(result)
+
+		result = tc.NotValid(NeedDistroVersionLE(higherVersion))
+		assert.False(result)
+
+		result = tc.NotValid(NeedDistroVersionLE(distroVersion))
+		assert.False(result)
+
+		result = tc.NotValid(NeedDistroVersionLT(higherVersion))
+		assert.False(result)
+
+		result = tc.NotValid(NeedDistroVersionLT(distroVersion))
+		assert.True(result)
+
+		result = tc.NotValid(NeedDistroVersionGE(higherVersion))
+		assert.True(result)
+
+		result = tc.NotValid(NeedDistroVersionGE(distroVersion))
+		assert.False(result)
+
+		result = tc.NotValid(NeedDistroVersionGT(higherVersion))
+		assert.True(result)
+
+		result = tc.NotValid(NeedDistroVersionGT(distroVersion))
+		assert.True(result)
+
+		result = tc.NotValid(NeedDistroVersionNotEquals(higherVersion))
+		assert.False(result)
+
+		result = tc.NotValid(NeedDistroVersionNotEquals(distroVersion))
+		assert.True(result)
+	}
+}
+
+func TestConstraintConstraintValid(t *testing.T) {
 	assert := assert.New(t)
 
 	type testData struct {
@@ -1312,7 +1494,7 @@ func TestConstraintConstraintInvalid(t *testing.T) {
 		for i, d := range data {
 			tc := NewTestConstraint(debug)
 
-			result := tc.constraintInvalid(d.fn)
+			result := tc.constraintValid(d.fn)
 
 			msg := fmt.Sprintf("test[%d]: %+v, result: %v", i, d, result)
 
@@ -1321,13 +1503,13 @@ func TestConstraintConstraintInvalid(t *testing.T) {
 			}
 
 			if d.valid {
-				assert.False(result, msg)
+				assert.True(result, msg)
 
 				if len(d.expected.Passed) != 0 {
 					assert.Equal(d.expected.Passed[0].Success, tc.Passed[0].Success, msg)
 				}
 			} else {
-				assert.True(result, msg)
+				assert.False(result, msg)
 
 				if len(d.expected.Failed) != 0 {
 					assert.Equal(d.expected.Failed[0].Success, tc.Failed[0].Success, msg)