Merge pull request #99 from arduino/per1234/improve-output

Improve output

Author: per1234

Diff for: check/check.go

@@ -18,7 +18,6 @@ package check
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/arduino/arduino-check/check/checkconfigurations"
 	"github.com/arduino/arduino-check/check/checkdata"
@@ -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 {
@@ -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.

Diff for: check/checkdata/schema/parsevalidationresult.go

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

Diff for: command/command.go

@@ -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.
@@ -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.

Diff for: configuration/configuration.go

@@ -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 {
@@ -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")

Diff for: configuration/configuration_test.go

@@ -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")

Diff for: configuration/defaults.go

@@ -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.
@@ -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.

Diff for: result/feedback/feedback.go

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

Diff for: result/result.go

@@ -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"
 )
 
@@ -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 {
@@ -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)
@@ -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.
@@ -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.

Diff for: result/result_test.go

@@ -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")
 
@@ -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))
 	}
 }
 
@@ -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())
 	}
 }