Delete old /test help messages after a successful invocation

pkg/pjutil/help.go

@@ -30,10 +30,15 @@ var (
 	RetestWithTargetRe  = regexp.MustCompile(`(?m)^/retest[ \t]+\S+`)
 	TestWithAnyTargetRe = regexp.MustCompile(`(?m)^/test[ \t]+\S+`)
 
+	anyTestRe = regexp.MustCompile(`(?m)^/(?:re)?test\b`)
+
 	TestWithoutTargetNote     = "The `/test` command needs one or more targets.\n"
 	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 +65,20 @@ func ShouldRespondWithHelp(body string, toRunOrSkip int) (bool, string) {
 	}
 }
 
+func ShouldRespondByPruningHelp(body string) bool {
+	return anyTestRe.MatchString(body)
+}
+
+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 +98,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,175 @@
+/*
+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
+		userComment   string
+		matchingTests int
+		mayNeedHelp   bool
+		shouldRespond bool
+		note          string
+		shouldPrune   bool
+	}{
+		{
+			name:        "empty comment is neither a failed nor a successful test trigger",
+			userComment: "",
+			mayNeedHelp: false,
+			shouldPrune: false,
+		},
+		{
+			name:        "random comment is neither a failed nor a successful test trigger",
+			userComment: "This is a comment about testing.",
+			mayNeedHelp: false,
+			shouldPrune: false,
+		},
+		{
+			name:          "request for existing test is a successful test trigger",
+			userComment:   "/test e2e",
+			matchingTests: 1,
+			mayNeedHelp:   true,
+			shouldRespond: false,
+			shouldPrune:   true,
+		},
+		{
+			name:          "request for non-existent test is an unsuccessful test trigger",
+			userComment:   "/test f2f",
+			matchingTests: 0,
+			mayNeedHelp:   true,
+			shouldRespond: true,
+			note:          TargetNotFoundNote,
+			shouldPrune:   false,
+		},
+		{
+			name:          "test all when tests exist is a successful test trigger",
+			userComment:   "/test all",
+			matchingTests: 1,
+			mayNeedHelp:   true,
+			shouldRespond: false,
+			shouldPrune:   true,
+		},
+		{
+			name:          "test all when no tests exist is an unsuccessful test trigger",
+			userComment:   "/test all",
+			matchingTests: 0,
+			mayNeedHelp:   true,
+			shouldRespond: true,
+			note:          ThereAreNoTestAllJobsNote,
+			shouldPrune:   false,
+		},
+		{
+			name:          "retest is a successful test trigger",
+			userComment:   "/retest",
+			matchingTests: 1,
+			mayNeedHelp:   false,
+			shouldPrune:   true,
+		},
+		{
+			name:          "retest is a successful test trigger, even when no tests exist",
+			userComment:   "/retest",
+			matchingTests: 0,
+			mayNeedHelp:   false,
+			shouldPrune:   true,
+		},
+		{
+			name:          "empty /test is invalid",
+			userComment:   "/test",
+			mayNeedHelp:   true,
+			shouldRespond: true,
+			note:          TestWithoutTargetNote,
+			shouldPrune:   false,
+		},
+		{
+			name:          "retest with target is invalid",
+			userComment:   "/retest e2e",
+			mayNeedHelp:   true,
+			shouldRespond: true,
+			note:          RetestWithTargetNote,
+			shouldPrune:   false,
+		},
+		{
+			name:          "/test ? is a request for help, not a trigger",
+			userComment:   "/test ?",
+			mayNeedHelp:   true,
+			shouldRespond: true,
+			note:          "",
+			shouldPrune:   false,
+		},
+	}
+
+	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) {
+			// None of the user comments should look like our help comment.
+			if IsHelpComment(tc.userComment) {
+				t.Errorf("Expected IsHelpComment(%q) to return false, got true", tc.userComment)
+			}
+
+			// MayNeedHelpComment is true if the comment uses `/test` or
+			// `/retest` at all (whether valid or invalid).
+			mayNeedHelp := MayNeedHelpComment(tc.userComment)
+			if mayNeedHelp != tc.mayNeedHelp {
+				t.Errorf("Expected MayNeedHelpComment(%q) to return %v, got %v", tc.userComment, tc.mayNeedHelp, mayNeedHelp)
+			}
+
+			// ShouldRespondWithHelp will check if userComment contains a
+			// `/test` or `/retest` invocation that is invalid given the
+			// number of matching tests.
+			shouldRespond, note := ShouldRespondWithHelp(tc.userComment, tc.matchingTests)
+			if shouldRespond != tc.shouldRespond {
+				t.Errorf("Expected ShouldRespondWithHelp(%q, %d) to return %v, got %v", tc.userComment, tc.matchingTests, tc.shouldRespond, shouldRespond)
+			}
+			if note != tc.note {
+				t.Errorf("Expected ShouldRespondWithHelp(%q) to return note %q, got %q", tc.userComment, tc.note, note)
+			}
+
+			// If we should respond with a help comment, then HelpMessage
+			// should return the expected message, and IsHelpComment should
+			// recognize it.
+			if shouldRespond {
+				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)
+				}
+			}
+
+			// If we shouldn't respond with a help comment, then we possibly
+			// should respond by deleted old help comments.
+			if !shouldRespond {
+				shouldPrune := ShouldRespondByPruningHelp(tc.userComment)
+				if shouldPrune != tc.shouldPrune {
+					t.Errorf("Expected ShouldRespondByPruningHelp(%q) to return %v, got %v", tc.userComment, tc.shouldPrune, shouldPrune)
+				}
+			}
+		})
+	}
+}

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,15 @@ func handleGenericComment(c Client, trigger plugins.Trigger, gc github.GenericCo
 			}
 		}
 	}
+
+	if pjutil.ShouldRespondByPruningHelp(textToCheck) {
+		// The run was triggered by a "/test" or "/retest" (and not by
+		// "/ok-to-test" for example), so prune any previous help message.
+		cp.PruneComments(func(comment github.IssueComment) bool {
+			return pjutil.IsHelpComment(comment.Body)
+		})
+	}
+
 	return RunRequestedWithLabels(c, pr, baseSHA, toTest, gc.GUID, additionalLabels)
 }