Skip to content

Improve output #99

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.

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 9 commits into from
Dec 12, 2020
14 changes: 3 additions & 11 deletions check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package check

import (
"fmt"
"os"

"github.com/arduino/arduino-check/check/checkconfigurations"
"github.com/arduino/arduino-check/check/checkdata"
Expand All @@ -33,15 +32,14 @@ import (

// RunChecks runs all checks for the given project and outputs the results.
func RunChecks(project project.Type) {
feedback.Printf("Checking %s in %s\n", project.ProjectType, project.Path)
feedback.Printf("\nChecking %s in %s\n", project.ProjectType, project.Path)

checkdata.Initialize(project, configuration.SchemasPath())

for _, checkConfiguration := range checkconfigurations.Configurations() {
runCheck, err := shouldRun(checkConfiguration, project)
if err != nil {
feedback.Errorf("Error while determining whether to run check: %v", err)
os.Exit(1)
panic(err)
}

if !runCheck {
Expand All @@ -55,15 +53,9 @@ func RunChecks(project project.Type) {
checkResult, checkOutput := checkConfiguration.CheckFunction()
reportText := result.Results.Record(project, checkConfiguration, checkResult, checkOutput)
if (checkResult == checkresult.Fail) || configuration.Verbose() {
feedback.Print(reportText)
feedback.Println(reportText)
}
}

// Checks are finished for this project, so summarize its check results in the report.
result.Results.AddProjectSummary(project)

// Print the project check results summary.
feedback.Print(result.Results.ProjectSummaryText(project))
}

// shouldRun returns whether a given check should be run for the given project under the current tool configuration.
Expand Down
2 changes: 1 addition & 1 deletion check/checkdata/schema/parsevalidationresult.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,10 @@ func validationErrorSchemaPointerValueMatch(
schemasPath *paths.Path,
) bool {
marshalledSchemaPointerValue, err := json.Marshal(schemaPointerValue(schemaURL, schemaPointer, schemasPath))
logrus.Tracef("Checking schema pointer value: %s match with regexp: %s", marshalledSchemaPointerValue, schemaPointerValueRegexp)
if err != nil {
panic(err)
}
logrus.Tracef("Checking schema pointer value: %s match with regexp: %s", marshalledSchemaPointerValue, schemaPointerValueRegexp)
return schemaPointerValueRegexp.Match(marshalledSchemaPointerValue)
}

Expand Down
8 changes: 7 additions & 1 deletion command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ func ArduinoCheck(rootCommand *cobra.Command, cliArguments []string) {

for _, project := range projects {
check.RunChecks(project)

// Checks are finished for this project, so summarize its check results in the report.
result.Results.AddProjectSummary(project)

// Print the project check results summary.
feedback.Printf("\n%s\n", result.Results.ProjectSummaryText(project))
}

// All projects have been checked, so summarize their check results in the report.
Expand All @@ -75,7 +81,7 @@ func ArduinoCheck(rootCommand *cobra.Command, cliArguments []string) {
if configuration.OutputFormat() == outputformat.Text {
if len(projects) > 1 {
// There are multiple projects, print the summary of check results for all projects.
fmt.Print(result.Results.SummaryText())
fmt.Printf("\n%s\n", result.Results.SummaryText())
}
} else {
// Print the complete JSON formatted report.
Expand Down
6 changes: 4 additions & 2 deletions configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,15 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error {
}
}

logrus.SetOutput(defaultLogOutput)

if logFormatString, ok := os.LookupEnv("ARDUINO_CHECK_LOG_FORMAT"); ok {
logFormat, err := logFormatFromString(logFormatString)
if err != nil {
return fmt.Errorf("--log-format flag value %s not valid", logFormatString)
}
logrus.SetFormatter(logFormat)
logrus.SetOutput(os.Stderr) // Enable log output.
}

if logLevelString, ok := os.LookupEnv("ARDUINO_CHECK_LOG_LEVEL"); ok {
Expand All @@ -70,8 +73,7 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error {
return fmt.Errorf("--log-level flag value %s not valid", logLevelString)
}
logrus.SetLevel(logLevel)
} else {
logrus.SetLevel(defaultLogLevel)
logrus.SetOutput(os.Stderr) // Enable log output.
}

superprojectTypeFilterString, _ := flags.GetString("project-type")
Expand Down
1 change: 0 additions & 1 deletion configuration/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ func TestInitializeLogFormat(t *testing.T) {

func TestInitializeLogLevel(t *testing.T) {
require.Nil(t, Initialize(test.ConfigurationFlags(), projectPaths))
assert.Equal(t, defaultLogLevel, logrus.GetLevel(), "Default level")

os.Setenv("ARDUINO_CHECK_LOG_LEVEL", "foo")
assert.Error(t, Initialize(test.ConfigurationFlags(), projectPaths), "Invalid level")
Expand Down
5 changes: 3 additions & 2 deletions configuration/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package configuration
// The default configuration settings.

import (
"io/ioutil"

"github.com/arduino/arduino-check/configuration/checkmode"
"github.com/arduino/arduino-check/project/projecttype"
"github.com/sirupsen/logrus"
)

// Default check modes for each superproject type.
Expand Down Expand Up @@ -60,4 +61,4 @@ var defaultCheckModes = map[projecttype.Type]map[checkmode.Type]bool{
},
}

var defaultLogLevel = logrus.FatalLevel
var defaultLogOutput = ioutil.Discard // Default to no log output.
31 changes: 22 additions & 9 deletions result/feedback/feedback.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,56 @@ package feedback

import (
"fmt"
"os"

"github.com/arduino/arduino-check/configuration"
"github.com/arduino/arduino-check/result/outputformat"
"github.com/sirupsen/logrus"
)

// VerbosePrintln behaves like Println but only prints when verbosity is enabled.
func VerbosePrintln(v ...interface{}) {
VerbosePrint(v...)
VerbosePrint("\n")
}

// VerbosePrintf behaves like Printf but only prints when verbosity is enabled.
func VerbosePrintf(format string, v ...interface{}) {
VerbosePrint(fmt.Sprintf(format, v...))
}

// VerbosePrint behaves like Print but only prints when verbosity is enabled.
func VerbosePrint(message string) {
func VerbosePrint(v ...interface{}) {
if configuration.Verbose() && (configuration.OutputFormat() == outputformat.Text) {
Printf(message)
Print(v...)
}
}

// Println behaves like fmt.Println but only prints when output format is set to `text`.
func Println(v ...interface{}) {
Print(v...)
Print("\n")
}

// Printf behaves like fmt.Printf but only prints when output format is set to `text`.
func Printf(format string, v ...interface{}) {
Print(fmt.Sprintf(format, v...))
}

// Print behaves like fmt.Print but only prints when output format is set to `text`.
func Print(message string) {
func Print(v ...interface{}) {
if configuration.OutputFormat() == outputformat.Text {
fmt.Printf(message)
fmt.Print(v...)
}
}

// Errorf behaves like fmt.Printf but also logs the error.
// Errorf behaves like fmt.Printf but adds a newline and also logs the error.
func Errorf(format string, v ...interface{}) {
Error(fmt.Sprintf(format, v...))
}

// Error behaves like fmt.Print but also logs the error.
func Error(errorMessage string) {
fmt.Printf(errorMessage)
logrus.Error(fmt.Sprint(errorMessage))
// Error behaves like fmt.Print but adds a newline and also logs the error.
func Error(v ...interface{}) {
fmt.Fprintln(os.Stderr, v...)
logrus.Error(fmt.Sprint(v...))
}
13 changes: 5 additions & 8 deletions result/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@ import (
"encoding/json"
"fmt"
"html/template"
"os"

"github.com/arduino/arduino-check/check/checkconfigurations"
"github.com/arduino/arduino-check/check/checklevel"
"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/project"
"github.com/arduino/arduino-check/result/feedback"
"github.com/arduino/go-paths-helper"
)

Expand Down Expand Up @@ -95,11 +93,10 @@ func (results *Type) Initialize() {
func (results *Type) Record(checkedProject project.Type, checkConfiguration checkconfigurations.Type, checkResult checkresult.Type, checkOutput string) string {
checkLevel, err := checklevel.CheckLevel(checkConfiguration, checkResult)
if err != nil {
feedback.Errorf("Error while determining check level: %v", err)
os.Exit(1)
panic(fmt.Errorf("Error while determining check level: %v", err))
}

summaryText := fmt.Sprintf("Check %s result: %s\n", checkConfiguration.ID, checkResult)
summaryText := fmt.Sprintf("Check %s result: %s", checkConfiguration.ID, checkResult)

checkMessage := ""
if checkResult == checkresult.Fail {
Expand All @@ -112,7 +109,7 @@ func (results *Type) Record(checkedProject project.Type, checkConfiguration chec

// Add explanation of check result if present.
if checkMessage != "" {
summaryText += fmt.Sprintf("%s: %s\n", checkLevel, checkMessage)
summaryText += fmt.Sprintf("\n%s: %s", checkLevel, checkMessage)
}

reportExists, projectReportIndex := results.getProjectReportIndex(checkedProject.Path)
Expand Down Expand Up @@ -187,7 +184,7 @@ func (results Type) ProjectSummaryText(checkedProject project.Type) string {
}

projectSummaryReport := results.Projects[projectReportIndex].Summary
return fmt.Sprintf("\nFinished checking project. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v\n\n", projectSummaryReport.WarningCount, projectSummaryReport.ErrorCount, projectSummaryReport.Pass)
return fmt.Sprintf("Finished checking project. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v", projectSummaryReport.WarningCount, projectSummaryReport.ErrorCount, projectSummaryReport.Pass)
}

// AddSummary summarizes the check results for all projects and adds it to the report.
Expand All @@ -212,7 +209,7 @@ func (results *Type) AddSummary() {

// SummaryText returns a text summary of the cumulative check results.
func (results Type) SummaryText() string {
return fmt.Sprintf("Finished checking projects. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v\n", results.Summary.WarningCount, results.Summary.ErrorCount, results.Summary.Pass)
return fmt.Sprintf("Finished checking projects. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v", results.Summary.WarningCount, results.Summary.ErrorCount, results.Summary.Pass)
}

// JSONReport returns a JSON formatted report of checks on all projects.
Expand Down
8 changes: 4 additions & 4 deletions result/result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ func TestRecord(t *testing.T) {
checkConfiguration := checkconfigurations.Configurations()[0]
checkOutput := "foo"
summaryText := results.Record(checkedProject, checkConfiguration, checkresult.Fail, checkOutput)
assert.Equal(t, fmt.Sprintf("Check %s result: %s\n%s: %s\n", checkConfiguration.ID, checkresult.Fail, checklevel.Error, message(checkConfiguration.MessageTemplate, checkOutput)), summaryText)
assert.Equal(t, fmt.Sprintf("Check %s result: %s\n%s: %s", checkConfiguration.ID, checkresult.Fail, checklevel.Error, message(checkConfiguration.MessageTemplate, checkOutput)), summaryText)
summaryText = results.Record(checkedProject, checkConfiguration, checkresult.NotRun, checkOutput)
assert.Equal(t, fmt.Sprintf("Check %s result: %s\n%s: %s\n", checkConfiguration.ID, checkresult.NotRun, checklevel.Notice, checkOutput), summaryText, "Non-fail result should not use message")
assert.Equal(t, fmt.Sprintf("Check %s result: %s\n%s: %s", checkConfiguration.ID, 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")

Expand Down Expand Up @@ -198,7 +198,7 @@ func TestAddProjectSummary(t *testing.T) {
assert.Equal(t, testTable.expectedPass, results.Projects[0].Summary.Pass)
assert.Equal(t, testTable.expectedWarningCount, results.Projects[0].Summary.WarningCount)
assert.Equal(t, testTable.expectedErrorCount, results.Projects[0].Summary.ErrorCount)
assert.Equal(t, fmt.Sprintf("\nFinished checking project. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v\n\n", testTable.expectedWarningCount, testTable.expectedErrorCount, testTable.expectedPass), results.ProjectSummaryText(checkedProject))
assert.Equal(t, fmt.Sprintf("Finished checking project. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v", testTable.expectedWarningCount, testTable.expectedErrorCount, testTable.expectedPass), results.ProjectSummaryText(checkedProject))
}
}

Expand Down Expand Up @@ -281,7 +281,7 @@ func TestAddSummary(t *testing.T) {
assert.Equal(t, testTable.expectedPass, results.Passed())
assert.Equal(t, testTable.expectedWarningCount, results.Summary.WarningCount)
assert.Equal(t, testTable.expectedErrorCount, results.Summary.ErrorCount)
assert.Equal(t, fmt.Sprintf("Finished checking projects. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v\n", testTable.expectedWarningCount, testTable.expectedErrorCount, testTable.expectedPass), results.SummaryText())
assert.Equal(t, fmt.Sprintf("Finished checking projects. Results:\nWarning count: %v\nError count: %v\nChecks passed: %v", testTable.expectedWarningCount, testTable.expectedErrorCount, testTable.expectedPass), results.SummaryText())
}
}

Expand Down