From e719b55f3f331edd478476891f7a8ace7e55b158 Mon Sep 17 00:00:00 2001 From: Maciej Kwiek Date: Fri, 13 Mar 2026 14:47:27 +0100 Subject: [PATCH] Improve feedback on dependency processing This change improves ariane feedback following a trigger phrase which dependencies haven't been met. If the dependency is still running - Ariane reacts to comment with a "+1" emoji to indicate that everything is fine and the trigger will be run after dependencies are complete. In verbose mode the comment is changed to be less negative. If the dependency failed - Ariane reacts to comment with a "confused" emoji which indicates that tests won't be triggered. In verbose mode the comment stays negative as it was. Signed-off-by: Maciej Kwiek --- internal/handlers/issue_comment.go | 19 ++- internal/handlers/issue_comment_test.go | 194 ++++++++++++++++++++++-- internal/handlers/workflow_processor.go | 13 +- 3 files changed, 208 insertions(+), 18 deletions(-) diff --git a/internal/handlers/issue_comment.go b/internal/handlers/issue_comment.go index 11fa23e..b1befa6 100644 --- a/internal/handlers/issue_comment.go +++ b/internal/handlers/issue_comment.go @@ -161,8 +161,23 @@ func (h *PRCommentHandler) Handle(ctx context.Context, eventType, deliveryID str err = processor.processWorkflowsForTrigger(ctx, submatch, prNumber, contextRef, headSHA, baseSHA, workflowsToTrigger, dependsOn, commenter) if err != nil { - comment := fmt.Sprintf("Failed to process workflows for trigger: %v", err) - logger.Error().Err(err).Msg(comment) + var comment string + err, ok := err.(TriggerSkippedDependencyInProgressError) + if ok { + comment = err.Error() + logger.Debug().Err(err).Msg(comment) + commentErr := commenter.reactToComment(ctx, commentID, "+1") + if commentErr == nil { + logger.Error().Err(err).Msg("Failed to react to comment with thumbs up emoji") + } + } else { + comment = fmt.Sprintf("Failed to process workflows for trigger: %v", err) + logger.Error().Err(err).Msg(comment) + commentErr := commenter.reactToComment(ctx, commentID, "confused") + if commentErr == nil { + logger.Error().Err(err).Msg("Failed to react to comment with confused emoji") + } + } if arianeConfig.GetVerbose() { _ = commenter.commentOnPullRequest(ctx, prNumber, comment) } diff --git a/internal/handlers/issue_comment_test.go b/internal/handlers/issue_comment_test.go index 98f475e..b8d163a 100644 --- a/internal/handlers/issue_comment_test.go +++ b/internal/handlers/issue_comment_test.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "net/http/httptest" "net/url" @@ -971,7 +972,7 @@ func TestHandle_FeedbackDisabled(t *testing.T) { mockCtrl := gomock.NewController(t) mockClientCreator := NewMockClientCreator(mockCtrl) - server := setMockServerWithFeedbackConfig(false, false) + server := setMockServerWithFeedbackConfig(false, false, &[]string{}, false, false) defer server.Close() client := github.NewClient(nil) @@ -1027,7 +1028,7 @@ func TestHandle_VerboseEnabled(t *testing.T) { mockCtrl := gomock.NewController(t) mockClientCreator := NewMockClientCreator(mockCtrl) - server := setMockServerWithFeedbackConfig(true, false) + server := setMockServerWithFeedbackConfig(true, false, &[]string{}, false, false) defer server.Close() client := github.NewClient(nil) @@ -1082,7 +1083,8 @@ func TestHandle_WorkflowsReportEnabled(t *testing.T) { mockCtrl := gomock.NewController(t) mockClientCreator := NewMockClientCreator(mockCtrl) - server := setMockServerWithFeedbackConfig(true, true) + reactions := make([]string, 0, 10) + server := setMockServerWithFeedbackConfig(true, true, &reactions, false, false) defer server.Close() client := github.NewClient(nil) @@ -1126,6 +1128,8 @@ func TestHandle_WorkflowsReportEnabled(t *testing.T) { err := handler.Handle(ctx, "issue_comment", "1", payload) assert.NoError(t, err) + + assert.EqualValues(t, []string{"eyes", "rocket"}, reactions) } func TestHandle_WorkflowsReportDisabled(t *testing.T) { @@ -1137,7 +1141,7 @@ func TestHandle_WorkflowsReportDisabled(t *testing.T) { mockCtrl := gomock.NewController(t) mockClientCreator := NewMockClientCreator(mockCtrl) - server := setMockServerWithFeedbackConfig(true, false) + server := setMockServerWithFeedbackConfig(true, false, &[]string{}, false, false) defer server.Close() client := github.NewClient(nil) @@ -1183,7 +1187,111 @@ func TestHandle_WorkflowsReportDisabled(t *testing.T) { assert.NoError(t, err) } -func setMockServerWithFeedbackConfig(verbose bool, workflowsReport bool) *httptest.Server { +func TestHandle_WorkflowsDependencyRunningReaction(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClientCreator := NewMockClientCreator(mockCtrl) + + reactions := make([]string, 0, 10) + server := setMockServerWithFeedbackConfig(false, false, &reactions, true, true) + defer server.Close() + + client := github.NewClient(nil) + client.BaseURL, _ = url.Parse(server.URL + "/") + mockClientCreator.EXPECT().NewInstallationClient(int64(0)).Return(client, nil) + + handler := &PRCommentHandler{ + ClientCreator: mockClientCreator, + RunDelay: time.Second, + MaxRetryAttempts: config.DefaultMaxRetryAttempts, + } + + payload := []byte(`{ + "issue": { + "pull_request": {}, + "number": 0 + }, + "action": "created", + "comment": { + "id": 1, + "body": "/test", + "user": { + "login": "user" + } + }, + "repository": { + "owner": { + "login": "owner" + }, + "name": "repo" + }, + "installation": { + "id": 0 + } + }`) + + var event github.IssueCommentEvent + _ = json.Unmarshal(payload, &event) + + ctx := context.Background() + + err := handler.Handle(ctx, "issue_comment", "1", payload) + assert.Error(t, err) + assert.EqualValues(t, []string{"eyes", "+1"}, reactions) +} + +func TestHandle_WorkflowsDependencyFailedReaction(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClientCreator := NewMockClientCreator(mockCtrl) + + reactions := make([]string, 0, 10) + server := setMockServerWithFeedbackConfig(false, false, &reactions, true, false) + defer server.Close() + + client := github.NewClient(nil) + client.BaseURL, _ = url.Parse(server.URL + "/") + mockClientCreator.EXPECT().NewInstallationClient(int64(0)).Return(client, nil) + + handler := &PRCommentHandler{ + ClientCreator: mockClientCreator, + RunDelay: time.Second, + MaxRetryAttempts: config.DefaultMaxRetryAttempts, + } + + payload := []byte(`{ + "issue": { + "pull_request": {}, + "number": 0 + }, + "action": "created", + "comment": { + "id": 1, + "body": "/test", + "user": { + "login": "user" + } + }, + "repository": { + "owner": { + "login": "owner" + }, + "name": "repo" + }, + "installation": { + "id": 0 + } + }`) + + var event github.IssueCommentEvent + _ = json.Unmarshal(payload, &event) + + ctx := context.Background() + + err := handler.Handle(ctx, "issue_comment", "1", payload) + assert.Error(t, err) + assert.EqualValues(t, []string{"eyes", "confused"}, reactions) +} + +func setMockServerWithFeedbackConfig(verbose bool, workflowsReport bool, reactions *[]string, dependencyTest bool, dependencyRunning bool) *httptest.Server { mux := http.NewServeMux() // Mock individual PR endpoint @@ -1216,9 +1324,56 @@ func setMockServerWithFeedbackConfig(verbose bool, workflowsReport bool) *httpte _ = json.NewEncoder(w).Encode([]*github.PullRequest{&pr}) }) - // Mock config file endpoint with feedback settings - mux.HandleFunc("/repos/owner/repo/contents/.github/ariane-config.yaml", func(w http.ResponseWriter, r *http.Request) { - configContent := fmt.Sprintf(`feedback: + if dependencyTest { + // Mock config file endpoint with feedback settings + mux.HandleFunc("/repos/owner/repo/contents/.github/ariane-config.yaml", func(w http.ResponseWriter, r *http.Request) { + configContent := ` +triggers: + /test: + workflows: + - foo.yaml + depends-on: + - /dependency + /dependency: + workflows: + - bar.yaml +workflows: + foo.yaml: + paths-regex: ".*" +` + + content := github.RepositoryContent{ + Content: github.Ptr(configContent), + Encoding: github.Ptr(""), + } + _ = json.NewEncoder(w).Encode(&content) + }) + mux.HandleFunc("/repos/owner/repo/actions/workflows/bar.yaml/runs", func(w http.ResponseWriter, r *http.Request) { + var conclusion string + var status string + if dependencyRunning { + status = "in_progress" + conclusion = "" + } else { + status = "completed" + conclusion = "failure" + } + workflowRuns := github.WorkflowRuns{ + WorkflowRuns: []*github.WorkflowRun{ + { + ID: github.Ptr(int64(2)), + Status: github.Ptr(status), + Conclusion: github.Ptr(conclusion), + }, + }, + TotalCount: github.Ptr(1), + } + _ = json.NewEncoder(w).Encode(&workflowRuns) + }) + } else { + // Mock config file endpoint with feedback settings + mux.HandleFunc("/repos/owner/repo/contents/.github/ariane-config.yaml", func(w http.ResponseWriter, r *http.Request) { + configContent := fmt.Sprintf(`feedback: verbose: %t workflows-report: %t triggers: @@ -1230,12 +1385,13 @@ workflows: paths-regex: ".*" `, verbose, workflowsReport) - content := github.RepositoryContent{ - Content: github.Ptr(configContent), - Encoding: github.Ptr(""), - } - _ = json.NewEncoder(w).Encode(&content) - }) + content := github.RepositoryContent{ + Content: github.Ptr(configContent), + Encoding: github.Ptr(""), + } + _ = json.NewEncoder(w).Encode(&content) + }) + } // Mock PR files endpoint mux.HandleFunc("/repos/owner/repo/pulls/0/files", func(w http.ResponseWriter, r *http.Request) { @@ -1277,6 +1433,16 @@ workflows: // Mock reactions endpoint mux.HandleFunc("/repos/owner/repo/issues/comments/1/reactions", func(w http.ResponseWriter, r *http.Request) { + bytes, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "setMockServerWithFeedbackConfig: could not read request body", http.StatusInternalServerError) + return + } + + incomingReaction := github.Reaction{} + _ = json.Unmarshal(bytes, &incomingReaction) + *reactions = append(*reactions, incomingReaction.GetContent()) + reaction := github.Reaction{ ID: github.Ptr(int64(1)), } diff --git a/internal/handlers/workflow_processor.go b/internal/handlers/workflow_processor.go index a22b017..fa64a1c 100644 --- a/internal/handlers/workflow_processor.go +++ b/internal/handlers/workflow_processor.go @@ -213,6 +213,12 @@ func (w *WorkflowProcessor) getPRFiles(ctx context.Context, prNumber int) ([]*gi return files, nil } +type TriggerSkippedDependencyInProgressError string + +func (e TriggerSkippedDependencyInProgressError) Error() string { + return string(e) +} + func (w *WorkflowProcessor) processWorkflowsForTrigger(ctx context.Context, submatch []string, prNumber int, contextRef, headSHA, baseSHA string, workflowsToTrigger, dependsOn []string, commenter *GithubCommenter) error { w.logger.Debug().Msgf("Found trigger phrase: %q", submatch) @@ -227,11 +233,14 @@ func (w *WorkflowProcessor) processWorkflowsForTrigger(ctx context.Context, subm if !canProceed { var comment string if inProgress { - comment = fmt.Sprintf("Skipping trigger: dependency %q is still in progress", dep) + comment = fmt.Sprintf("Skipping trigger: dependency %q is still in progress. Trigger will be retriggered after dependency completes.", dep) + w.logger.Info().Msg(comment) + err := TriggerSkippedDependencyInProgressError(comment) + return err } else { comment = fmt.Sprintf("Skipping trigger: dependency %q has not completed successfully", dep) + w.logger.Info().Msg(comment) } - w.logger.Info().Msg(comment) return errors.New(comment) }