From 48530c501a758a3761d64836856fcb5c58d2619f Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 23 Jun 2023 11:55:37 +0200 Subject: [PATCH 1/5] adjust substeps len of container setup --- legacy/builder/container_setup.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/legacy/builder/container_setup.go b/legacy/builder/container_setup.go index dd52af49b8f..3a00ff6edaa 100644 --- a/legacy/builder/container_setup.go +++ b/legacy/builder/container_setup.go @@ -23,14 +23,13 @@ import ( type ContainerSetupHardwareToolsLibsSketchAndProps struct{} func (s *ContainerSetupHardwareToolsLibsSketchAndProps) Run(ctx *types.Context) error { - // total number of steps in this container: 4 - ctx.Progress.AddSubSteps(4) - defer ctx.Progress.RemoveSubSteps() commands := []types.Command{ &AddAdditionalEntriesToContext{}, &FailIfBuildPathEqualsSketchPath{}, &LibrariesLoader{}, } + ctx.Progress.AddSubSteps(len(commands)) + defer ctx.Progress.RemoveSubSteps() for _, command := range commands { PrintRingNameIfDebug(ctx, command) err := command.Run(ctx) From 4f9cad23d0e4df9ba00de9c00137f2538f2f78d8 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 23 Jun 2023 12:36:23 +0200 Subject: [PATCH 2/5] run builder commands without the usage of runCommands func to explicitly handle the substeps --- legacy/builder/builder.go | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/legacy/builder/builder.go b/legacy/builder/builder.go index fe11d684faa..90d92938811 100644 --- a/legacy/builder/builder.go +++ b/legacy/builder/builder.go @@ -41,7 +41,7 @@ func (s *Builder) Run(ctx *types.Context) error { return err } - var _err error + var _err, mainErr error commands := []types.Command{ &ContainerSetupHardwareToolsLibsSketchAndProps{}, @@ -92,12 +92,25 @@ func (s *Builder) Run(ctx *types.Context) error { &RecipeByPrefixSuffixRunner{Prefix: "recipe.hooks.postbuild", Suffix: ".pattern", SkipIfOnlyUpdatingCompilationDatabase: true}, } - mainErr := runCommands(ctx, commands) + ctx.Progress.AddSubSteps(len(commands) + 4) + defer ctx.Progress.RemoveSubSteps() + + for _, command := range commands { + PrintRingNameIfDebug(ctx, command) + err := command.Run(ctx) + if err != nil { + mainErr = errors.WithStack(err) + break + } + ctx.Progress.CompleteStep() + ctx.PushProgress() + } if ctx.CompilationDatabase != nil { ctx.CompilationDatabase.SaveToFile() } + var otherErr error commands = []types.Command{ &PrintUsedAndNotUsedLibraries{SketchError: mainErr != nil}, @@ -107,7 +120,16 @@ func (s *Builder) Run(ctx *types.Context) error { &phases.Sizer{SketchError: mainErr != nil}, } - otherErr := runCommands(ctx, commands) + for _, command := range commands { + PrintRingNameIfDebug(ctx, command) + err := command.Run(ctx) + if err != nil { + otherErr = errors.WithStack(err) + break + } + ctx.Progress.CompleteStep() + ctx.PushProgress() + } if mainErr != nil { return mainErr @@ -193,8 +215,7 @@ func PrintRingNameIfDebug(ctx *types.Context, command types.Command) { } func RunBuilder(ctx *types.Context) error { - command := Builder{} - return command.Run(ctx) + return runCommands(ctx, []types.Command{&Builder{}}) } func RunParseHardware(ctx *types.Context) error { From 8d4ca45b32e0460cbb4ccff139ced75e7ce72c88 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 23 Jun 2023 12:37:07 +0200 Subject: [PATCH 3/5] set progress as completed when reaches progress 100 --- legacy/builder/types/context.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/legacy/builder/types/context.go b/legacy/builder/types/context.go index 8bbd05f7308..0a160324a46 100644 --- a/legacy/builder/types/context.go +++ b/legacy/builder/types/context.go @@ -200,7 +200,10 @@ func (ctx *Context) ExtractBuildOptions() *properties.Map { func (ctx *Context) PushProgress() { if ctx.ProgressCB != nil { - ctx.ProgressCB(&rpc.TaskProgress{Percent: ctx.Progress.Progress}) + ctx.ProgressCB(&rpc.TaskProgress{ + Percent: ctx.Progress.Progress, + Completed: ctx.Progress.Progress >= 100.0, + }) } } From 0b522404321690990ffa4c22eafb3851fb951d71 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 23 Jun 2023 14:00:40 +0200 Subject: [PATCH 4/5] add integration test --- .../integrationtest/daemon/daemon_test.go | 13 ++++++++ .../daemon/task_progress_test.go | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 internal/integrationtest/daemon/task_progress_test.go diff --git a/internal/integrationtest/daemon/daemon_test.go b/internal/integrationtest/daemon/daemon_test.go index 2c10183eaa1..e87d7d47f37 100644 --- a/internal/integrationtest/daemon/daemon_test.go +++ b/internal/integrationtest/daemon/daemon_test.go @@ -171,6 +171,7 @@ func TestDaemonCompileOptions(t *testing.T) { // Build sketch (without errors) compile, err = grpcInst.Compile(context.Background(), "arduino:avr:uno:some_menu=good", sk.String(), "") require.NoError(t, err) + analyzer := NewTaskProgressAnalyzer(t) for { msg, err := compile.Recv() if err == io.EOF { @@ -180,7 +181,19 @@ func TestDaemonCompileOptions(t *testing.T) { if msg.ErrStream != nil { fmt.Printf("COMPILE> %v\n", string(msg.GetErrStream())) } + analyzer.Process(msg.GetProgress()) } + // https://github.com/arduino/arduino-cli/issues/2016 + // assert that the task progress is increasing and doesn't contain multiple 100% values + results := analyzer.Results[""] + require.True(t, results[len(results)-1].Completed) + require.IsNonDecreasing(t, func() []float32{ + res := make([]float32, len(results)) + for i := 0; i < len(results); i++ { + res[i] = results[i].Percent + } + return res + }()) } func TestDaemonCompileAfterFailedLibInstall(t *testing.T) { diff --git a/internal/integrationtest/daemon/task_progress_test.go b/internal/integrationtest/daemon/task_progress_test.go new file mode 100644 index 00000000000..bd9cf1038de --- /dev/null +++ b/internal/integrationtest/daemon/task_progress_test.go @@ -0,0 +1,31 @@ +package daemon_test + +import ( + "testing" + + "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" +) + +// TaskProgressAnalyzer analyzes TaskProgress messages for consistency +type TaskProgressAnalyzer struct { + t *testing.T + Results map[string][]*commands.TaskProgress +} + +// NewTaskProgressAnalyzer creates a new TaskProgressAnalyzer +func NewTaskProgressAnalyzer(t *testing.T) *TaskProgressAnalyzer { + return &TaskProgressAnalyzer{ + t: t, + Results: map[string][]*commands.TaskProgress{}, + } +} + +// Process the given TaskProgress message. +func (a *TaskProgressAnalyzer) Process(progress *commands.TaskProgress) { + if progress == nil { + return + } + + taskName := progress.GetName() + a.Results[taskName] = append(a.Results[taskName], progress) +} From d1a7a6a292ca008a640d851d1f76f69b7aa8c927 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Thu, 3 Aug 2023 15:08:18 +0200 Subject: [PATCH 5/5] apply CR suggestions --- internal/integrationtest/daemon/daemon_test.go | 9 ++------- .../integrationtest/daemon/task_progress_test.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/internal/integrationtest/daemon/daemon_test.go b/internal/integrationtest/daemon/daemon_test.go index e87d7d47f37..cbb227293c4 100644 --- a/internal/integrationtest/daemon/daemon_test.go +++ b/internal/integrationtest/daemon/daemon_test.go @@ -23,6 +23,7 @@ import ( "time" "github.com/arduino/arduino-cli/arduino" + f "github.com/arduino/arduino-cli/internal/algorithms" "github.com/arduino/arduino-cli/internal/integrationtest" "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/go-paths-helper" @@ -187,13 +188,7 @@ func TestDaemonCompileOptions(t *testing.T) { // assert that the task progress is increasing and doesn't contain multiple 100% values results := analyzer.Results[""] require.True(t, results[len(results)-1].Completed) - require.IsNonDecreasing(t, func() []float32{ - res := make([]float32, len(results)) - for i := 0; i < len(results); i++ { - res[i] = results[i].Percent - } - return res - }()) + require.IsNonDecreasing(t, f.Map(results, (*commands.TaskProgress).GetPercent)) } func TestDaemonCompileAfterFailedLibInstall(t *testing.T) { diff --git a/internal/integrationtest/daemon/task_progress_test.go b/internal/integrationtest/daemon/task_progress_test.go index bd9cf1038de..c106a2c6588 100644 --- a/internal/integrationtest/daemon/task_progress_test.go +++ b/internal/integrationtest/daemon/task_progress_test.go @@ -1,3 +1,18 @@ +// This file is part of arduino-cli. +// +// Copyright 2022 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + package daemon_test import (