clean unreachable code

[Abirdcfly]

Signed-off-by: Abirdcfly fp544037857@gmail.com

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

clean unreachable code

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[Abirdcfly]

/sig testing

[aojea]

Why fatal is unreachable and error no?

[Abirdcfly]

https://go.dev/play/p/5K3KZ5RzHhR

package main

import (
	"testing"
)

// LastIndex returns the index of the last instance of x in list, or
// -1 if x is not present.
func LastIndex(list []int, x int) int {
	for i := len(list) - 1; i >= 0; i-- {
		if list[i] == x {
			return i
		}
	}
	return -1
}

func TestLastIndex(t *testing.T) {
	tests := []struct {
		list []int
		x    int
		want int
	}{
		{list: []int{1}, x: 1, want: 0},
		{list: []int{1, 1}, x: 1, want: 3}, // change want:1 to want:3, should fail
		{list: []int{2, 1}, x: 2, want: 0},
		{list: []int{1, 2, 1, 1}, x: 2, want: 1},
		{list: []int{1, 1, 1, 2, 2, 1}, x: 3, want: -1},
		{list: []int{3, 1, 2, 2, 1, 1}, x: 3, want: 111}, // change want:0 to want:111, should fail
	}
	for i, tt := range tests {
		t.Logf("index:%d", i)
		if got := LastIndex(tt.list, tt.x); got != tt.want {
			t.Errorf("LastIndex(%v, %v) = %v, want %v", tt.list, tt.x, got, tt.want)
		}
	}
}

func TestLastIndex2(t *testing.T) {
	tests := []struct {
		list []int
		x    int
		want int
	}{
		{list: []int{1}, x: 1, want: 0},
		{list: []int{1, 1}, x: 1, want: 3}, // change want:1 to want:3, should fail
		{list: []int{2, 1}, x: 2, want: 0},
		{list: []int{1, 2, 1, 1}, x: 2, want: 1},
		{list: []int{1, 1, 1, 2, 2, 1}, x: 3, want: -1},
		{list: []int{3, 1, 2, 2, 1, 1}, x: 3, want: 111}, // change want:0 to want:111, should fail
	}
	for i, tt := range tests {
		t.Logf("index:%d", i)
		if got := LastIndex(tt.list, tt.x); got != tt.want {
			t.Fatalf("LastIndex(%v, %v) = %v, want %v", tt.list, tt.x, got, tt.want) // Fatalf is equivalent to Logf followed by FailNow, so the following code will be unreachable
			continue  // unreachable
		}
	}
}

/* output:

=== RUN   TestLastIndex
    prog.go:32: index:0
    prog.go:32: index:1
    prog.go:34: LastIndex([1 1], 1) = 1, want 3
    prog.go:32: index:2
    prog.go:32: index:3
    prog.go:32: index:4
    prog.go:32: index:5
    prog.go:34: LastIndex([3 1 2 2 1 1], 3) = 0, want 111
--- FAIL: TestLastIndex (0.00s)
=== RUN   TestLastIndex2
    prog.go:53: index:0
    prog.go:53: index:1
    prog.go:55: LastIndex([1 1], 1) = 1, want 3
--- FAIL: TestLastIndex2 (0.00s)
FAIL

*/

[aojea]

got it 👍

[liggitt]

all failures in this helper method should be fatal or the caller will continue running the test

if this truly is unreachable, we could switch this to an unchecked cast which will just panic if incorrect

[Abirdcfly]

all failures in this helper method should be fatal or the caller will continue running the test

I guess a more appropriate update would be to delete continue on line 1900 and keep t.Fatalf on line 1899.

[Abirdcfly]

all failures in this helper method should be fatal or the caller will continue running the test

I guess a more appropriate update would be to delete continue on line 1900 and keep t.Fatalf on line 1899.

Done. And more discussion in golang/go vet tool in golang/go#53968

[aojea]

/lgtm
/assign @liggitt

one comment though
c205b98#r924156868

[Abirdcfly]

/test pull-kubernetes-e2e-gce-ubuntu-containerd

[Abirdcfly]

Here I think a more appropriate way would be to change t.Fatalf to t.Errorf. Obviously, after the error the code tries to get the contents of res.Body

[Abirdcfly]

/test pull-kubernetes-e2e-kind

[leilajal]

/triage accepted

[aojea]

/lgtm

[Abirdcfly]

@liggitt golang tools just merged a similar issue golang/tools#387, but golang/go didn't find the problem😂.

Where else should I update here?

There is a related issue placed separately in #111241

[liggitt]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Abirdcfly, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/proxy/OWNERS [liggitt]
• staging/src/k8s.io/apiserver/OWNERS [liggitt]
• staging/src/k8s.io/client-go/OWNERS [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment