Delete old /test help messages after a successful invocation

Author: danwinship

pkg/pjutil/help.go

@@ -34,6 +34,9 @@ var (
 	RetestWithTargetNote      = "The `/retest` command does not accept any targets.\n"
 	TargetNotFoundNote        = "The specified target(s) for `/test` were not found.\n"
 	ThereAreNoTestAllJobsNote = "No jobs can be run with `/test all`.\n"
+
+	availableRequiredTestsNote = "The following commands are available to trigger required jobs:"
+	availableOptionalTestsNote = "The following commands are available to trigger optional jobs:"
 )
 
 func MayNeedHelpComment(body string) bool {
@@ -60,6 +63,16 @@ func ShouldRespondWithHelp(body string, toRunOrSkip int) (bool, string) {
 	}
 }
 
+func IsHelpComment(body string) bool {
+	return strings.Contains(body, TestWithoutTargetNote) ||
+		strings.Contains(body, RetestWithTargetNote) ||
+		strings.Contains(body, TargetNotFoundNote) ||
+		strings.Contains(body, ThereAreNoTestAllJobsNote) ||
+		// The response to "/test ?" has no header, so we have to search for these as well.
+		strings.Contains(body, availableRequiredTestsNote) ||
+		strings.Contains(body, availableOptionalTestsNote)
+}
+
 // HelpMessage returns a user friendly help message with the
 //
 //	available /test commands that can be triggered
@@ -79,10 +92,10 @@ func HelpMessage(org, repo, branch, note string, testAllNames, optionalTestComma
 
 	resp = note
 	if requiredTestCommands.Len() > 0 {
-		resp += fmt.Sprintf("The following commands are available to trigger required jobs:%s\n\n", listBuilder(requiredTestCommands))
+		resp += availableRequiredTestsNote + listBuilder(requiredTestCommands) + "\n\n"
 	}
 	if optionalTestCommands.Len() > 0 {
-		resp += fmt.Sprintf("The following commands are available to trigger optional jobs:%s\n\n", listBuilder(optionalTestCommands))
+		resp += availableOptionalTestsNote + listBuilder(optionalTestCommands) + "\n\n"
 	}
 
 	var testAllNote string

pkg/pjutil/help_test.go

@@ -0,0 +1,131 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package pjutil
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+func TestHelp(t *testing.T) {
+	tests := []struct {
+		name        string
+
+		// If the user writes userComment (which does not match IsHelpComment())
+		userComment string
+
+		// then MayNeedHelpComment will return mayNeedHelp...
+		mayNeedHelp bool
+
+		// and if true, ShouldRespondWithHelp(body, 1) will return (shouldRespond, note)
+		shouldRespond bool
+		note          string
+
+		// and if true, note+helpMessageText should match the output of
+		// HelpMessage() and should match IsHelpComment()
+	}{
+		{
+			name: "empty comment",
+			userComment: "",
+			mayNeedHelp: false,
+		},
+		{
+			name: "random comment",
+			userComment: "This is a comment about testing.",
+			mayNeedHelp: false,
+		},
+		{
+			name: "valid test request",
+			userComment: "/test e2e",
+			mayNeedHelp: true,
+			shouldRespond: false,
+		},
+		{
+			name: "valid test all",
+			userComment: "/test all",
+			mayNeedHelp: true,
+			shouldRespond: false,
+		},
+		{
+			name: "valid retest",
+			userComment: "/retest",
+			mayNeedHelp: false,
+		},
+		{
+			name: "empty test",
+			userComment: "/test",
+			mayNeedHelp: true,
+			shouldRespond: true,
+			note: TestWithoutTargetNote,
+		},
+		{
+			name: "retest with target",
+			userComment: "/retest e2e",
+			mayNeedHelp: true,
+			shouldRespond: true,
+			note: RetestWithTargetNote,
+		},
+		{
+			name: "/test ?",
+			userComment: "/test ?",
+			mayNeedHelp: true,
+			shouldRespond: true,
+			note: "",
+		},
+	}
+
+	required := sets.New("/test e2e", "/test e2e-serial", "/test unit")
+	optional := sets.New("/test lint", "/test e2e-conformance-commodore64")
+	all := required.Union(optional)
+	helpBody := HelpMessage("", "", "", "", all, optional, required)
+	
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if IsHelpComment(tc.userComment) {
+				t.Errorf("Expected IsHelpComment(%q) to return false, got true", tc.userComment)
+			}
+			mayNeedHelp := MayNeedHelpComment(tc.userComment)
+			if mayNeedHelp != tc.mayNeedHelp {
+				t.Errorf("Expected MayNeedHelpComment(%q) to return %v, got %v", tc.userComment, tc.mayNeedHelp, mayNeedHelp)
+			}
+			if !tc.mayNeedHelp {
+				return
+			}
+			shouldRespond, note := ShouldRespondWithHelp(tc.userComment, 1)
+			if shouldRespond != tc.shouldRespond {
+				t.Errorf("Expected ShouldRespondWithHelp(%q) to return %v, got %v", tc.userComment, tc.shouldRespond, shouldRespond)
+			}
+			if !tc.shouldRespond {
+				return
+			}
+			if note != tc.note {
+				t.Errorf("Expected ShouldRespondWithHelp(%q) to return note %q, got %q", tc.userComment, tc.note, note)
+			}
+
+			expectHelpMessage := tc.note + helpBody
+			helpMessage := HelpMessage("", "", "", note, all, optional, required)
+			if helpMessage != expectHelpMessage {
+				t.Errorf("Expected HelpMessage() to return %q, got %q", expectHelpMessage, helpMessage)
+			}
+			if !IsHelpComment(helpMessage) {
+				t.Errorf("Expected IsHelpComment(%q) to return true, got false", helpMessage)
+			}
+		})
+	}
+}
+

