From db86b5c97caa8efd64fd5d6399c2128db5bd7d65 Mon Sep 17 00:00:00 2001 From: per1234 Date: Tue, 8 Dec 2020 19:54:50 -0800 Subject: [PATCH 1/2] Initialize results data in result.Initialize() It seems like the behavior that would be expected from the function. --- result/result.go | 1 + result/result_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/result/result.go b/result/result.go index b91478131..5b3c0db91 100644 --- a/result/result.go +++ b/result/result.go @@ -83,6 +83,7 @@ type summaryReportType struct { // Initialize adds the tool configuration data to the results data. func (results *Type) Initialize() { + *results = *new(Type) results.Configuration = toolConfigurationReportType{ Paths: configuration.TargetPaths(), ProjectType: configuration.SuperprojectTypeFilter().String(), diff --git a/result/result_test.go b/result/result_test.go index c8f2cdfa1..3009843bb 100644 --- a/result/result_test.go +++ b/result/result_test.go @@ -69,8 +69,8 @@ func TestRecord(t *testing.T) { assert.Equal(t, "", "", summaryText, "Non-failure result with no check function output should result in an empty summary") checkResult := checkresult.Pass - results = Type{} + results.Initialize() results.Record(checkedProject, checkConfiguration, checkResult, checkOutput) projectReport := results.Projects[0] assert.Equal(t, checkedProject.Path, projectReport.Path) From 7dcfd5fb370d147aa8445e1995cdaa570137f8f0 Mon Sep 17 00:00:00 2001 From: per1234 Date: Tue, 8 Dec 2020 18:26:50 -0800 Subject: [PATCH 2/2] Add --verbose flag Previously, the tool alway provides output for every check it runs. This is not necessarily a bad thing because in `--format text` mode it provides a progress indicator, and in `--format json` mode it provides information on all the checks that are run under the current tool configuration. However, as the number of checks grows, the amount of output has become a bit overwhelming and may make it hard to find the information for the failed checks. So it makes sense to default to only showing information on the failed checks while providing the option to show all checks. --- check/check.go | 7 ++- cli/cli.go | 1 + configuration/configuration.go | 10 +++++ configuration/configuration_test.go | 12 +++++ result/feedback/feedback.go | 12 +++++ result/result.go | 32 ++++++------- result/result_test.go | 70 +++++++++++++++++++++++------ util/test/test.go | 1 + 8 files changed, 113 insertions(+), 32 deletions(-) diff --git a/check/check.go b/check/check.go index 7eb950027..a659622d3 100644 --- a/check/check.go +++ b/check/check.go @@ -22,6 +22,7 @@ import ( "github.com/arduino/arduino-check/check/checkconfigurations" "github.com/arduino/arduino-check/check/checkdata" + "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" @@ -49,11 +50,13 @@ func RunChecks(project project.Type) { } // Output will be printed after all checks are finished when configured for "json" output format. - feedback.Printf("Running check %s: ", checkConfiguration.ID) + feedback.VerbosePrintf("Running check %s...\n", checkConfiguration.ID) checkResult, checkOutput := checkConfiguration.CheckFunction() reportText := result.Results.Record(project, checkConfiguration, checkResult, checkOutput) - feedback.Print(reportText) + if (checkResult == checkresult.Fail) || configuration.Verbose() { + feedback.Print(reportText) + } } // Checks are finished for this project, so summarize its check results in the report. diff --git a/cli/cli.go b/cli/cli.go index 48be62221..dd4504d6e 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -37,6 +37,7 @@ func Root() *cobra.Command { rootCommand.PersistentFlags().String("project-type", "all", "Only check projects of the specified type and their subprojects. Can be {sketch|library|all}.") rootCommand.PersistentFlags().Bool("recursive", true, "Search path recursively for Arduino projects to check. Can be {true|false}.") rootCommand.PersistentFlags().String("report-file", "", "Save a report on the checks to this file.") + rootCommand.PersistentFlags().BoolP("verbose", "v", false, "Show more information while running checks.") rootCommand.PersistentFlags().Bool("version", false, "Print version and timestamp of the build.") return rootCommand diff --git a/configuration/configuration.go b/configuration/configuration.go index e93d35271..63c471518 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -85,6 +85,8 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error { reportFilePathString, _ := flags.GetString("report-file") reportFilePath = paths.New(reportFilePathString) + verbose, _ = flags.GetBool("verbose") + versionMode, _ = flags.GetBool("version") targetPaths = nil @@ -125,6 +127,7 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error { "superproject type filter": SuperprojectTypeFilter(), "recursive": Recursive(), "report file": ReportFilePath(), + "verbose": Verbose(), "projects path": TargetPaths(), }).Debug("Configuration initialized") @@ -178,6 +181,13 @@ func ReportFilePath() *paths.Path { return reportFilePath } +var verbose bool + +// Verbose returns the verbosity setting. +func Verbose() bool { + return verbose +} + var versionMode bool func VersionMode() bool { diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index c3fa325fb..29e510ed6 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -194,6 +194,18 @@ func TestInitializeVersion(t *testing.T) { assert.False(t, VersionMode()) } +func TestInitializeVerbose(t *testing.T) { + flags := test.ConfigurationFlags() + + flags.Set("verbose", "true") + assert.Nil(t, Initialize(flags, projectPaths)) + assert.True(t, Verbose()) + + flags.Set("verbose", "false") + assert.Nil(t, Initialize(flags, projectPaths)) + assert.False(t, Verbose()) +} + func TestInitializeProjectPath(t *testing.T) { assert.Nil(t, Initialize(test.ConfigurationFlags(), []string{})) workingDirectoryPath, err := os.Getwd() diff --git a/result/feedback/feedback.go b/result/feedback/feedback.go index 6e14a9ea7..bf603aa14 100644 --- a/result/feedback/feedback.go +++ b/result/feedback/feedback.go @@ -24,6 +24,18 @@ import ( "github.com/sirupsen/logrus" ) +// 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) { + if configuration.Verbose() && (configuration.OutputFormat() == outputformat.Text) { + Printf(message) + } +} + // 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...)) diff --git a/result/result.go b/result/result.go index 5b3c0db91..3bc592ed8 100644 --- a/result/result.go +++ b/result/result.go @@ -99,7 +99,7 @@ func (results *Type) Record(checkedProject project.Type, checkConfiguration chec os.Exit(1) } - summaryText := fmt.Sprintf("%s\n", checkResult) + summaryText := fmt.Sprintf("Check %s result: %s\n", checkConfiguration.ID, checkResult) checkMessage := "" if checkLevel == checklevel.Error { @@ -115,17 +115,6 @@ func (results *Type) Record(checkedProject project.Type, checkConfiguration chec summaryText += fmt.Sprintf("%s: %s\n", checkLevel, checkMessage) } - checkReport := checkReportType{ - Category: checkConfiguration.Category, - Subcategory: checkConfiguration.Subcategory, - ID: checkConfiguration.ID, - Brief: checkConfiguration.Brief, - Description: checkConfiguration.Description, - Result: checkResult.String(), - Level: checkLevel.String(), - Message: checkMessage, - } - reportExists, projectReportIndex := results.getProjectReportIndex(checkedProject.Path) if !reportExists { // There is no existing report for this project. @@ -140,11 +129,22 @@ func (results *Type) Record(checkedProject project.Type, checkConfiguration chec LibraryManagerUpdate: configuration.CheckModes(checkedProject.ProjectType)[checkmode.LibraryManagerIndexed], Official: configuration.CheckModes(checkedProject.ProjectType)[checkmode.Official], }, - Checks: []checkReportType{checkReport}, + Checks: []checkReportType{}, }, ) - } else { - // There's already a report for this project; just add the checks report to it. + } + + if (checkResult == checkresult.Fail) || configuration.Verbose() { + checkReport := checkReportType{ + Category: checkConfiguration.Category, + Subcategory: checkConfiguration.Subcategory, + ID: checkConfiguration.ID, + Brief: checkConfiguration.Brief, + Description: checkConfiguration.Description, + Result: checkResult.String(), + Level: checkLevel.String(), + Message: checkMessage, + } results.Projects[projectReportIndex].Checks = append(results.Projects[projectReportIndex].Checks, checkReport) } @@ -266,7 +266,7 @@ func (results Type) getProjectReportIndex(projectPath *paths.Path) (bool, int) { } // There is no element in the report for this project. - return false, index + 1 + return false, len(results.Projects) } // message fills the message template provided by the check configuration with the check output. diff --git a/result/result_test.go b/result/result_test.go index 3009843bb..735b673ea 100644 --- a/result/result_test.go +++ b/result/result_test.go @@ -34,6 +34,16 @@ import ( "github.com/stretchr/testify/require" ) +var projectPaths []string + +func init() { + projectPath, err := os.Getwd() // Path to an arbitrary folder that is guaranteed to exist. + if err != nil { + panic(err) + } + projectPaths = []string{projectPath} +} + func TestInitialize(t *testing.T) { flags := test.ConfigurationFlags() flags.Set("project-type", "sketch") @@ -52,6 +62,9 @@ func TestInitialize(t *testing.T) { } func TestRecord(t *testing.T) { + flags := test.ConfigurationFlags() + require.Nil(t, configuration.Initialize(flags, projectPaths)) + checkedProject := project.Type{ Path: paths.New("/foo/bar"), ProjectType: projecttype.Sketch, @@ -59,17 +72,19 @@ func TestRecord(t *testing.T) { } var results Type + results.Initialize() checkConfiguration := checkconfigurations.Configurations()[0] checkOutput := "foo" 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) + assert.Equal(t, fmt.Sprintf("Check %s result: %s\n%s: %s\n", checkConfiguration.ID, 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") + 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") 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") + flags.Set("verbose", "true") + require.Nil(t, configuration.Initialize(flags, projectPaths)) checkResult := checkresult.Pass - results.Initialize() results.Record(checkedProject, checkConfiguration, checkResult, checkOutput) projectReport := results.Projects[0] @@ -80,7 +95,7 @@ func TestRecord(t *testing.T) { assert.Equal(t, configuration.CheckModes(checkedProject.ProjectType)[checkmode.LibraryManagerSubmission], projectConfigurationReport.LibraryManagerSubmit) assert.Equal(t, configuration.CheckModes(checkedProject.ProjectType)[checkmode.LibraryManagerIndexed], projectConfigurationReport.LibraryManagerUpdate) assert.Equal(t, configuration.CheckModes(checkedProject.ProjectType)[checkmode.Official], projectConfigurationReport.Official) - + assert.Equal(t, 1, len(results.Projects[0].Checks), "Passing check reports should be written to report in verbose mode") checkReport := projectReport.Checks[0] assert.Equal(t, checkConfiguration.Category, checkReport.Category) assert.Equal(t, checkConfiguration.Subcategory, checkReport.Subcategory) @@ -92,15 +107,25 @@ func TestRecord(t *testing.T) { assert.Equal(t, checkLevel.String(), checkReport.Level) assert.Equal(t, checkOutput, checkReport.Message) + flags.Set("verbose", "false") + require.Nil(t, configuration.Initialize(flags, projectPaths)) + results.Initialize() + results.Record(checkedProject, checkConfiguration, checkresult.Pass, checkOutput) + assert.Equal(t, 0, len(results.Projects[0].Checks), "Passing check reports should not be written to report in non-verbose mode") + + results.Initialize() + results.Record(checkedProject, checkConfiguration, checkresult.Fail, checkOutput) + require.Equal(t, 1, len(projectReport.Checks), "Failing check reports should be written to report in non-verbose mode") + assert.Len(t, results.Projects, 1) previousProjectPath := checkedProject.Path checkedProject.Path = paths.New("/foo/baz") - results.Record(checkedProject, checkConfiguration, checkResult, checkOutput) + results.Record(checkedProject, checkConfiguration, checkresult.Fail, checkOutput) assert.Len(t, results.Projects, 2) assert.Len(t, results.Projects[0].Checks, 1) checkedProject.Path = previousProjectPath - results.Record(checkedProject, checkconfigurations.Configurations()[1], checkResult, checkOutput) + results.Record(checkedProject, checkconfigurations.Configurations()[1], checkresult.Fail, checkOutput) assert.Len(t, results.Projects[0].Checks, 2) } @@ -114,6 +139,7 @@ func TestAddProjectSummary(t *testing.T) { testTables := []struct { results []checkresult.Type levels []checklevel.Type + verbose string expectedPass bool expectedWarningCount int expectedErrorCount int @@ -121,6 +147,15 @@ func TestAddProjectSummary(t *testing.T) { { []checkresult.Type{checkresult.Pass, checkresult.Pass}, []checklevel.Type{checklevel.Info, checklevel.Info}, + "true", + true, + 0, + 0, + }, + { + []checkresult.Type{checkresult.Pass, checkresult.Pass}, + []checklevel.Type{checklevel.Info, checklevel.Info}, + "false", true, 0, 0, @@ -128,6 +163,7 @@ func TestAddProjectSummary(t *testing.T) { { []checkresult.Type{checkresult.Pass, checkresult.Fail}, []checklevel.Type{checklevel.Info, checklevel.Warning}, + "false", true, 1, 0, @@ -135,6 +171,7 @@ func TestAddProjectSummary(t *testing.T) { { []checkresult.Type{checkresult.Fail, checkresult.Fail}, []checklevel.Type{checklevel.Error, checklevel.Warning}, + "false", false, 1, 1, @@ -142,12 +179,21 @@ func TestAddProjectSummary(t *testing.T) { } for _, testTable := range testTables { + flags := test.ConfigurationFlags() + flags.Set("verbose", testTable.verbose) + require.Nil(t, configuration.Initialize(flags, projectPaths)) + var results Type - for _, result := range testTable.results { + results.Initialize() + + checkIndex := 0 + for testDataIndex, result := range testTable.results { results.Record(checkedProject, checkconfigurations.Configurations()[0], result, "") - } - for checkIndex, level := range testTable.levels { - results.Projects[0].Checks[checkIndex].Level = level.String() + if (result == checkresult.Fail) || configuration.Verbose() { + level := testTable.levels[testDataIndex].String() + results.Projects[0].Checks[checkIndex].Level = level + checkIndex += 1 + } } results.AddProjectSummary(checkedProject) assert.Equal(t, testTable.expectedPass, results.Projects[0].Summary.Pass) @@ -243,10 +289,6 @@ func TestAddSummary(t *testing.T) { func TestWriteReport(t *testing.T) { flags := test.ConfigurationFlags() - projectPath, err := os.Getwd() // Path to an arbitrary folder that is guaranteed to exist. - require.Nil(t, err) - projectPaths := []string{projectPath} - reportFolderPathString, err := ioutil.TempDir("", "arduino-check-result-TestWriteReport") require.Nil(t, err) defer os.RemoveAll(reportFolderPathString) // clean up diff --git a/util/test/test.go b/util/test/test.go index d4f3e15ed..f9179bae7 100644 --- a/util/test/test.go +++ b/util/test/test.go @@ -29,6 +29,7 @@ func ConfigurationFlags() *pflag.FlagSet { flags.String("project-type", "all", "") flags.Bool("recursive", true, "") flags.String("report-file", "", "") + flags.Bool("verbose", false, "") flags.Bool("version", false, "") return flags