Repository: kubernetes-sigs/prow

Issue:
`require_self_approval: true` should prevent suggesting PR author for approval
[This is re-filed from https://github.com/kubernetes/test-infra/issues/29827]

**What happened**:

TLDR in a repo with implicit_self_approval off, prow will sometimes suggest the author as the approver, it should suggest someone else.


https://kubernetes.slack.com/archives/CDECRSC5U/p1669738130214399

**What you expected to happen**:

suggest non-pr-author for finding an approver

**How to reproduce it (as minimally and precisely as possible)**:

.... you'll need implicit_self_approval disabled, yourself in OWNERS, and then file a PR until this happens.

**Please provide links to example occurrences, if any**:

_lots of sample links in the slack thread linked above_

**Anything else we need to know?**:

/sig testing

Hints from issue comments:
@k8s-triage-robot: The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle stale`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle stale

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@k8s-triage-robot: The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle rotten`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle rotten

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@petr-muller: Approver logic is tricky but filtering out author name from the list of options does not seem *too* scary, so let's try marking it as a good first issue. I hope I'm not throwing someone under the bus :grimacing: @Bharadwajshivam28: Hey @BenTheElder @petr-muller can I look into this issue? also if yes then can you suggest me some steps to get started on this?@petr-muller: @Bharadwajshivam28 Sure, feel free to give it a shot! 

Here are some breadcrumbs. The problem is that Prow sometimes asks `...please ask for approval from AUTHOR...` even when `require_self_approval` is set to `true`, indicating that authors with approval permissions do not get an automated `approved` label, and it is _likely_ desirable for other approver to approve the PR (although the author can still approve it themselves, but if that behavior is desirable then the repo should disable `require_self_approval`.

The message seems to be built here from a `Approvers` struct (partially from its `AssignedCCs` member, partially from `SuggestedCCs`), you can follow its call chain to figure out how the `Approver` struct that ends us passed here is built and prevent the author from being added here if `require_self_approval: true` (unless they are the only approver, I guess).

https://github.com/kubernetes-sigs/prow/blob/24e765300acd79b8c9095558cbd00c344d033aee/pkg/plugins/approve/approvers/owners.go#L685-L691

I retitled this issue to mention the `require_self_approval: true` config instead, which is the current name of that config option. The original `implicit_self_approve` was [renamed with inverted defaults](https://github.com/kubernetes-sigs/prow/commit/3489d134b68210aadaa28d2694588e4fa6783cbb) to current `require_self_approval` in 2019.@Be3751: hi, @BenTheElder, @Bharadwajshivam28, @petr-muller. Can I work on this issue? 
/assign @Be3751@nikhilnishad-goog: > The problem is that Prow sometimes asks ...please ask for approval from AUTHOR... even when require_self_approval is set to true, indicating that authors with approval permissions do not get an automated approved label, and it is likely desirable for other approver to approve the PR

@petr-muller It makes sense to me that prow will ask the approval from the author when there is flag enabled explicitly saying `require_self_approval`. Is it not contradictory that `require_self_approval` is set to true and then at the same time Prow in the comments does not ask the author for approval? 

In other words -> doesn't `require_self_approval=true` actually mean that author does not get implicit approval and needs to  **ATLEAST** explicitly approve it themselves? That is to say, other people's approval are secondary (even optional maybe) but the author's approval is **required**.  In such case, I feel it is counter intuitive to suggest **only** other people for approval and not the author.

What could be better messaging in the comments -> `please ask approval from LIST_OF_APPROVERS_EXCLUDING_AUTHOR and since require_self_approval is true, AUTHOR must also approve it`.@petr-muller: > In other words -> doesn't require_self_approval=true actually mean that author does not get implicit approval and needs to ATLEAST explicitly approve it themselves?

Nope, `require_self_approval=true` does not mean the author always must approve it, although I see how the name may indicate this. Any approver is enough (including the author, but the author needs to approve explicitly if they want to use their privilege). I think the name was selected because it is an inverted form of the original `implicit_self_approve` config.@nikhilnishad-goog: @petr-muller I was working on this but forgot to assign, can you please assign this to me so that there is no confusion.@matthyx: /assign @nikhilnishad-goog 

Solution patch:
diff --git a/pkg/plugins/approve/approve.go b/pkg/plugins/approve/approve.go
index dbcc26783..f71b49243 100644
--- a/pkg/plugins/approve/approve.go
+++ b/pkg/plugins/approve/approve.go
@@ -439,10 +439,8 @@ func handle(log *logrus.Entry, ghc githubClient, repo approvers.Repo, githubConf
 	// Author implicitly approves their own PR if config allows it
 	if opts.HasSelfApproval() {
 		approversHandler.AddAuthorSelfApprover(pr.author, pr.htmlURL+"#", false)
-	} else {
-		// Treat the author as an assignee, and suggest them if possible
-		approversHandler.AddAssignees(pr.author)
 	}
+
 	log.WithField("duration", time.Since(start).String()).Debug("Completed configuring approversHandler in handle")
 
 	start = time.Now()



Test patch:
diff --git a/pkg/plugins/approve/approve_test.go b/pkg/plugins/approve/approve_test.go
index c2da12c38..51be01fa2 100644
--- a/pkg/plugins/approve/approve_test.go
+++ b/pkg/plugins/approve/approve_test.go
@@ -250,7 +250,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -262,7 +262,40 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["cjwagner"]} -->`,
+<!-- META={"approvers":["cblecker"]} -->`,
+		},
+		{
+			name:                "notification for unapproved state but assignee is also a potential approver",
+			hasLabel:            false,
+			files:               []string{"c/c.go", "e/e.go"},
+			comments:            []github.IssueComment{},
+			reviews:             []github.Review{},
+			selfApprove:         false,
+			needsIssue:          false,
+			lgtmActsAsApprove:   false,
+			reviewActsAsApprove: false,
+			githubLinkURL:       &url.URL{Scheme: "https", Host: "github.com"},
+
+			expectDelete:  false,
+			expectToggle:  false,
+			expectComment: true,
+			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
+
+This pull-request has been approved by:
+**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [spxtr](https://github.com/spxtr) and additionally assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+
+The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
+
+<details open>
+Needs approval from an approver in each of these files:
+
+- **[c/OWNERS](https://github.com/org/repo/blob/master/c/OWNERS)**
+- **[e/OWNERS](https://github.com/org/repo/blob/master/e/OWNERS)**
+
+Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
+Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
+</details>
+<!-- META={"approvers":["cblecker","spxtr"]} -->`,
 		},
 		{
 			name:                "no-issue comment",
@@ -751,7 +784,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -763,7 +796,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["cjwagner"]} -->`,
+<!-- META={"approvers":["cblecker"]} -->`,
 		},
 		{
 			name:                "approved review with reviewActsAsApprove enabled",
@@ -820,7 +853,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -832,7 +865,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["cjwagner"]} -->`,
+<!-- META={"approvers":["cblecker"]} -->`,
 		},
 		{
 			name:     "review in request changes state means cancel",
@@ -857,7 +890,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -869,7 +902,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["cjwagner"]} -->`,
+<!-- META={"approvers":["cblecker"]} -->`,
 		},
 		{
 			name:     "dismissed review doesn't cancel prior approval",
@@ -932,7 +965,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -944,7 +977,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["cjwagner"]} -->`,
+<!-- META={"approvers":["cblecker"]} -->`,
 		},
 		{
 			name:     "remove-approve command supersedes earlier approved review",
@@ -969,7 +1002,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -981,7 +1014,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["cjwagner"]} -->`,
+<!-- META={"approvers":["cblecker"]} -->`,
 		},
 		{
 			name:     "approve cancel command supersedes simultaneous approved review",
@@ -1003,7 +1036,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner) and additionally assign [alice](https://github.com/alice) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [alice](https://github.com/alice), [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -1016,7 +1049,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["alice","cjwagner"]} -->`,
+<!-- META={"approvers":["alice","cblecker"]} -->`,
 		},
 		{
 			name:     "remove-approve command supersedes simultaneous approved review",
@@ -1038,7 +1071,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
 
 This pull-request has been approved by:
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner) and additionally assign [alice](https://github.com/alice) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+**Once this PR has been reviewed and has the lgtm label**, please assign [alice](https://github.com/alice), [cblecker](https://github.com/cblecker) for approval. For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
 
 The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
 
@@ -1051,7 +1084,7 @@ Needs approval from an approver in each of these files:
 Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
 Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
 </details>
-<!-- META={"approvers":["alice","cjwagner"]} -->`,
+<!-- META={"approvers":["alice","cblecker"]} -->`,
 		},
 		{
 			name:                "approve command supersedes simultaneous changes requested review",
@@ -1185,39 +1218,6 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 </details>
 <!-- META={"approvers":[]} -->`,
 		},
-		{
-			name:                "don't suggest to /assign already assigned cjwagner",
-			hasLabel:            false,
-			files:               []string{"a/a.go", "c/c.go"},
-			comments:            []github.IssueComment{},
-			reviews:             []github.Review{newTestReview("Alice", "/approve", github.ReviewStateChangesRequested)},
-			selfApprove:         false,
-			needsIssue:          false,
-			lgtmActsAsApprove:   false,
-			reviewActsAsApprove: true,
-			githubLinkURL:       &url.URL{Scheme: "https", Host: "github.com"},
-
-			expectDelete:  false,
-			expectToggle:  false,
-			expectComment: true,
-			expectedComment: `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
-
-This pull-request has been approved by: *<a href="" title="Approved">Alice</a>*
-**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [cjwagner](https://github.com/cjwagner). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
-
-The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
-
-<details open>
-Needs approval from an approver in each of these files:
-
-- ~~[a/OWNERS](https://github.com/org/repo/blob/master/a/OWNERS)~~ [Alice]
-- **[c/OWNERS](https://github.com/org/repo/blob/master/c/OWNERS)**
-
-Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
-Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
-</details>
-<!-- META={"approvers":["cjwagner"]} -->`,
-		},
 	}
 
 	fr := fakeRepo{
@@ -1225,11 +1225,13 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			"a":   layeredsets.NewString("alice"),
 			"a/b": layeredsets.NewString("alice", "bob"),
 			"c":   layeredsets.NewString("cblecker", "cjwagner"),
+			"e":   layeredsets.NewString("spxtr"),
 		},
 		leafApprovers: map[string]sets.Set[string]{
 			"a":   sets.New[string]("alice"),
 			"a/b": sets.New[string]("bob"),
 			"c":   sets.New[string]("cblecker", "cjwagner"),
+			"e":   sets.New[string]("spxtr"),
 		},
 		approverOwners: map[string]string{
 			"a/a.go":                   "a",
@@ -1237,6 +1239,7 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 			"a/b/b.go":                 "a/b",
 			"c/c.go":                   "c",
 			"d/new-folder/new_file.go": "d",
+			"e/e.go":                   "e",
 		},
 		autoApproveUnownedSubfolders: map[string]bool{
 			"d": true,
diff --git a/pkg/plugins/approve/approvers/approvers_test.go b/pkg/plugins/approve/approvers/approvers_test.go
index 2b795c8d5..33873ea3e 100644
--- a/pkg/plugins/approve/approvers/approvers_test.go
+++ b/pkg/plugins/approve/approvers/approvers_test.go
@@ -902,6 +902,44 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen
 	}
 }
 
+func TestGetMessageNotFullyApprovedAndAssigneeIsUsefulApprover(t *testing.T) {
+	ap := NewApprovers(
+		Owners{
+			filenames: []string{"a/a.go", "b/b.go"},
+			repo: createFakeRepo(map[string]sets.Set[string]{
+				"a": sets.New[string]("Alice"),
+				"b": sets.New[string]("Bill"),
+			}),
+			log: logrus.WithField("plugin", "some_plugin"),
+		},
+	)
+	ap.AddAssignees("Bill") // Bill is an approver for b/b.go and assignee.
+	ap.AddApprover("Alice", "REFERENCE", false)
+
+	want := `[APPROVALNOTIFIER] This PR is **NOT APPROVED**
+
+This pull-request has been approved by: *<a href="REFERENCE" title="Approved">Alice</a>*
+**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [bill](https://github.com/bill). For more information see [the Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).
+
+The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=org%2Frepo).
+
+<details open>
+Needs approval from an approver in each of these files:
+
+- ~~[a/OWNERS](https://github.com/org/repo/blob/master/a/OWNERS)~~ [Alice]
+- **[b/OWNERS](https://github.com/org/repo/blob/master/b/OWNERS)**
+
+Approvers can indicate their approval by writing ` + "`/approve`" + ` in a comment
+Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a comment
+</details>
+<!-- META={"approvers":["bill"]} -->`
+	if got := GetMessage(ap, &url.URL{Scheme: "https", Host: "github.com"}, "https://go.k8s.io/bot-commands", "https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process", "org", "repo", "master"); got == nil {
+		t.Error("GetMessage() failed")
+	} else if *got != want {
+		t.Errorf("GetMessage() = %+v, want = %+v", *got, want)
+	}
+}
+
 func TestGetMessageNoneApproved(t *testing.T) {
 	ap := NewApprovers(
 		Owners{
diff --git a/test/integration/test/sinker_test.go b/test/integration/test/sinker_test.go
index b9681ba4e..b9b7c8ff6 100644
--- a/test/integration/test/sinker_test.go
+++ b/test/integration/test/sinker_test.go
@@ -24,7 +24,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/wait"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	prowjobv1 "sigs.k8s.io/prow/pkg/apis/prowjobs/v1"
 )
@@ -283,35 +282,27 @@ func TestDeletePod(t *testing.T) {
 			}
 
 			// Make sure pod is deleted.
-			t.Logf("Wait for sinker deleting pod or timeout in 1 minute: %s", pod.Name)
-			var scheduled_for_deletion bool
-			err = wait.PollUntilContextTimeout(ctx, time.Second, 1*time.Minute, true, func(ctx context.Context) (bool, error) {
-				pods := &corev1.PodList{}
-				err := kubeClient.List(ctx, pods, ctrlruntimeclient.InNamespace(testpodNamespace))
-				if err != nil {
-					return false, err
-				}
-				for _, p := range pods.Items {
-					if p.Name == pod.Name {
-						if p.ObjectMeta.DeletionTimestamp != nil { // Pod scheduled to deletion
-							scheduled_for_deletion = true
-							break
-						}
-					}
+			t.Logf("Checking for sinker deleting pod after 1 minutes: %s", pod.Name)
+			time.Sleep(1 * time.Minute)
+			podDeleted := true
+			pods := &corev1.PodList{}
+			err = kubeClient.List(ctx, pods, ctrlruntimeclient.InNamespace(testpodNamespace))
+			if err != nil {
+				t.Fatalf("Failed listing pods: %v", err)
+			}
+			for _, p := range pods.Items {
+				if p.Name == pod.Name && p.ObjectMeta.DeletionTimestamp == nil {
+					podDeleted = false
 				}
-				return scheduled_for_deletion, nil
-			})
-			// Check for the error of `List` call.
-			if err != nil && !wait.Interrupted(err) {
-				t.Fatal(err)
 			}
-			t.Logf("Pod %s scheduled for deletion: %v", pod.Name, scheduled_for_deletion)
-			if want, got := tt.wantPodDeleted, scheduled_for_deletion; want != got {
-				t.Fatalf("wantPodDeleted: %v, but got scheduled_for_deletion: %v", want, got)
+
+			t.Logf("Pod %s scheduled for deletion: %v", pod.Name, podDeleted)
+			if want, got := tt.wantPodDeleted, podDeleted; want != got {
+				t.Fatalf("wantPodDeleted: %v, but got podDeleted: %v", want, got)
 			}
 
 			// Check for prowjob deletion.
-			var prowjob_exists bool
+			var prowjobExists bool
 			if prowjob != nil {
 				pjs := &prowjobv1.ProwJobList{}
 				err = kubeClient.List(ctx, pjs, ctrlruntimeclient.InNamespace(defaultNamespace))
@@ -320,11 +311,11 @@ func TestDeletePod(t *testing.T) {
 				}
 				for _, pj := range pjs.Items {
 					if pj.Name == prowjob.Name {
-						prowjob_exists = true
+						prowjobExists = true
 						break
 					}
 				}
-				if tt.wantJobDeleted && prowjob_exists {
+				if tt.wantJobDeleted && prowjobExists {
 					t.Fatalf("Wanted ProwJob deletion, but it still exists")
 				}
 			}