rename errors.FileSync errors.Sync (#5625)

* rename errors.FileSync errors.Sync * add test for AutoTriggerDiff()
GoogleContainerTools · Apr 2, 2021 · 5757b03 · 5757b03
1 parent 2c10abe
commit 5757b03
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 15 deletions.
diff --git a/pkg/skaffold/errors/errors.go b/pkg/skaffold/errors/errors.go
@@ -36,7 +36,7 @@ const (
 	Test        = Phase("Test")
 	Deploy      = Phase("Deploy")
 	StatusCheck = Phase("StatusCheck")
-	FileSync    = Phase("FileSync")
+	Sync        = Phase("Sync")
 	DevInit     = Phase("DevInit")
 	Cleanup     = Phase("Cleanup")
 
@@ -182,7 +182,7 @@ var allErrors = map[Phase][]problem{
 		errCode:    proto.StatusCode_STATUSCHECK_UNKNOWN,
 		suggestion: reportIssueSuggestion,
 	}},
-	FileSync: {{
+	Sync: {{
 		regexp:     re(".*"),
 		errCode:    proto.StatusCode_SYNC_UNKNOWN,
 		suggestion: reportIssueSuggestion,

diff --git a/pkg/skaffold/errors/errors_test.go b/pkg/skaffold/errors/errors_test.go
@@ -195,7 +195,7 @@ func TestShowAIError(t *testing.T) {
 		},
 		{
 			description: "file sync unknown error",
-			phase:       FileSync,
+			phase:       Sync,
 			err:         fmt.Errorf("sync failed: something went wrong"),
 			expected:    "sync failed: something went wrong",
 			expectedAE: &proto.ActionableErr{

diff --git a/pkg/skaffold/event/event.go b/pkg/skaffold/event/event.go
@@ -413,7 +413,7 @@ func FileSyncInProgress(fileCount int, image string) {
 
 // FileSyncFailed notifies that a file sync has failed.
 func FileSyncFailed(fileCount int, image string, err error) {
-	aiErr := sErrors.ActionableErr(sErrors.FileSync, err)
+	aiErr := sErrors.ActionableErr(sErrors.Sync, err)
 	handler.handleFileSyncEvent(&proto.FileSyncEvent{FileCount: int32(fileCount), Image: image, Status: Failed,
 		Err: err.Error(), ErrCode: aiErr.ErrCode, ActionableErr: aiErr})
 }

diff --git a/pkg/skaffold/event/v2/event.go b/pkg/skaffold/event/v2/event.go
@@ -241,16 +241,16 @@ func emptyStatusCheckState() *proto.StatusCheckState {
 	}
 }
 
-func AutoTriggerDiff(name string, val bool) (bool, error) {
-	switch name {
-	case "build":
+func AutoTriggerDiff(phase sErrors.Phase, val bool) (bool, error) {
+	switch phase {
+	case sErrors.Build:
 		return val != handler.getState().BuildState.AutoTrigger, nil
-	case "sync":
+	case sErrors.Sync:
 		return val != handler.getState().FileSyncState.AutoTrigger, nil
-	case "deploy":
+	case sErrors.Deploy:
 		return val != handler.getState().DeployState.AutoTrigger, nil
 	default:
-		return false, fmt.Errorf("unknown phase %v not found in handler state", name)
+		return false, fmt.Errorf("unknown phase %v not found in handler state", phase)
 	}
 }
 

diff --git a/pkg/skaffold/event/v2/event_test.go b/pkg/skaffold/event/v2/event_test.go
@@ -291,6 +291,67 @@ func TestTaskFailed(t *testing.T) {
 	}
 }
 
+func TestAutoTriggerDiff(t *testing.T) {
+	tests := []struct {
+		description  string
+		phase        sErrors.Phase
+		handlerState proto.State
+		val          bool
+		expected     bool
+	}{
+		{
+			description: "build needs update",
+			phase:       sErrors.Build,
+			val:         true,
+			handlerState: proto.State{
+				BuildState: &proto.BuildState{
+					AutoTrigger: false,
+				},
+			},
+			expected: true,
+		},
+		{
+			description: "deploy doesn't need update",
+			phase:       sErrors.Deploy,
+			val:         true,
+			handlerState: proto.State{
+				BuildState: &proto.BuildState{
+					AutoTrigger: false,
+				},
+				DeployState: &proto.DeployState{
+					AutoTrigger: true,
+				},
+			},
+			expected: false,
+		},
+		{
+			description: "sync needs update",
+			phase:       sErrors.Sync,
+			val:         false,
+			handlerState: proto.State{
+				FileSyncState: &proto.FileSyncState{
+					AutoTrigger: true,
+				},
+			},
+			expected: true,
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			// Setup handler state
+			handler.setState(test.handlerState)
+
+			got, err := AutoTriggerDiff(test.phase, test.val)
+			if err != nil {
+				t.Fail()
+			}
+
+			t.CheckDeepEqual(test.expected, got)
+		})
+	}
+}
+
 func TestSaveEventsToFile(t *testing.T) {
 	f, err := ioutil.TempFile("", "")
 	if err != nil {

diff --git a/pkg/skaffold/runner/dev.go b/pkg/skaffold/runner/dev.go
@@ -86,7 +86,7 @@ func (r *SkaffoldRunner) doDev(ctx context.Context, out io.Writer, logger *kuber
 			if err := r.syncer.Sync(ctx, s); err != nil {
 				logrus.Warnln("Skipping deploy due to sync error:", err)
 				fileSyncFailed(fileCount, s.Image, err)
-				event.DevLoopFailedInPhase(r.devIteration, sErrors.FileSync, err)
+				event.DevLoopFailedInPhase(r.devIteration, sErrors.Sync, err)
 				return nil
 			}
 

diff --git a/pkg/skaffold/server/v2/endpoints.go b/pkg/skaffold/server/v2/endpoints.go
@@ -23,6 +23,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
+	sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
 	event "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event/v2"
 	proto "github.com/GoogleContainerTools/skaffold/proto/v2"
 )
@@ -71,18 +72,18 @@ func (s *Server) Execute(ctx context.Context, request *proto.UserIntentRequest)
 }
 
 func (s *Server) AutoBuild(ctx context.Context, request *proto.TriggerRequest) (res *empty.Empty, err error) {
-	return executeAutoTrigger("build", request, event.UpdateStateAutoBuildTrigger, event.ResetStateOnBuild, s.AutoBuildCallback)
+	return executeAutoTrigger(sErrors.Build, request, event.UpdateStateAutoBuildTrigger, event.ResetStateOnBuild, s.AutoBuildCallback)
 }
 
 func (s *Server) AutoDeploy(ctx context.Context, request *proto.TriggerRequest) (res *empty.Empty, err error) {
-	return executeAutoTrigger("deploy", request, event.UpdateStateAutoDeployTrigger, event.ResetStateOnDeploy, s.AutoDeployCallback)
+	return executeAutoTrigger(sErrors.Deploy, request, event.UpdateStateAutoDeployTrigger, event.ResetStateOnDeploy, s.AutoDeployCallback)
 }
 
 func (s *Server) AutoSync(ctx context.Context, request *proto.TriggerRequest) (res *empty.Empty, err error) {
-	return executeAutoTrigger("sync", request, event.UpdateStateAutoSyncTrigger, func() {}, s.AutoSyncCallback)
+	return executeAutoTrigger(sErrors.Sync, request, event.UpdateStateAutoSyncTrigger, func() {}, s.AutoSyncCallback)
 }
 
-func executeAutoTrigger(triggerName string, request *proto.TriggerRequest, updateTriggerStateFunc func(bool), resetPhaseStateFunc func(), serverCallback func(bool)) (res *empty.Empty, err error) {
+func executeAutoTrigger(triggerName sErrors.Phase, request *proto.TriggerRequest, updateTriggerStateFunc func(bool), resetPhaseStateFunc func(), serverCallback func(bool)) (res *empty.Empty, err error) {
 	res = &empty.Empty{}
 
 	trigger := request.GetState().GetEnabled()