From 22c0b30d5f7468c2477291419bee3e88dfba490c Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 2 Jun 2018 19:48:39 +0100 Subject: [PATCH] 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.. as (because dump regexp) this also grep over non test files. --- .presubmit/test-lowercase | 9 ++++- core/dnsserver/register_test.go | 2 +- plugin/cache/cache_test.go | 2 +- plugin/cache/spoof_test.go | 6 +-- plugin/dnssec/black_lies_test.go | 6 +-- plugin/dnssec/handler_test.go | 6 +-- plugin/dnstap/handler_test.go | 2 +- plugin/dnstap/setup_test.go | 2 +- plugin/etcd/cname_test.go | 2 +- plugin/etcd/group_test.go | 2 +- plugin/etcd/multi_test.go | 2 +- plugin/etcd/other_test.go | 2 +- plugin/etcd/stub_test.go | 2 +- plugin/file/ent_test.go | 2 +- plugin/file/lookup_test.go | 2 +- plugin/pkg/edns/edns_test.go | 4 +- plugin/pkg/response/typify_test.go | 10 ++--- plugin/pkg/singleflight/singleflight_test.go | 6 +-- plugin/test/helpers.go | 40 ++++++++++---------- 19 files changed, 57 insertions(+), 52 deletions(-) diff --git a/.presubmit/test-lowercase b/.presubmit/test-lowercase index 5db137e5f..705228886 100755 --- a/.presubmit/test-lowercase +++ b/.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 diff --git a/core/dnsserver/register_test.go b/core/dnsserver/register_test.go index 0079bb2c8..a2d24e1dd 100644 --- a/core/dnsserver/register_test.go +++ b/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) } } diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go index e67521986..c2ab74920 100644 --- a/plugin/cache/cache_test.go +++ b/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 } diff --git a/plugin/cache/spoof_test.go b/plugin/cache/spoof_test.go index 71930f4dc..04d714f4a 100644 --- a/plugin/cache/spoof_test.go +++ b/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) } } diff --git a/plugin/dnssec/black_lies_test.go b/plugin/dnssec/black_lies_test.go index b704d67e9..a9a29029e 100644 --- a/plugin/dnssec/black_lies_test.go +++ b/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") } } diff --git a/plugin/dnssec/handler_test.go b/plugin/dnssec/handler_test.go index d2d566af9..a1c35c635 100644 --- a/plugin/dnssec/handler_test.go +++ b/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) } } } diff --git a/plugin/dnstap/handler_test.go b/plugin/dnstap/handler_test.go index 01dbb873a..b86fe019d 100644 --- a/plugin/dnstap/handler_test.go +++ b/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:] } diff --git a/plugin/dnstap/setup_test.go b/plugin/dnstap/setup_test.go index efac6aa56..eeeb45027 100644 --- a/plugin/dnstap/setup_test.go +++ b/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) } } } diff --git a/plugin/etcd/cname_test.go b/plugin/etcd/cname_test.go index dc3213655..bf27573cc 100644 --- a/plugin/etcd/cname_test.go +++ b/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 } diff --git a/plugin/etcd/group_test.go b/plugin/etcd/group_test.go index 049bbd922..f5493dc1f 100644 --- a/plugin/etcd/group_test.go +++ b/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 } diff --git a/plugin/etcd/multi_test.go b/plugin/etcd/multi_test.go index b06d0bc7e..9d5062677 100644 --- a/plugin/etcd/multi_test.go +++ b/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 } diff --git a/plugin/etcd/other_test.go b/plugin/etcd/other_test.go index a06a982c2..d37a04706 100644 --- a/plugin/etcd/other_test.go +++ b/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 } diff --git a/plugin/etcd/stub_test.go b/plugin/etcd/stub_test.go index c71089e0e..c4a262b86 100644 --- a/plugin/etcd/stub_test.go +++ b/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 { diff --git a/plugin/file/ent_test.go b/plugin/file/ent_test.go index 64244a44d..496d1a727 100644 --- a/plugin/file/ent_test.go +++ b/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 } diff --git a/plugin/file/lookup_test.go b/plugin/file/lookup_test.go index 897b3aa90..97cae6b0e 100644 --- a/plugin/file/lookup_test.go +++ b/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 } diff --git a/plugin/pkg/edns/edns_test.go b/plugin/pkg/edns/edns_test.go index 89ac6d2ec..a775b50f1 100644 --- a/plugin/pkg/edns/edns_test.go +++ b/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) } } diff --git a/plugin/pkg/response/typify_test.go b/plugin/pkg/response/typify_test.go index faeaf3579..6be9aa8ff 100644 --- a/plugin/pkg/response/typify_test.go +++ b/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) } } diff --git a/plugin/pkg/singleflight/singleflight_test.go b/plugin/pkg/singleflight/singleflight_test.go index d1d406e0b..a32e046db 100644 --- a/plugin/pkg/singleflight/singleflight_test.go +++ b/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) } } diff --git a/plugin/test/helpers.go b/plugin/test/helpers.go index 97cb510f3..70159e99a 100644 --- a/plugin/test/helpers.go +++ b/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)