Skip to content

Improve handling of check output #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 4, 2020
Merged

Improve handling of check output #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa59ce0
Suppress "notice:" output on check not run when there is no explanation
per1234 Dec 3, 2020
a74c218
Return effective check level from checklevel.CheckLevel()
per1234 Dec 3, 2020
e8ff10d
Store effective check message in report
per1234 Dec 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion check/checkconfigurations/checkconfigurations_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down
13 changes: 9 additions & 4 deletions check/checklevel/checklevel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand Down
25 changes: 14 additions & 11 deletions check/checklevel/checklevel_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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()
Expand All @@ -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)
}
}
}
19 changes: 12 additions & 7 deletions result/result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,20 +92,25 @@ 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)
}

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)
}

Expand Down
14 changes: 7 additions & 7 deletions result/result_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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
Expand Down