simple: simplify t.Errorf patterns where applicable

[spencerschrock]

I've noticed a few t.Errorf patterns during code review that work but could be simplified. They're not significant enough to manually flag in review, but I'd enforce it via a linter if simple supported them. All of my examples will use t.Errorf, but the same argument could be made for t.Error (or their B, F, or TB equivalents).

Is a rule like this something that could be a good fit for simple?

Unnecessary t.Fail()

Errorf is equivalent to Logf followed by Fail.

Example from one of my repos, where the t.Fail() call can be removed:

if err != nil && !tt.want.err {
	t.Errorf("CreateProportionalScoreWeighted unexpected error '%v'", err)
	t.Fail()
}

should be

if err != nil && !tt.want.err {
	t.Errorf("CreateProportionalScoreWeighted unexpected error '%v'", err)
}

t.Fatalf could be used instead

Fatalf is equivalent to Logf followed by FailNow.

Always equivalent

t.Errorf followed by t.FailNow can be replaced by t.Fatalf

Example occurrence from kubernetes/kubernetes

} else if err != nil {
	t.Errorf("unexpected error: %v", err)
	t.FailNow()
}

should be

} else if err != nil {
	t.Fatalf("unexpected error: %v", err)
}

Sometimes equivalent

t.Errorf followed by return can sometimes be replaced by t.Fatalf. Google's style guide documents one case where this isn't true. To avoid issues of knowing when we're in a goroutine, the suggestion could only apply to occurrences directly in a top level test (TestFoo (t *testing.T)) or directly in a t.Run.

example

	t.Run(tt.name, func(t *testing.T) {
		// ...	
		if (err != nil) != tt.wantErr {
			t.Errorf("SecurityPolicy() error = %v, wantErr %v", err, tt.wantErr)
			return
		}
		// ...	

should be

	t.Run(tt.name, func(t *testing.T) {
		// ...	
		if (err != nil) != tt.wantErr {
			t.Fatalf("SecurityPolicy() error = %v, wantErr %v", err, tt.wantErr)
		}
		// ...	

not equivalent example

Example not to flag:

func TestNextScheduleTime(t *testing.T) {
	logger, _ := ktesting.NewTestContext(t)
	// schedule is hourly on the hour
	schedule := "0 * * * ?"

	ParseSchedule := func(schedule string) cron.Schedule {
		sched, err := cron.ParseStandard(schedule)
		if err != nil {
			t.Errorf("Error parsing schedule: %#v", err)
			return nil
		}
		return sched
	}

[ccoVeille]

Hi

I'm a random gopher and I like your suggestion.

One remark, what you are asking is more a checker about the presence of FailNow + preceded by Errorf, Error, Log, or Logf than a checker about the Errorf usage.