Fix double 100% progress on CompileResponse (#2225)

Author: alessio-perugini

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"
@@ -171,6 +172,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 +182,13 @@ 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, f.Map(results, (*commands.TaskProgress).GetPercent))
 }
 
 func TestDaemonCompileAfterFailedLibInstall(t *testing.T) {

internal/integrationtest/daemon/task_progress_test.go

@@ -0,0 +1,46 @@
+// 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 [email protected].
+
+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)
+}

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 {

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)

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