Return effective check level from checklevel.CheckLevel()

Previously, the level returned was for a failure result, regardless of the actual result of the check, with the
effective check level adjusted according to the check result in the result package. The code is more maintainable and
easier to understand if the check level is determined in a single location. The checklevel package is the logical place
for that.

Author: per1234

Diff for: check/checkconfigurations/checkconfigurations_test.go

@@ -32,7 +32,7 @@ func TestConfigurationResolution(t *testing.T) {
 			enabled, err := check.IsEnabled(checkConfiguration, map[checkmode.Type]bool{checkMode: true})
 			assert.Nil(t, err, fmt.Sprintf("Enable configuration of check %s doesn't resolve for check mode %s", checkConfiguration.ID, checkMode))
 			if err == nil && enabled {
-				_, err := checklevel.CheckLevelForCheckModes(checkConfiguration, map[checkmode.Type]bool{checkMode: true})
+				_, err := checklevel.FailCheckLevel(checkConfiguration, map[checkmode.Type]bool{checkMode: true})
 				assert.Nil(t, err, fmt.Sprintf("Level configuration of check %s doesn't resolve for check mode %s", checkConfiguration.ID, checkMode))
 			}
 		}

Diff for: check/checklevel/checklevel.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/arduino/arduino-check/check/checkconfigurations"
+	"github.com/arduino/arduino-check/check/checkresult"
 	"github.com/arduino/arduino-check/configuration"
 	"github.com/arduino/arduino-check/configuration/checkmode"
 )
@@ -36,13 +37,17 @@ const (
 	Notice              // notice
 )
 
