Make actions plan preview handle timeout option (#4648)

* Format and make tests pass in tool/actions-plan-preview Signed-off-by: mi11km <mi11km.program@gmail.com> * Use --piped-handle-timeout to make the timeout arg activate Signed-off-by: mi11km <mi11km.program@gmail.com> * Add tests to model/notificationevent (#4631) * Add tests to model/notificationevent Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com> * Add Parallel() Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com> --------- Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com> Signed-off-by: mi11km <mi11km.program@gmail.com> * add piped-handle-timeout(input args) Signed-off-by: mi11km <mi11km.program@gmail.com> --------- Signed-off-by: mi11km <mi11km.program@gmail.com> Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com> Co-authored-by: Kenta Kozuka <kenta.kozuka@gmail.com> Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com>
pipe-cd · Nov 13, 2023 · 7722661 · 7722661
1 parent 442e597
commit 7722661
Show file tree

Hide file tree

Showing 8 changed files with 178 additions and 20 deletions.
diff --git a/tool/actions-plan-preview/main.go b/tool/actions-plan-preview/main.go
@@ -36,11 +36,12 @@ const (
 	commentEventName     = "issue_comment"
 	pushEventName        = "push"
 
-	argAddress = "address"
-	argAPIKey  = "api-key"
-	argToken   = "token"
-	argTimeout = "timeout"
-	argPRNum   = "pull-request-number"
+	argAddress            = "address"
+	argAPIKey             = "api-key"
+	argToken              = "token"
+	argTimeout            = "timeout"
+	argPipedHandleTimeout = "piped-handle-timeout"
+	argPRNum              = "pull-request-number"
 )
 
 func main() {
@@ -121,6 +122,7 @@ func main() {
 		args.Address,
 		args.APIKey,
 		args.Timeout,
+		args.PipedHandleTimeout,
 	)
 	if err != nil {
 		log.Fatal(err)
@@ -166,11 +168,12 @@ func main() {
 }
 
 type arguments struct {
-	Address string
-	APIKey  string
-	Token   string
-	Timeout time.Duration
-	PRNum   int
+	Address            string
+	APIKey             string
+	Token              string
+	Timeout            time.Duration
+	PipedHandleTimeout time.Duration
+	PRNum              int
 }
 
 func parseArgs(args []string) (arguments, error) {
@@ -194,6 +197,12 @@ func parseArgs(args []string) (arguments, error) {
 				return arguments{}, err
 			}
 			out.Timeout = d
+		case argPipedHandleTimeout:
+			d, err := time.ParseDuration(ps[1])
+			if err != nil {
+				return arguments{}, err
+			}
+			out.PipedHandleTimeout = d
 		case argPRNum:
 			if ps[1] == "" {
 				continue
@@ -221,6 +230,9 @@ func parseArgs(args []string) (arguments, error) {
 	if out.Timeout == 0 {
 		out.Timeout = defaultTimeout
 	}
+	if out.PipedHandleTimeout == 0 {
+		out.PipedHandleTimeout = defaultTimeout
+	}
 
 	return out, nil
 }

diff --git a/tool/actions-plan-preview/main_test.go b/tool/actions-plan-preview/main_test.go
@@ -16,8 +16,11 @@ package main
 
 import (
 	"embed"
+	"fmt"
 	"testing"
+	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -33,3 +36,134 @@ func readTestdataFile(t *testing.T, name string) []byte {
 func boolPointer(b bool) *bool {
 	return &b
 }
+
+func Test_parseArgs(t *testing.T) {
+	type args struct {
+		args []string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    arguments
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name: "minimum required args with no error",
+			args: args{
+				args: []string{
+					"address=localhost:8080",
+					"api-key=xxxxxxxxxxxxxx",
+					"token=xxxxxxxxxxxxxxxx",
+				},
+			},
+			want: arguments{
+				Address:            "localhost:8080",
+				APIKey:             "xxxxxxxxxxxxxx",
+				Token:              "xxxxxxxxxxxxxxxx",
+				Timeout:            defaultTimeout,
+				PipedHandleTimeout: defaultTimeout,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "minimum required args and specified timeout arg with no error",
+			args: args{
+				args: []string{
+					"address=localhost:8080",
+					"api-key=xxxxxxxxxxxxxx",
+					"token=xxxxxxxxxxxxxxxx",
+					"timeout=10m",
+				},
+			},
+			want: arguments{
+				Address:            "localhost:8080",
+				APIKey:             "xxxxxxxxxxxxxx",
+				Token:              "xxxxxxxxxxxxxxxx",
+				Timeout:            10 * time.Minute,
+				PipedHandleTimeout: defaultTimeout,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "minimum required args and specified piped-handle-timeout arg with no error",
+			args: args{
+				args: []string{
+					"address=localhost:8080",
+					"api-key=xxxxxxxxxxxxxx",
+					"token=xxxxxxxxxxxxxxxx",
+					"piped-handle-timeout=10m",
+				},
+			},
+			want: arguments{
+				Address:            "localhost:8080",
+				APIKey:             "xxxxxxxxxxxxxx",
+				Token:              "xxxxxxxxxxxxxxxx",
+				Timeout:            defaultTimeout,
+				PipedHandleTimeout: 10 * time.Minute,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "minimum required args and specified timeout and piped-handle-timeout arg with no error",
+			args: args{
+				args: []string{
+					"address=localhost:8080",
+					"api-key=xxxxxxxxxxxxxx",
+					"token=xxxxxxxxxxxxxxxx",
+					"timeout=12m",
+					"piped-handle-timeout=15m",
+				},
+			},
+			want: arguments{
+				Address:            "localhost:8080",
+				APIKey:             "xxxxxxxxxxxxxx",
+				Token:              "xxxxxxxxxxxxxxxx",
+				Timeout:            12 * time.Minute,
+				PipedHandleTimeout: 15 * time.Minute,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "missing required args (address) returns error",
+			args: args{
+				args: []string{
+					"api-key=xxxxxxxxxxxxxx",
+					"token=xxxxxxxxxxxxxxxx",
+				},
+			},
+			want:    arguments{},
+			wantErr: assert.Error,
+		},
+		{
+			name: "missing required args (api-key) returns error",
+			args: args{
+				args: []string{
+					"address=localhost:8080",
+					"token=xxxxxxxxxxxxxxxx",
+				},
+			},
+			want:    arguments{},
+			wantErr: assert.Error,
+		},
+		{
+			name: "missing required args (token) returns error",
+			args: args{
+				args: []string{
+					"address=localhost:8080",
+					"api-key=xxxxxxxxxxxxxx",
+				},
+			},
+			want:    arguments{},
+			wantErr: assert.Error,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := parseArgs(tt.args.args)
+			if tt.wantErr(t, err, fmt.Sprintf("parseArgs(%v)", tt.args.args)) {
+				return
+			}
+			assert.Equalf(t, tt.want, got, "parseArgs(%v)", tt.args.args)
+		})
+	}
+}
diff --git a/tool/actions-plan-preview/planpreview.go b/tool/actions-plan-preview/planpreview.go
@@ -84,6 +84,7 @@ func retrievePlanPreview(
 	address,
 	apiKey string,
 	timeout time.Duration,
+	pipedHandleTimeout time.Duration,
 ) (*PlanPreviewResult, error) {
 
 	dir, err := os.MkdirTemp("", "")
@@ -101,6 +102,7 @@ func retrievePlanPreview(
 		"--address", address,
 		"--api-key", apiKey,
 		"--timeout", timeout.String(),
+		"--piped-handle-timeout", pipedHandleTimeout.String(),
 		"--out", outPath,
 	}
 	cmd := exec.CommandContext(ctx, "pipectl", args...)

diff --git a/tool/actions-plan-preview/testdata/comment-has-failed-app.txt b/tool/actions-plan-preview/testdata/comment-has-failed-app.txt
@@ -3,7 +3,9 @@
 
 Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations.
 
-## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
+## Plans
+
+### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
 Sync strategy: PIPELINE
 Summary: plan-summary-1
 
@@ -17,12 +19,11 @@ plan-details-1
 </p>
 </details>
 
----
 
 ## NOTE
 
 **An error occurred while building plan-preview for the following applications**
 
-## app: [app-name-2](app-url-2), env: env-2, kind: app-kind-2
+### app: [app-name-2](app-url-2), env: env-2, kind: app-kind-2
 Reason: reason-2
 
diff --git a/tool/actions-plan-preview/testdata/comment-has-failed-piped.txt b/tool/actions-plan-preview/testdata/comment-has-failed-piped.txt
@@ -3,7 +3,9 @@
 
 Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations.
 
-## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
+## Plans
+
+### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
 Sync strategy: PIPELINE
 Summary: plan-summary-1
 
@@ -17,12 +19,11 @@ plan-details-1
 </p>
 </details>
 
----
 
 ## NOTE
 
 **An error occurred while building plan-preview for applications of the following Pipeds**
 
-## piped: [piped-id-1](piped-url-1)
+### piped: [piped-id-1](piped-url-1)
 Reason: piped-reason-1
 
diff --git a/tool/actions-plan-preview/testdata/comment-has-no-diff-apps.txt b/tool/actions-plan-preview/testdata/comment-has-no-diff-apps.txt
@@ -3,7 +3,9 @@
 
 Ran plan-preview against head commit abc of this pull request. PipeCD detected `3` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations.
 
-## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
+## Plans
+
+### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
 Sync strategy: PIPELINE
 Summary: plan-summary-1
 
@@ -17,7 +19,7 @@ plan-details-1
 </p>
 </details>
 
-## No resource changes were detected but the following apps will also be triggered
+### No resource changes were detected but the following apps will also be triggered
 
 ###### `PIPELINE`
 

diff --git a/tool/actions-plan-preview/testdata/comment-no-env.txt b/tool/actions-plan-preview/testdata/comment-no-env.txt
@@ -3,7 +3,9 @@
 
 Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations.
 
-## app: [app-name-1](app-url-1), kind: app-kind-1
+## Plans
+
+### app: [app-name-1](app-url-1), kind: app-kind-1
 Sync strategy: PIPELINE
 Summary: plan-summary-1
 
@@ -16,3 +18,4 @@ plan-details-1
 ```
 </p>
 </details>
+
diff --git a/tool/actions-plan-preview/testdata/comment-only-changed-app.txt b/tool/actions-plan-preview/testdata/comment-only-changed-app.txt
@@ -3,7 +3,9 @@
 
 Ran plan-preview against head commit abc of this pull request. PipeCD detected `1` updated applications and here are their plan results. Once this pull request got merged their deployments will be triggered to run as these estimations.
 
-## app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
+## Plans
+
+### app: [app-name-1](app-url-1), env: env-1, kind: app-kind-1
 Sync strategy: PIPELINE
 Summary: plan-summary-1
 
@@ -16,3 +18,4 @@ plan-details-1
 ```
 </p>
 </details>
+