Fix incorrect deployment times (#34)

* Removed unused releaseTime variable

* Introduce time interface so sleeps now current time can be mocked in tests

* Add simple test for single branch deployment

* Fix bug where wrong interval is used during release

Author: peterklijn

ergo.go

@@ -1,6 +1,9 @@
 package ergo
 
-import "context"
+import (
+	"context"
+	"time"
+)
 
 // MessageLevel defines the level of output message.
 type MessageLevel string
@@ -29,6 +32,12 @@ type CLI interface {
 	Input() (string, error)
 }
 
+// Time describes actions around time and waiting.
+type Time interface {
+	Sleep(duration time.Duration)
+	Now() time.Time
+}
+
 // Deploy describes the deploy process.
 type Deploy interface {
 	Do(ctx context.Context, releaseIntervalInput, releaseOffsetInput string, allowForcePush bool) error

mock/cli.go

@@ -1,6 +1,8 @@
 package mock
 
 import (
+	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/beatlabs/ergo"
@@ -13,6 +15,7 @@ type CLI struct {
 	mu                sync.Mutex
 	ConfirmationCalls int
 	PrintTableCalls   []PrintTableVal
+	PrintLines        []string
 }
 
 // PrintTableVal represents the values send to the PrintTable method.
@@ -32,7 +35,13 @@ func (c *CLI) PrintTable(header []string, values [][]string) {
 func (c *CLI) PrintColorizedLine(title, content string, level ergo.MessageLevel) {}
 
 // PrintLine is a mock implementation.
-func (c *CLI) PrintLine(content ...interface{}) {}
+func (c *CLI) PrintLine(content ...interface{}) {
+	words := make([]string, 0, len(content))
+	for _, c := range content {
+		words = append(words, fmt.Sprintf("%v", c))
+	}
+	c.PrintLines = append(c.PrintLines, strings.Join(words, " "))
+}
 
 // Confirmation is a mock implementation.
 func (c *CLI) Confirmation(actionText, cancellationMessage, successMessage string) (bool, error) {

mock/time.go

@@ -0,0 +1,23 @@
+package mock
+
+import "time"
+
+// Time is a mock implementation.
+type Time struct {
+	CurrentTime time.Time
+}
+
+// NewMockedTime creates a new mocked time implementation.
+func NewMockedTime(initialTime time.Time) *Time {
+	return &Time{CurrentTime: initialTime}
+}
+
+// Sleep mocks the sleep action, adding the duration to the time.
+func (t *Time) Sleep(duration time.Duration) {
+	t.CurrentTime = t.CurrentTime.Add(duration)
+}
+
+// Now returns the mocked time.
+func (t *Time) Now() time.Time {
+	return t.CurrentTime
+}

release/deploy.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/beatlabs/ergo"
 	"github.com/beatlabs/ergo/cli"
+	ergoTime "github.com/beatlabs/ergo/time"
 )
 
 // Deploy is responsible to describe the release process.
@@ -20,6 +21,7 @@ type Deploy struct {
 	releaseBodyReplace  string
 	releaseBranches     []string
 	releaseBodyBranches map[string]string
+	time                ergo.Time
 }
 
 // NewDeploy initialize and return a new Deploy object.
@@ -38,6 +40,7 @@ func NewDeploy(
 		releaseBodyReplace:  releaseBodyReplace,
 		releaseBranches:     releaseBranches,
 		releaseBodyBranches: releaseBodyBranches,
+		time:                ergoTime.Time{},
 	}
 }
 
@@ -78,7 +81,7 @@ func (r *Deploy) Do(
 	r.printReleaseTimeBoard(releaseTime, r.releaseBranches, intervalDurations)
 
 	if skipConfirm {
-		return r.deployToAllReleaseBranches(ctx, intervalDurations, releaseTime, release, allowForcePush)
+		return r.deployToAllReleaseBranches(ctx, intervalDurations, release, allowForcePush)
 	}
 
 	confirm, err := r.c.Confirmation("Deployment", "No deployment", "")
@@ -95,35 +98,35 @@ func (r *Deploy) Do(
 
 	untilReleaseTime := time.Until(releaseTime)
 	r.c.PrintLine("Deployment will start in", untilReleaseTime.String())
-	time.Sleep(untilReleaseTime)
+	r.time.Sleep(untilReleaseTime)
 
-	return r.deployToAllReleaseBranches(ctx, intervalDurations, releaseTime, release, allowForcePush)
+	return r.deployToAllReleaseBranches(ctx, intervalDurations, release, allowForcePush)
 }
 
 func (r *Deploy) deployToAllReleaseBranches(
 	ctx context.Context,
 	intervalDurations []time.Duration,
-	releaseTime time.Time,
 	release *ergo.Release,
 	allowForcePush bool,
 ) error {
 	for i, branch := range r.releaseBranches {
-		intervalDuration := intervalDurations[i%len(intervalDurations)]
-		if i != 0 {
-			time.Sleep(intervalDuration)
-			releaseTime = releaseTime.Add(intervalDuration)
-		}
-		r.c.PrintLine("Deploying", time.Now().Format("15:04:05"), branch)
+		r.c.PrintLine("Deploying", r.time.Now().Format("15:04:05"), branch)
 
 		if errRelease := r.host.UpdateBranchFromTag(ctx, release.TagName, branch, allowForcePush); errRelease != nil {
 			return errRelease
 		}
-		r.c.PrintLine(time.Now().Format("15:04:05"), "Triggered Successfully")
+		r.c.PrintLine(r.time.Now().Format("15:04:05"), "Triggered Successfully")
 
 		err := r.updateHostReleaseBody(ctx, r.releaseBodyBranches, branch, r.releaseBodyFind, r.releaseBodyReplace)
 		if err != nil {
 			return err
 		}
+
+		// Don't sleep after the last deployment
+		if i < (len(r.releaseBranches) - 1) {
+			intervalDuration := intervalDurations[i%len(intervalDurations)]
+			r.time.Sleep(intervalDuration)
+		}
 	}
 	return nil
 }

release/deploy_test.go

@@ -354,7 +354,7 @@ func TestNonLinearIntervals(t *testing.T) {
 	}
 }
 
-func TestNonLinearIntervalHandling(t *testing.T) {
+func TestPrintReleaseTimeBoard_NonLinearIntervalHandling(t *testing.T) {
 	ts := func(t time.Time) string {
 		return t.Format("15:04 MST")
 	}
@@ -436,3 +436,109 @@ func TestNonLinearIntervalHandling(t *testing.T) {
 		})
 	}
 }
+
+func TestDeployToAllReleaseBranches_NonLinearIntervalHandling(t *testing.T) {
+	ts := func(t time.Time) string {
+		return t.Format("15:04:05")
+	}
+	releaseTime := time.Date(2022, 8, 4, 13, 37, 0, 0, time.UTC)
+	tests := []struct {
+		name              string
+		branches          []string
+		intervals         string
+		expectedPrints    []string
+		expectedFinalTime time.Time
+	}{
+		{
+			name:      "single branch, single interval",
+			branches:  []string{"branch1"},
+			intervals: "10m",
+			expectedPrints: []string{
+				fmt.Sprintf("Deploying %v branch1", ts(releaseTime)),
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime)),
+			},
+			expectedFinalTime: releaseTime,
+		},
+		{
+			name:      "multiple branches, single interval",
+			branches:  []string{"branch1", "branch2"},
+			intervals: "10m",
+			expectedPrints: []string{
+				fmt.Sprintf("Deploying %v branch1", ts(releaseTime)),
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime)),
+				fmt.Sprintf("Deploying %v branch2", ts(releaseTime.Add(10*time.Minute))),
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(10*time.Minute))),
+			},
+			expectedFinalTime: releaseTime.Add(10 * time.Minute),
+		},
+		{
+			name:      "multiple branches, decreasing interval",
+			branches:  []string{"branch1", "branch2", "branch3", "branch4", "branch5"},
+			intervals: "10m,5m,1m,1m",
+			expectedPrints: []string{
+				fmt.Sprintf("Deploying %v branch1", ts(releaseTime)),
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime)),
+				fmt.Sprintf("Deploying %v branch2", ts(releaseTime.Add(10*time.Minute))), // 10
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(10*time.Minute))),
+				fmt.Sprintf("Deploying %v branch3", ts(releaseTime.Add(15*time.Minute))), // 10 + 5
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(15*time.Minute))),
+				fmt.Sprintf("Deploying %v branch4", ts(releaseTime.Add(16*time.Minute))), // 10 + 5 + 1
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(16*time.Minute))),
+				fmt.Sprintf("Deploying %v branch5", ts(releaseTime.Add(17*time.Minute))), // 10 + 5 + 1 + 1
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(17*time.Minute))),
+			},
+			expectedFinalTime: releaseTime.Add(17 * time.Minute),
+		},
+		{
+			name:      "multiple branches, fewer intervals",
+			branches:  []string{"branch1", "branch2", "branch3", "branch4", "branch5"},
+			intervals: "10m,5m",
+			expectedPrints: []string{
+				fmt.Sprintf("Deploying %v branch1", ts(releaseTime)),
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime)),
+				fmt.Sprintf("Deploying %v branch2", ts(releaseTime.Add(10*time.Minute))), // 10
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(10*time.Minute))),
+				fmt.Sprintf("Deploying %v branch3", ts(releaseTime.Add(15*time.Minute))), // 10 + 5
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(15*time.Minute))),
+				fmt.Sprintf("Deploying %v branch4", ts(releaseTime.Add(25*time.Minute))), // 10 + 5 + 10
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(25*time.Minute))),
+				fmt.Sprintf("Deploying %v branch5", ts(releaseTime.Add(30*time.Minute))), // 10 + 5 + 10 + 5
+				fmt.Sprintf("%v Triggered Successfully", ts(releaseTime.Add(30*time.Minute))),
+			},
+			expectedFinalTime: releaseTime.Add(30 * time.Minute),
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cliMock := &mock.CLI{}
+			timeMock := mock.NewMockedTime(releaseTime)
+			deploy := &Deploy{
+				c:               cliMock,
+				releaseBranches: test.branches,
+				host:            &mock.RepositoryClient{},
+				time:            timeMock,
+			}
+			intervalDurations, _, err := deploy.calculateReleaseTime(test.intervals, "1ms")
+			if err != nil {
+				t.Errorf("NewDeploy().Do() returned error: %v", err)
+			}
+			err = deploy.deployToAllReleaseBranches(context.Background(), intervalDurations, &ergo.Release{}, false)
+			if err != nil {
+				t.Errorf("Deploy.deployToAllReleaseBranches() returned error %v", err)
+			}
+			if len(test.expectedPrints) != len(cliMock.PrintLines) {
+				t.Errorf("Expected %d number of prints to the mock, got %d", len(test.expectedPrints), len(cliMock.PrintLines))
+			} else {
+				for i, expected := range test.expectedPrints {
+					got := cliMock.PrintLines[i]
+					if !reflect.DeepEqual(expected, got) {
+						t.Errorf("Expected print %d: '%v' to equal '%v'", i, expected, got)
+					}
+				}
+			}
+			if test.expectedFinalTime != timeMock.CurrentTime {
+				t.Errorf("Expected final time to equal %v, got %v", test.expectedFinalTime, timeMock.CurrentTime)
+			}
+		})
+	}
+}

time/time.go

@@ -0,0 +1,13 @@
+package time
+
+import "time"
+
+type Time struct{}
+
+func (w Time) Sleep(duration time.Duration) {
+	time.Sleep(duration)
+}
+
+func (w Time) Now() time.Time {
+	return time.Now()
+}