presubmit: Check errorf as well (#1845)

Uppercase all these test errors as well. And extend the presubmit to
check for these in the future. Also do a slightly smarter grep to only
get t.<something>. as (because dump regexp) this also grep over non test
files.

.presubmit/test-lowercase

@@ -3,12 +3,17 @@
 echo "** presubmit/$(basename $0)"
 
 # Get the tests that call t.* without capitalizing the first char - seems we standardized on that.
-if egrep -r 't\.Fatal.?\("[a-z]' "$@"; then
+if egrep -r '\bt\.Fatal.?\("[a-z]' "$@"; then
     echo "** presubmit/$(basename $0): please start with an upper case letter when using t.Fatal*()"
     exit 1
 fi
 
-if egrep -r 't\.Log.?\("[a-z]' "$@"; then
+if egrep -r '\bt\.Error.?\("[a-z]' "$@"; then
+    echo "** presubmit/$(basename $0): please start with an upper case letter when using t.Error*()"
+    exit 1
+fi
+
+if egrep -r '\bt\.Log.?\("[a-z]' "$@"; then
     echo "** presubmit/$(basename $0): please start with an upper case letter when using t.Log*()"
     exit 1
 fi

core/dnsserver/register_test.go

@@ -113,7 +113,7 @@ func TestGroupingServers(t *testing.T) {
 		}
 		for _, v := range test.expectedGroups {
 			if _, ok := groups[v]; !ok {
-				t.Errorf("test %d : expected value %v to be in the group, was not", i, v)
+				t.Errorf("Test %d : expected value %v to be in the group, was not", i, v)
 
 			}
 		}

plugin/cache/cache_test.go

@@ -175,7 +175,7 @@ func TestCache(t *testing.T) {
 		ok := i != nil
 
 		if ok != tc.shouldCache {
-			t.Errorf("cached message that should not have been cached: %s", state.Name())
+			t.Errorf("Cached message that should not have been cached: %s", state.Name())
 			continue
 		}
 

plugin/cache/spoof_test.go

@@ -24,7 +24,7 @@ func TestSpoof(t *testing.T) {
 
 	qname := rec.Msg.Question[0].Name
 	if c.pcache.Len() != 0 {
-		t.Errorf("cached %s, while reply had %s", "example.org.", qname)
+		t.Errorf("Cached %s, while reply had %s", "example.org.", qname)
 	}
 
 	// qtype
@@ -35,7 +35,7 @@ func TestSpoof(t *testing.T) {
 
 	qtype := rec.Msg.Question[0].Qtype
 	if c.pcache.Len() != 0 {
-		t.Errorf("cached %s type %d, while reply had %d", "example.org.", dns.TypeMX, qtype)
+		t.Errorf("Cached %s type %d, while reply had %d", "example.org.", dns.TypeMX, qtype)
 	}
 }
 
@@ -51,7 +51,7 @@ func TestResponse(t *testing.T) {
 	c.ServeDNS(context.TODO(), rec, req)
 
 	if c.pcache.Len() != 0 {
-		t.Errorf("cached %s, while reply had response set to %t", "example.net.", rec.Msg.Response)
+		t.Errorf("Cached %s, while reply had response set to %t", "example.net.", rec.Msg.Response)
 	}
 }
 

plugin/dnssec/black_lies_test.go

@@ -51,11 +51,11 @@ func TestBlackLiesNoError(t *testing.T) {
 	m = d.Sign(state, time.Now().UTC(), server)
 
 	if m.Rcode != dns.RcodeSuccess {
-		t.Errorf("expected rcode %d, got %d", dns.RcodeSuccess, m.Rcode)
+		t.Errorf("Expected rcode %d, got %d", dns.RcodeSuccess, m.Rcode)
 	}
 
 	if len(m.Answer) != 2 {
-		t.Errorf("answer section should have 2 RRs")
+		t.Errorf("Answer section should have 2 RRs")
 	}
 	sig, txt := false, false
 	for _, rr := range m.Answer {
@@ -67,7 +67,7 @@ func TestBlackLiesNoError(t *testing.T) {
 		}
 	}
 	if !sig || !txt {
-		t.Errorf("expected RRSIG and TXT in answer section")
+		t.Errorf("Expected RRSIG and TXT in answer section")
 	}
 }
 

plugin/dnssec/handler_test.go

@@ -118,7 +118,7 @@ func TestLookupZone(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := dh.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			return
 		}
 
@@ -141,7 +141,7 @@ func TestLookupDNSKEY(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := dh.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			return
 		}
 
@@ -157,7 +157,7 @@ func TestLookupDNSKEY(t *testing.T) {
 			if n, ok := rr.(*dns.NSEC); ok {
 				for i := range n.TypeBitMap {
 					if n.TypeBitMap[i] == tc.Qtype {
-						t.Errorf("bitmap contains qtype: %d", tc.Qtype)
+						t.Errorf("Bitmap contains qtype: %d", tc.Qtype)
 					}
 				}
 			}

plugin/dnstap/handler_test.go

@@ -42,7 +42,7 @@ func (w *writer) Dnstap(e tap.Dnstap) {
 		w.t.Error("Message not expected.")
 	}
 	if !test.MsgEqual(w.queue[0], e.Message) {
-		w.t.Errorf("want: %v, have: %v", w.queue[0], e.Message)
+		w.t.Errorf("Want: %v, have: %v", w.queue[0], e.Message)
 	}
 	w.queue = w.queue[1:]
 }

plugin/dnstap/setup_test.go

@@ -29,7 +29,7 @@ func TestConfig(t *testing.T) {
 		} else if err != nil || conf.target != c.path ||
 			conf.full != c.full || conf.socket != c.socket {
 
-			t.Errorf("expected: %+v\nhave: %+v\nerror: %s\n", c, conf, err)
+			t.Errorf("Expected: %+v\nhave: %+v\nerror: %s\n", c, conf, err)
 		}
 	}
 }

plugin/etcd/cname_test.go

@@ -28,7 +28,7 @@ func TestCnameLookup(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := etc.ServeDNS(ctxt, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			return
 		}
 

plugin/etcd/group_test.go

@@ -25,7 +25,7 @@ func TestGroupLookup(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := etc.ServeDNS(ctxt, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			continue
 		}
 

plugin/etcd/multi_test.go

@@ -27,7 +27,7 @@ func TestMultiLookup(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := etc.ServeDNS(ctxt, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			return
 		}
 

plugin/etcd/other_test.go

@@ -29,7 +29,7 @@ func TestOtherLookup(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := etc.ServeDNS(ctxt, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			continue
 		}
 

plugin/etcd/stub_test.go

@@ -58,7 +58,7 @@ func TestStubLookup(t *testing.T) {
 			continue
 		}
 		if err != nil {
-			t.Errorf("expected no error, got %v for %s\n", err, m.Question[0].Name)
+			t.Errorf("Expected no error, got %v for %s\n", err, m.Question[0].Name)
 		}
 		resp := rec.Msg
 		if resp == nil {

plugin/file/ent_test.go

@@ -45,7 +45,7 @@ func TestLookupEnt(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := fm.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			return
 		}
 

plugin/file/lookup_test.go

@@ -117,7 +117,7 @@ func TestLookup(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := fm.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v\n", err)
 			return
 		}
 

plugin/pkg/edns/edns_test.go

@@ -12,7 +12,7 @@ func TestVersion(t *testing.T) {
 
 	_, err := Version(m)
 	if err == nil {
-		t.Errorf("expected wrong version, but got OK")
+		t.Errorf("Expected wrong version, but got OK")
 	}
 }
 
@@ -22,7 +22,7 @@ func TestVersionNoEdns(t *testing.T) {
 
 	_, err := Version(m)
 	if err != nil {
-		t.Errorf("expected no error, but got one: %s", err)
+		t.Errorf("Expected no error, but got one: %s", err)
 	}
 }
 

plugin/pkg/response/typify_test.go

@@ -14,7 +14,7 @@ func TestTypifyNilMsg(t *testing.T) {
 
 	ty, _ := Typify(m, time.Now().UTC())
 	if ty != OtherError {
-		t.Errorf("message wrongly typified, expected OtherError, got %s", ty)
+		t.Errorf("Message wrongly typified, expected OtherError, got %s", ty)
 	}
 }
 
@@ -22,7 +22,7 @@ func TestTypifyDelegation(t *testing.T) {
 	m := delegationMsg()
 	mt, _ := Typify(m, time.Now().UTC())
 	if mt != Delegation {
-		t.Errorf("message is wrongly typified, expected Delegation, got %s", mt)
+		t.Errorf("Message is wrongly typified, expected Delegation, got %s", mt)
 	}
 }
 
@@ -32,19 +32,19 @@ func TestTypifyRRSIG(t *testing.T) {
 
 	m := delegationMsgRRSIGOK()
 	if mt, _ := Typify(m, utc); mt != Delegation {
-		t.Errorf("message is wrongly typified, expected Delegation, got %s", mt)
+		t.Errorf("Message is wrongly typified, expected Delegation, got %s", mt)
 	}
 
 	// Still a Delegation because EDNS0 OPT DO bool is not set, so we won't check the sigs.
 	m = delegationMsgRRSIGFail()
 	if mt, _ := Typify(m, utc); mt != Delegation {
-		t.Errorf("message is wrongly typified, expected Delegation, got %s", mt)
+		t.Errorf("Message is wrongly typified, expected Delegation, got %s", mt)
 	}
 
 	m = delegationMsgRRSIGFail()
 	m = addOpt(m)
 	if mt, _ := Typify(m, utc); mt != OtherError {
-		t.Errorf("message is wrongly typified, expected OtherError, got %s", mt)
+		t.Errorf("Message is wrongly typified, expected OtherError, got %s", mt)
 	}
 }
 

plugin/pkg/singleflight/singleflight_test.go

@@ -48,7 +48,7 @@ func TestDoErr(t *testing.T) {
 		t.Errorf("Do error = %v; want someErr", err)
 	}
 	if v != nil {
-		t.Errorf("unexpected non-nil value %#v", v)
+		t.Errorf("Unexpected non-nil value %#v", v)
 	}
 }
 
@@ -71,7 +71,7 @@ func TestDoDupSuppress(t *testing.T) {
 				t.Errorf("Do error: %v", err)
 			}
 			if v.(string) != "bar" {
-				t.Errorf("got %q; want %q", v, "bar")
+				t.Errorf("Got %q; want %q", v, "bar")
 			}
 			wg.Done()
 		}()
@@ -80,6 +80,6 @@ func TestDoDupSuppress(t *testing.T) {
 	c <- "bar"
 	wg.Wait()
 	if got := atomic.LoadInt32(&calls); got != 1 {
-		t.Errorf("number of calls = %d; want 1", got)
+		t.Errorf("Number of calls = %d; want 1", got)
 	}
 }

plugin/test/helpers.go

@@ -115,20 +115,20 @@ func OPT(bufsize int, do bool) *dns.OPT {
 // Header test if the header in resp matches the header as defined in tc.
 func Header(t *testing.T, tc Case, resp *dns.Msg) bool {
 	if resp.Rcode != tc.Rcode {
-		t.Errorf("rcode is %q, expected %q", dns.RcodeToString[resp.Rcode], dns.RcodeToString[tc.Rcode])
+		t.Errorf("Rcode is %q, expected %q", dns.RcodeToString[resp.Rcode], dns.RcodeToString[tc.Rcode])
 		return false
 	}
 
 	if len(resp.Answer) != len(tc.Answer) {
-		t.Errorf("answer for %q contained %d results, %d expected", tc.Qname, len(resp.Answer), len(tc.Answer))
+		t.Errorf("Answer for %q contained %d results, %d expected", tc.Qname, len(resp.Answer), len(tc.Answer))
 		return false
 	}
 	if len(resp.Ns) != len(tc.Ns) {
-		t.Errorf("authority for %q contained %d results, %d expected", tc.Qname, len(resp.Ns), len(tc.Ns))
+		t.Errorf("Authority for %q contained %d results, %d expected", tc.Qname, len(resp.Ns), len(tc.Ns))
 		return false
 	}
 	if len(resp.Extra) != len(tc.Extra) {
-		t.Errorf("additional for %q contained %d results, %d expected", tc.Qname, len(resp.Extra), len(tc.Extra))
+		t.Errorf("Additional for %q contained %d results, %d expected", tc.Qname, len(resp.Extra), len(tc.Extra))
 		return false
 	}
 	return true
@@ -148,82 +148,82 @@ func Section(t *testing.T, tc Case, sec sect, rr []dns.RR) bool {
 
 	for i, a := range rr {
 		if a.Header().Name != section[i].Header().Name {
-			t.Errorf("rr %d should have a Header Name of %q, but has %q", i, section[i].Header().Name, a.Header().Name)
+			t.Errorf("RR %d should have a Header Name of %q, but has %q", i, section[i].Header().Name, a.Header().Name)
 			return false
 		}
 		// 303 signals: don't care what the ttl is.
 		if section[i].Header().Ttl != 303 && a.Header().Ttl != section[i].Header().Ttl {
 			if _, ok := section[i].(*dns.OPT); !ok {
 				// we check edns0 bufize on this one
-				t.Errorf("rr %d should have a Header TTL of %d, but has %d", i, section[i].Header().Ttl, a.Header().Ttl)
+				t.Errorf("RR %d should have a Header TTL of %d, but has %d", i, section[i].Header().Ttl, a.Header().Ttl)
 				return false
 			}
 		}
 		if a.Header().Rrtype != section[i].Header().Rrtype {
-			t.Errorf("rr %d should have a header rr type of %d, but has %d", i, section[i].Header().Rrtype, a.Header().Rrtype)
+			t.Errorf("RR %d should have a header rr type of %d, but has %d", i, section[i].Header().Rrtype, a.Header().Rrtype)
 			return false
 		}
 
 		switch x := a.(type) {
 		case *dns.SRV:
 			if x.Priority != section[i].(*dns.SRV).Priority {
-				t.Errorf("rr %d should have a Priority of %d, but has %d", i, section[i].(*dns.SRV).Priority, x.Priority)
+				t.Errorf("RR %d should have a Priority of %d, but has %d", i, section[i].(*dns.SRV).Priority, x.Priority)
 				return false
 			}
 			if x.Weight != section[i].(*dns.SRV).Weight {
-				t.Errorf("rr %d should have a Weight of %d, but has %d", i, section[i].(*dns.SRV).Weight, x.Weight)
+				t.Errorf("RR %d should have a Weight of %d, but has %d", i, section[i].(*dns.SRV).Weight, x.Weight)
 				return false
 			}
 			if x.Port != section[i].(*dns.SRV).Port {
-				t.Errorf("rr %d should have a Port of %d, but has %d", i, section[i].(*dns.SRV).Port, x.Port)
+				t.Errorf("RR %d should have a Port of %d, but has %d", i, section[i].(*dns.SRV).Port, x.Port)
 				return false
 			}
 			if x.Target != section[i].(*dns.SRV).Target {
-				t.Errorf("rr %d should have a Target of %q, but has %q", i, section[i].(*dns.SRV).Target, x.Target)
+				t.Errorf("RR %d should have a Target of %q, but has %q", i, section[i].(*dns.SRV).Target, x.Target)
 				return false
 			}
 		case *dns.RRSIG:
 			if x.TypeCovered != section[i].(*dns.RRSIG).TypeCovered {
-				t.Errorf("rr %d should have a TypeCovered of %d, but has %d", i, section[i].(*dns.RRSIG).TypeCovered, x.TypeCovered)
+				t.Errorf("RR %d should have a TypeCovered of %d, but has %d", i, section[i].(*dns.RRSIG).TypeCovered, x.TypeCovered)
 				return false
 			}
 			if x.Labels != section[i].(*dns.RRSIG).Labels {
-				t.Errorf("rr %d should have a Labels of %d, but has %d", i, section[i].(*dns.RRSIG).Labels, x.Labels)
+				t.Errorf("RR %d should have a Labels of %d, but has %d", i, section[i].(*dns.RRSIG).Labels, x.Labels)
 				return false
 			}
 			if x.SignerName != section[i].(*dns.RRSIG).SignerName {
-				t.Errorf("rr %d should have a SignerName of %s, but has %s", i, section[i].(*dns.RRSIG).SignerName, x.SignerName)
+				t.Errorf("RR %d should have a SignerName of %s, but has %s", i, section[i].(*dns.RRSIG).SignerName, x.SignerName)
 				return false
 			}
 		case *dns.NSEC:
 			if x.NextDomain != section[i].(*dns.NSEC).NextDomain {
-				t.Errorf("rr %d should have a NextDomain of %s, but has %s", i, section[i].(*dns.NSEC).NextDomain, x.NextDomain)
+				t.Errorf("RR %d should have a NextDomain of %s, but has %s", i, section[i].(*dns.NSEC).NextDomain, x.NextDomain)
 				return false
 			}
 			// TypeBitMap
 		case *dns.A:
 			if x.A.String() != section[i].(*dns.A).A.String() {
-				t.Errorf("rr %d should have a Address of %q, but has %q", i, section[i].(*dns.A).A.String(), x.A.String())
+				t.Errorf("RR %d should have a Address of %q, but has %q", i, section[i].(*dns.A).A.String(), x.A.String())
 				return false
 			}
 		case *dns.AAAA:
 			if x.AAAA.String() != section[i].(*dns.AAAA).AAAA.String() {
-				t.Errorf("rr %d should have a Address of %q, but has %q", i, section[i].(*dns.AAAA).AAAA.String(), x.AAAA.String())
+				t.Errorf("RR %d should have a Address of %q, but has %q", i, section[i].(*dns.AAAA).AAAA.String(), x.AAAA.String())
 				return false
 			}
 		case *dns.TXT:
 			for j, txt := range x.Txt {
 				if txt != section[i].(*dns.TXT).Txt[j] {
-					t.Errorf("rr %d should have a Txt of %q, but has %q", i, section[i].(*dns.TXT).Txt[j], txt)
+					t.Errorf("RR %d should have a Txt of %q, but has %q", i, section[i].(*dns.TXT).Txt[j], txt)
 					return false
 				}
 			}
 		case *dns.HINFO:
 			if x.Cpu != section[i].(*dns.HINFO).Cpu {
-				t.Errorf("rr %d should have a Cpu of %s, but has %s", i, section[i].(*dns.HINFO).Cpu, x.Cpu)
+				t.Errorf("RR %d should have a Cpu of %s, but has %s", i, section[i].(*dns.HINFO).Cpu, x.Cpu)
 			}
 			if x.Os != section[i].(*dns.HINFO).Os {
-				t.Errorf("rr %d should have a Os of %s, but has %s", i, section[i].(*dns.HINFO).Os, x.Os)
+				t.Errorf("RR %d should have a Os of %s, but has %s", i, section[i].(*dns.HINFO).Os, x.Os)
 			}
 		case *dns.SOA:
 			tt := section[i].(*dns.SOA)