pkg/plugins/trigger/generic-comment.go

@@ -31,7 +31,11 @@ import (
 	"sigs.k8s.io/prow/pkg/plugins"
 )
 
-func handleGenericComment(c Client, trigger plugins.Trigger, gc github.GenericCommentEvent) error {
+type commentPruner interface {
+	PruneComments(shouldPrune func(github.IssueComment) bool)
+}
+
+func handleGenericComment(c Client, cp commentPruner, trigger plugins.Trigger, gc github.GenericCommentEvent) error {
 	org := gc.Repo.Owner.Login
 	repo := gc.Repo.Name
 	number := gc.Number
@@ -174,6 +178,13 @@ func handleGenericComment(c Client, trigger plugins.Trigger, gc github.GenericCo
 			}
 		}
 	}
+
+	// The user has successfully requested a test run, so delete any previous /test
+	// help message.
+	cp.PruneComments(func(comment github.IssueComment) bool {
+		return pjutil.IsHelpComment(comment.Body)
+	})
+
 	return RunRequestedWithLabels(c, pr, baseSHA, toTest, gc.GUID, additionalLabels)
 }
 

pkg/plugins/trigger/generic-comment_test.go

@@ -47,6 +47,10 @@ func issueLabels(labels ...string) []string {
 
 const shouldNotAddComment = "<none>"
 
+type fakeCommentPruner struct{}
+
+func (f *fakeCommentPruner) PruneComments(shouldPrune func(github.IssueComment) bool) {}
+
 type testcase struct {
 	name string
 
@@ -1485,15 +1489,17 @@ func TestHandleGenericComment(t *testing.T) {
 			}
 			trigger.SetDefaults()
 
+			cp := &fakeCommentPruner{}
+
 			log.Printf("running case %s", tc.name)
 			// In some cases handleGenericComment can be called twice for the same event.
 			// For instance on Issue/PR creation and modification.
 			// Let's call it twice to ensure idempotency.
-			if err := handleGenericComment(c, trigger, event); err != nil {
+			if err := handleGenericComment(c, cp, trigger, event); err != nil {
 				t.Fatalf("%s: didn't expect error: %s", tc.name, err)
 			}
 			validate(t, fakeProwJobClient.Fake.Actions(), g, tc)
-			if err := handleGenericComment(c, trigger, event); err != nil {
+			if err := handleGenericComment(c, cp, trigger, event); err != nil {
 				t.Fatalf("%s: didn't expect error: %s", tc.name, err)
 			}
 			validate(t, fakeProwJobClient.Fake.Actions(), g, tc)

pkg/plugins/trigger/trigger.go

@@ -210,7 +210,11 @@ func handlePullRequest(pc plugins.Agent, pr github.PullRequestEvent) error {
 }
 
 func handleGenericCommentEvent(pc plugins.Agent, gc github.GenericCommentEvent) error {
-	return handleGenericComment(getClient(pc), pc.PluginConfig.TriggerFor(gc.Repo.Owner.Login, gc.Repo.Name), gc)
+	cp, err := pc.CommentPruner()
+	if err != nil {
+		return err
+	}
+	return handleGenericComment(getClient(pc), cp, pc.PluginConfig.TriggerFor(gc.Repo.Owner.Login, gc.Repo.Name), gc)
 }
 
 func handlePush(pc plugins.Agent, pe github.PushEvent) error {