-// CheckLevel determines the check level assigned to failure of the given check under the current tool configuration.
-func CheckLevel(checkConfiguration checkconfigurations.Type) (Type, error) {
+// CheckLevel determines the check level assigned to the given result of the given check under the current tool configuration.
+func CheckLevel(checkConfiguration checkconfigurations.Type, checkResult checkresult.Type) (Type, error) {
+	if checkResult != checkresult.Fail {
+		return Notice, nil // Level provided by FailCheckLevel() is only relevant for failure result.
+	}
 	configurationCheckModes := configuration.CheckModes(checkConfiguration.ProjectType)
-	return CheckLevelForCheckModes(checkConfiguration, configurationCheckModes)
+	return FailCheckLevel(checkConfiguration, configurationCheckModes)
 }
 
-func CheckLevelForCheckModes(checkConfiguration checkconfigurations.Type, configurationCheckModes map[checkmode.Type]bool) (Type, error) {
+// FailCheckLevel determines the level of a failed check for the given check modes.
+func FailCheckLevel(checkConfiguration checkconfigurations.Type, configurationCheckModes map[checkmode.Type]bool) (Type, error) {
 	for _, errorMode := range checkConfiguration.ErrorModes {
 		if configurationCheckModes[errorMode] {
 			return Error, nil

Diff for: check/checklevel/checklevel_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/arduino/arduino-check/check/checkconfigurations"
+	"github.com/arduino/arduino-check/check/checkresult"
 	"github.com/arduino/arduino-check/configuration"
 	"github.com/arduino/arduino-check/configuration/checkmode"
 	"github.com/arduino/arduino-check/util/test"
@@ -31,19 +32,21 @@ func TestCheckLevel(t *testing.T) {
 		infoModes             []checkmode.Type
 		warningModes          []checkmode.Type
 		errorModes            []checkmode.Type
+		checkResult           checkresult.Type
 		libraryManagerSetting string
 		permissiveSetting     string
 		expectedLevel         Type
 		errorAssertion        assert.ErrorAssertionFunc
 	}{
-		{"Error", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "submit", "false", Error, assert.NoError},
-		{"Warning", []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{}, "submit", "false", Warning, assert.NoError},
-		{"Info", []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{}, []checkmode.Type{}, "submit", "false", Info, assert.NoError},
-		{"Default to Error", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{checkmode.Default}, "submit", "false", Error, assert.NoError},
-		{"Default to Warning", []checkmode.Type{}, []checkmode.Type{checkmode.Default}, []checkmode.Type{}, "submit", "false", Warning, assert.NoError},
-		{"Default to Info", []checkmode.Type{checkmode.Default}, []checkmode.Type{}, []checkmode.Type{}, "submit", "false", Info, assert.NoError},
-		{"Default override", []checkmode.Type{checkmode.Default}, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, "submit", "false", Error, assert.NoError},
-		{"Unable to resolve", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{}, "submit", "false", Info, assert.Error},
+		{"Non-fail", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, checkresult.Skip, "submit", "false", Notice, assert.NoError},
+		{"Error", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, checkresult.Fail, "submit", "false", Error, assert.NoError},
+		{"Warning", []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{}, checkresult.Fail, "submit", "false", Warning, assert.NoError},
+		{"Info", []checkmode.Type{checkmode.LibraryManagerSubmission}, []checkmode.Type{}, []checkmode.Type{}, checkresult.Fail, "submit", "false", Info, assert.NoError},
+		{"Default to Error", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{checkmode.Default}, checkresult.Fail, "submit", "false", Error, assert.NoError},
+		{"Default to Warning", []checkmode.Type{}, []checkmode.Type{checkmode.Default}, []checkmode.Type{}, checkresult.Fail, "submit", "false", Warning, assert.NoError},
+		{"Default to Info", []checkmode.Type{checkmode.Default}, []checkmode.Type{}, []checkmode.Type{}, checkresult.Fail, "submit", "false", Info, assert.NoError},
+		{"Default override", []checkmode.Type{checkmode.Default}, []checkmode.Type{}, []checkmode.Type{checkmode.LibraryManagerSubmission}, checkresult.Fail, "submit", "false", Error, assert.NoError},
+		{"Unable to resolve", []checkmode.Type{}, []checkmode.Type{}, []checkmode.Type{}, checkresult.Fail, "submit", "false", Info, assert.Error},
 	}
 
 	flags := test.ConfigurationFlags()
@@ -60,10 +63,10 @@ func TestCheckLevel(t *testing.T) {
 			ErrorModes:   testTable.errorModes,
 		}
 
-		level, err := CheckLevel(checkConfiguration)
-		testTable.errorAssertion(t, err)
+		level, err := CheckLevel(checkConfiguration, testTable.checkResult)
+		testTable.errorAssertion(t, err, testTable.testName)
 		if err == nil {
-			assert.Equal(t, testTable.expectedLevel, level)
+			assert.Equal(t, testTable.expectedLevel, level, testTable.testName)
 		}
 	}
 }

Diff for: result/result.go

@@ -94,7 +94,7 @@ func (results *Type) Initialize() {
 func (results *Type) Record(checkedProject project.Type, checkConfiguration checkconfigurations.Type, checkResult checkresult.Type, checkOutput string) string {
 	checkMessage := message(checkConfiguration.MessageTemplate, checkOutput)
 
-	checkLevel, err := checklevel.CheckLevel(checkConfiguration)
+	checkLevel, err := checklevel.CheckLevel(checkConfiguration, checkResult)
 	if err != nil {
 		feedback.Errorf("Error while determining check level: %v", err)
 		os.Exit(1)

Diff for: result/result_test.go

@@ -90,7 +90,7 @@ func TestRecord(t *testing.T) {
 	assert.Equal(t, checkConfiguration.Brief, checkReport.Brief)
 	assert.Equal(t, checkConfiguration.Description, checkReport.Description)
 	assert.Equal(t, checkResult.String(), checkReport.Result)
-	checkLevel, _ := checklevel.CheckLevel(checkConfiguration)
+	checkLevel, _ := checklevel.CheckLevel(checkConfiguration, checkResult)
 	assert.Equal(t, checkLevel.String(), checkReport.Level)
 	assert.Equal(t, message(checkConfiguration.MessageTemplate, checkOutput), checkReport.Message)