diff --git a/check/checkconfigurations/checkconfigurations_test.go b/check/checkconfigurations/checkconfigurations_test.go index b8de7293..a0e13ba3 100644 --- a/check/checkconfigurations/checkconfigurations_test.go +++ b/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 --git a/check/checklevel/checklevel.go b/check/checklevel/checklevel.go index 55659bf9..17225d95 100644 --- a/check/checklevel/checklevel.go +++ b/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 --git a/check/checklevel/checklevel_test.go b/check/checklevel/checklevel_test.go index d15d766e..88c3d4d4 100644 --- a/check/checklevel/checklevel_test.go +++ b/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 --git a/result/result.go b/result/result.go index b70e53a3..b9147813 100644 --- a/result/result.go +++ b/result/result.go @@ -92,9 +92,7 @@ func (results *Type) Initialize() { // Record records the result of a check and returns a text summary for it. 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) @@ -102,10 +100,17 @@ func (results *Type) Record(checkedProject project.Type, checkConfiguration chec summaryText := fmt.Sprintf("%s\n", checkResult) - if checkResult == checkresult.NotRun { - // TODO: make the check functions output an explanation for why they didn't run - summaryText += fmt.Sprintf("%s: %s\n", checklevel.Notice, checkOutput) - } else if checkResult != checkresult.Pass { + checkMessage := "" + if checkLevel == checklevel.Error { + checkMessage = message(checkConfiguration.MessageTemplate, checkOutput) + } else { + // Checks may provide an explanation for their non-fail result. + // The message template should not be used in this case, since it is written for a failure result. + checkMessage = checkOutput + } + + // Add explanation of check result if present. + if checkMessage != "" { summaryText += fmt.Sprintf("%s: %s\n", checkLevel, checkMessage) } diff --git a/result/result_test.go b/result/result_test.go index 04da68a6..c8f2cdfa 100644 --- a/result/result_test.go +++ b/result/result_test.go @@ -61,12 +61,12 @@ func TestRecord(t *testing.T) { var results Type checkConfiguration := checkconfigurations.Configurations()[0] checkOutput := "foo" - summaryText := results.Record(checkedProject, checkConfiguration, checkresult.Pass, checkOutput) - assert.Equal(t, fmt.Sprintf("%s\n", checkresult.Pass), summaryText) - summaryText = results.Record(checkedProject, checkConfiguration, checkresult.NotRun, checkOutput) - assert.Equal(t, fmt.Sprintf("%s\n%s: %s\n", checkresult.NotRun, checklevel.Notice, checkOutput), summaryText) - summaryText = results.Record(checkedProject, checkConfiguration, checkresult.Fail, checkOutput) + summaryText := results.Record(checkedProject, checkConfiguration, checkresult.Fail, checkOutput) assert.Equal(t, fmt.Sprintf("%s\n%s: %s\n", checkresult.Fail, checklevel.Error, message(checkConfiguration.MessageTemplate, checkOutput)), summaryText) + summaryText = results.Record(checkedProject, checkConfiguration, checkresult.NotRun, checkOutput) + assert.Equal(t, fmt.Sprintf("%s\n%s: %s\n", checkresult.NotRun, checklevel.Notice, checkOutput), summaryText, "Non-fail result should not use message") + summaryText = results.Record(checkedProject, checkConfiguration, checkresult.Pass, "") + assert.Equal(t, "", "", summaryText, "Non-failure result with no check function output should result in an empty summary") checkResult := checkresult.Pass results = Type{} @@ -88,9 +88,9 @@ 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) + assert.Equal(t, checkOutput, checkReport.Message) assert.Len(t, results.Projects, 1) previousProjectPath := checkedProject.Path