Stop importing testing in the main binary (#2479)

* Stop importing testing in the main binary

Stop importing "testing" into the main binary:

* test/helpers.go imported it; remote that and change function signature
* update all tests that use this

Signed-off-by: Miek Gieben <miek@miek.nl>

* Drop import testing from metrics plugin

Signed-off-by: Miek Gieben <miek@miek.nl>

* more fiddling

Signed-off-by: Miek Gieben <miek@miek.nl>

plugin/autopath/autopath_test.go

@@ -52,7 +52,7 @@ func TestAutoPath(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := ap.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("Expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v", err)
 			continue
 		}
 
@@ -60,18 +60,18 @@ func TestAutoPath(t *testing.T) {
 		// test of the answer.
 		resp := rec.Msg
 
-		if !test.Header(t, tc, resp) {
+		if err := test.Header(tc, resp); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 			continue
 		}
-		if !test.Section(t, tc, test.Answer, resp.Answer) {
+		if err := test.Section(tc, test.Answer, resp.Answer); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 		}
-		if !test.Section(t, tc, test.Ns, resp.Ns) {
+		if err := test.Section(tc, test.Ns, resp.Ns); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 		}
-		if !test.Section(t, tc, test.Extra, resp.Extra) {
+		if err := test.Section(tc, test.Extra, resp.Extra); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 		}
 	}
 }
@@ -97,7 +97,7 @@ func TestAutoPathNoAnswer(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		rcode, err := ap.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("Expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v", err)
 			continue
 		}
 		if plugin.ClientWrite(rcode) {

plugin/cache/cache_test.go

@@ -184,20 +184,19 @@ func TestCache(t *testing.T) {
 		if ok {
 			resp := i.toMsg(m, time.Now().UTC())
 
-			if !test.Header(t, tc.Case, resp) {
+			if err := test.Header(tc.Case, resp); err != nil {
-				t.Logf("%v\n", resp)
+				t.Error(err)
 				continue
 			}
 
-			if !test.Section(t, tc.Case, test.Answer, resp.Answer) {
+			if err := test.Section(tc.Case, test.Answer, resp.Answer); err != nil {
-				t.Logf("%v\n", resp)
+				t.Error(err)
 			}
-			if !test.Section(t, tc.Case, test.Ns, resp.Ns) {
+			if err := test.Section(tc.Case, test.Ns, resp.Ns); err != nil {
-				t.Logf("%v\n", resp)
+				t.Error(err)
-
 			}
-			if !test.Section(t, tc.Case, test.Extra, resp.Extra) {
+			if err := test.Section(tc.Case, test.Extra, resp.Extra); err != nil {
-				t.Logf("%v\n", resp)
+				t.Error(err)
 			}
 		}
 	}

plugin/dnssec/handler_test.go

@@ -112,11 +112,13 @@ func TestLookupZone(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := dh.ServeDNS(context.TODO(), rec, m)
 		if err != nil {
-			t.Errorf("Expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v", err)
 			return
 		}
 
-		test.SortAndCheck(t, rec.Msg, tc)
+		if err := test.SortAndCheck(rec.Msg, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 
@@ -133,7 +135,7 @@ func TestLookupDNSKEY(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := dh.ServeDNS(context.TODO(), rec, m)
 		if err != nil {
-			t.Errorf("Expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v", err)
 			return
 		}
 
@@ -142,7 +144,9 @@ func TestLookupDNSKEY(t *testing.T) {
 			t.Errorf("Authoritative Answer should be true, got false")
 		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 
 		// If there is an NSEC present in authority section check if the bitmap does not have the qtype set.
 		for _, rr := range resp.Ns {

plugin/dnstap/setup_test.go

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

plugin/etcd/cname_test.go

@@ -28,23 +28,23 @@ 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", err)
 			return
 		}
 
 		resp := rec.Msg
-		if !test.Header(t, tc, resp) {
+		if err := test.Header(tc, resp); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 			continue
 		}
-		if !test.Section(t, tc, test.Answer, resp.Answer) {
+		if err := test.Section(tc, test.Answer, resp.Answer); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 		}
-		if !test.Section(t, tc, test.Ns, resp.Ns) {
+		if err := test.Section(tc, test.Ns, resp.Ns); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 		}
-		if !test.Section(t, tc, test.Extra, resp.Extra) {
+		if err := test.Section(tc, test.Extra, resp.Extra); err != nil {
-			t.Logf("%v\n", resp)
+			t.Error(err)
 		}
 	}
 }

plugin/etcd/group_test.go

@@ -25,12 +25,14 @@ 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", err)
 			continue
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/etcd/lookup_test.go

@@ -319,7 +319,9 @@ func TestLookup(t *testing.T) {
 		etc.ServeDNS(ctxt, rec, m)
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/etcd/multi_test.go

@@ -27,12 +27,14 @@ 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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/etcd/other_test.go

@@ -29,12 +29,14 @@ 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", err)
 			continue
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/federation/federation_test.go

@@ -71,12 +71,14 @@ func TestFederationKubernetes(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := fed.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("Test %d, expected no error, got %v\n", i, err)
+			t.Errorf("Test %d, expected no error, got %v", i, err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/delegation_test.go

@@ -173,12 +173,14 @@ func testDelegation(t *testing.T, z, origin string, testcases []test.Case) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := fm.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("Expected no error, got %q\n", err)
+			t.Errorf("Expected no error, got %q", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/dname_test.go

@@ -107,7 +107,7 @@ func TestLookupDNAME(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", err)
 			return
 		}
 
@@ -156,7 +156,7 @@ func TestLookupDNAMEDNSSEC(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", err)
 			return
 		}
 

plugin/file/dnssec_test.go

@@ -131,12 +131,14 @@ func TestLookupDNSSEC(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/ds_test.go

@@ -65,11 +65,13 @@ func TestLookupDS(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/file/ent_test.go

@@ -44,12 +44,14 @@ 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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/glue_test.go

@@ -46,12 +46,14 @@ func TestLookupGlue(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/lookup_test.go

@@ -117,12 +117,14 @@ 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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/tree/less_test.go

@@ -65,7 +65,7 @@ Tests:
 		sort.Sort(set(test.in))
 		for i := 0; i < len(test.in); i++ {
 			if test.in[i] != test.out[i] {
-				t.Errorf("Test %d: expected %s, got %s\n", j, test.out[i], test.in[i])
+				t.Errorf("Test %d: expected %s, got %s", j, test.out[i], test.in[i])
 				n := ""
 				for k, in := range test.in {
 					if k+1 == len(test.in) {

plugin/file/wildcard_test.go

@@ -90,12 +90,14 @@ func TestLookupWildcard(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 
@@ -151,12 +153,14 @@ func TestLookupDoubleWildcard(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 
@@ -206,12 +210,14 @@ func TestLookupApexWildcard(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 
@@ -249,12 +255,14 @@ func TestLookupMultiWildcard(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", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/file/zone_test.go

@@ -21,10 +21,10 @@ func TestNameFromRight(t *testing.T) {
 	for i, tc := range tests {
 		got, shot := z.nameFromRight(tc.in, tc.labels)
 		if got != tc.expected {
-			t.Errorf("Test %d: expected %s, got %s\n", i, tc.expected, got)
+			t.Errorf("Test %d: expected %s, got %s", i, tc.expected, got)
 		}
 		if shot != tc.shot {
-			t.Errorf("Test %d: expected shot to be %t, got %t\n", i, tc.shot, shot)
+			t.Errorf("Test %d: expected shot to be %t, got %t", i, tc.shot, shot)
 		}
 	}
 }

plugin/hosts/hosts_test.go

@@ -26,12 +26,14 @@ func TestLookupA(t *testing.T) {
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		_, err := h.ServeDNS(ctx, rec, m)
 		if err != nil {
-			t.Errorf("Expected no error, got %v\n", err)
+			t.Errorf("Expected no error, got %v", err)
 			return
 		}
 
 		resp := rec.Msg
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/k8s_external/apex_test.go

@@ -40,7 +40,9 @@ func TestApex(t *testing.T) {
 		if resp == nil {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/k8s_external/external_test.go

@@ -45,7 +45,9 @@ func TestExternal(t *testing.T) {
 		if resp == nil {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
-		test.SortAndCheck(t, resp, tc)
+		if err = test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/kubernetes/external_test.go

@@ -61,16 +61,16 @@ func TestExternal(t *testing.T) {
 		svc, rcode := k.External(state)
 
 		if x := tc.Rcode; x != rcode {
-			t.Errorf("Test %d, expected rcode %d, got %d\n", i, x, rcode)
+			t.Errorf("Test %d, expected rcode %d, got %d", i, x, rcode)
 		}
 
 		if len(svc) != len(tc.Msg) {
-			t.Errorf("Test %d, expected %d for messages, got %d\n", i, len(tc.Msg), len(svc))
+			t.Errorf("Test %d, expected %d for messages, got %d", i, len(tc.Msg), len(svc))
 		}
 
 		for j, s := range svc {
 			if x := tc.Msg[j].Key; x != s.Key {
-				t.Errorf("Test %d, expected key %s, got %s\n", i, x, s.Key)
+				t.Errorf("Test %d, expected key %s, got %s", i, x, s.Key)
 			}
 			return
 		}

plugin/kubernetes/handler_case_test.go

@@ -66,6 +66,8 @@ func TestPreserveCase(t *testing.T) {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/kubernetes/handler_ignore_emptyservice_test.go

@@ -49,8 +49,12 @@ func TestServeDNSEmptyService(t *testing.T) {
 		}
 
 		// Before sorting, make sure that CNAMES do not appear after their target records
-		test.CNAMEOrder(t, resp)
+		if err := test.CNAMEOrder(resp); err != nil {
+			t.Error(err)
+		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/kubernetes/handler_pod_disabled_test.go

@@ -54,6 +54,8 @@ func TestServeDNSModeDisabled(t *testing.T) {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/kubernetes/handler_pod_insecure_test.go

@@ -89,6 +89,8 @@ func TestServeDNSModeInsecure(t *testing.T) {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/kubernetes/handler_pod_verified_test.go

@@ -75,6 +75,8 @@ func TestServeDNSModeVerified(t *testing.T) {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/kubernetes/handler_test.go

@@ -371,9 +371,13 @@ func TestServeDNS(t *testing.T) {
 		}
 
 		// Before sorting, make sure that CNAMES do not appear after their target records
-		test.CNAMEOrder(t, resp)
+		if err := test.CNAMEOrder(resp); err != nil {
+			t.Error(err)
+		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 
@@ -417,10 +421,13 @@ func TestNotSyncedServeDNS(t *testing.T) {
 			t.Fatalf("Test %d, got nil message and no error for %q", i, r.Question[0].Name)
 		}
 
-		// Before sorting, make sure that CNAMES do not appear after their target records
+		if err := test.CNAMEOrder(resp); err != nil {
-		test.CNAMEOrder(t, resp)
+			t.Error(err)
+		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }
 

plugin/kubernetes/kubernetes_apex_test.go

@@ -72,7 +72,7 @@ func TestServeDNSApex(t *testing.T) {
 
 		_, err := k.ServeDNS(ctx, w, r)
 		if err != tc.Error {
-			t.Errorf("Test %d, expected no error, got %v\n", i, err)
+			t.Errorf("Test %d, expected no error, got %v", i, err)
 			return
 		}
 		if tc.Error != nil {
@@ -84,6 +84,8 @@ func TestServeDNSApex(t *testing.T) {
 			t.Fatalf("Test %d, got nil message and no error ford", i)
 		}
 
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/kubernetes/reverse_test.go

@@ -200,6 +200,8 @@ func TestReverse(t *testing.T) {
 		if resp == nil {
 			t.Fatalf("Test %d: got nil message and no error for: %s %d", i, r.Question[0].Name, r.Question[0].Qtype)
 		}
-		test.SortAndCheck(t, resp, tc)
+		if err := test.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

plugin/metrics/metrics_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"github.com/coredns/coredns/plugin"
-	mtest "github.com/coredns/coredns/plugin/metrics/test"
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/test"
 
@@ -71,10 +70,10 @@ func TestMetrics(t *testing.T) {
 			t.Fatalf("Test %d: Expected no error, but got %s", i, err)
 		}
 
-		result := mtest.Scrape(t, "http://"+ListenAddr+"/metrics")
+		result := test.Scrape("http://" + ListenAddr + "/metrics")
 
 		if tc.expectedValue != "" {
-			got, _ := mtest.MetricValue(tc.metric, result)
+			got, _ := test.MetricValue(tc.metric, result)
 			if got != tc.expectedValue {
 				t.Errorf("Test %d: Expected value %s for metrics %s, but got %s", i, tc.expectedValue, tc.metric, got)
 			}

plugin/normalize_test.go

@@ -26,7 +26,7 @@ func TestZoneNormalize(t *testing.T) {
 
 	for _, actual := range zones {
 		if actual != expected {
-			t.Errorf("Expected %v, got %v\n", expected, actual)
+			t.Errorf("Expected %v, got %v", expected, actual)
 		}
 	}
 }
@@ -63,7 +63,7 @@ func TestNameNormalize(t *testing.T) {
 		expected := names[i+1]
 		actual := Name(ts).Normalize()
 		if expected != actual {
-			t.Errorf("Expected %v, got %v\n", expected, actual)
+			t.Errorf("Expected %v, got %v", expected, actual)
 		}
 	}
 }
@@ -78,7 +78,7 @@ func TestHostNormalize(t *testing.T) {
 		expected := hosts[i+1]
 		actual := Host(ts).Normalize()
 		if expected != actual {
-			t.Errorf("Expected %v, got %v\n", expected, actual)
+			t.Errorf("Expected %v, got %v", expected, actual)
 		}
 	}
 }

plugin/reload/reload.go

@@ -56,7 +56,7 @@ func hook(event caddy.EventName, info interface{}) error {
 					r.usage = maybeUsed
 					_, err := instance.Restart(corefile)
 					if err != nil {
-						log.Errorf("Corefile changed but reload failed: %s\n", err)
+						log.Errorf("Corefile changed but reload failed: %s", err)
 						continue
 					}
 					// we are done, if the plugin was not set used, then it is not.

plugin/test/helpers.go

@@ -2,8 +2,8 @@ package test
 
 import (
 	"context"
+	"fmt"
 	"sort"
-	"testing"
 
 	"github.com/miekg/dns"
 )
@@ -113,29 +113,25 @@ 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 {
+func Header(tc Case, resp *dns.Msg) error {
 	if resp.Rcode != tc.Rcode {
-		t.Errorf("Rcode is %q, expected %q", dns.RcodeToString[resp.Rcode], dns.RcodeToString[tc.Rcode])
+		return fmt.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))
+		return fmt.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))
+		return fmt.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))
+		return fmt.Errorf("Additional for %q contained %d results, %d expected", tc.Qname, len(resp.Extra), len(tc.Extra))
-		return false
 	}
-	return true
+	return nil
 }
 
 // Section tests if the the section in tc matches rr.
-func Section(t *testing.T, tc Case, sec sect, rr []dns.RR) bool {
+func Section(tc Case, sec sect, rr []dns.RR) error {
 	section := []dns.RR{}
 	switch sec {
 	case 0:
@@ -148,134 +144,112 @@ 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)
+			return fmt.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)
+				return fmt.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)
+			return fmt.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)
+				return fmt.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)
+				return fmt.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)
+				return fmt.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)
+				return fmt.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)
+				return fmt.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)
+				return fmt.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)
+				return fmt.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)
+				return fmt.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())
+				return fmt.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())
+				return fmt.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)
+					return fmt.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)
+				return fmt.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)
+				return fmt.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)
 			if x.Ns != tt.Ns {
-				t.Errorf("SOA nameserver should be %q, but is %q", tt.Ns, x.Ns)
+				return fmt.Errorf("SOA nameserver should be %q, but is %q", tt.Ns, x.Ns)
-				return false
 			}
 		case *dns.PTR:
 			tt := section[i].(*dns.PTR)
 			if x.Ptr != tt.Ptr {
-				t.Errorf("PTR ptr should be %q, but is %q", tt.Ptr, x.Ptr)
+				return fmt.Errorf("PTR ptr should be %q, but is %q", tt.Ptr, x.Ptr)
-				return false
 			}
 		case *dns.CNAME:
 			tt := section[i].(*dns.CNAME)
 			if x.Target != tt.Target {
-				t.Errorf("CNAME target should be %q, but is %q", tt.Target, x.Target)
+				return fmt.Errorf("CNAME target should be %q, but is %q", tt.Target, x.Target)
-				return false
 			}
 		case *dns.MX:
 			tt := section[i].(*dns.MX)
 			if x.Mx != tt.Mx {
-				t.Errorf("MX Mx should be %q, but is %q", tt.Mx, x.Mx)
+				return fmt.Errorf("MX Mx should be %q, but is %q", tt.Mx, x.Mx)
-				return false
 			}
 			if x.Preference != tt.Preference {
-				t.Errorf("MX Preference should be %q, but is %q", tt.Preference, x.Preference)
+				return fmt.Errorf("MX Preference should be %q, but is %q", tt.Preference, x.Preference)
-				return false
 			}
 		case *dns.NS:
 			tt := section[i].(*dns.NS)
 			if x.Ns != tt.Ns {
-				t.Errorf("NS nameserver should be %q, but is %q", tt.Ns, x.Ns)
+				return fmt.Errorf("NS nameserver should be %q, but is %q", tt.Ns, x.Ns)
-				return false
 			}
 		case *dns.OPT:
 			tt := section[i].(*dns.OPT)
 			if x.UDPSize() != tt.UDPSize() {
-				t.Errorf("OPT UDPSize should be %d, but is %d", tt.UDPSize(), x.UDPSize())
+				return fmt.Errorf("OPT UDPSize should be %d, but is %d", tt.UDPSize(), x.UDPSize())
-				return false
 			}
 			if x.Do() != tt.Do() {
-				t.Errorf("OPT DO should be %t, but is %t", tt.Do(), x.Do())
+				return fmt.Errorf("OPT DO should be %t, but is %t", tt.Do(), x.Do())
-				return false
 			}
 		}
 	}
-	return true
+	return nil
 }
 
 // CNAMEOrder makes sure that CNAMES do not appear after their target records
-func CNAMEOrder(t *testing.T, res *dns.Msg) {
+func CNAMEOrder(res *dns.Msg) error {
 	for i, c := range res.Answer {
 		if c.Header().Rrtype != dns.TypeCNAME {
 			continue
@@ -284,38 +258,32 @@ func CNAMEOrder(t *testing.T, res *dns.Msg) {
 			if a.Header().Name != c.(*dns.CNAME).Target {
 				continue
 			}
-			t.Errorf("CNAME found after target record\n")
+			return fmt.Errorf("CNAME found after target record")
-			t.Logf("%v\n", res)
-
 		}
 	}
+	return nil
 }
 
 // SortAndCheck sorts resp and the checks the header and three sections against the testcase in tc.
-func SortAndCheck(t *testing.T, resp *dns.Msg, tc Case) {
+func SortAndCheck(resp *dns.Msg, tc Case) error {
 	sort.Sort(RRSet(resp.Answer))
 	sort.Sort(RRSet(resp.Ns))
 	sort.Sort(RRSet(resp.Extra))
 
-	if !Header(t, tc, resp) {
+	if err := Header(tc, resp); err != nil {
-		t.Logf("%v\n", resp)
+		return err
-		return
 	}
-
+	if err := Section(tc, Answer, resp.Answer); err != nil {
-	if !Section(t, tc, Answer, resp.Answer) {
+		return err
-		t.Logf("%v\n", resp)
-		return
 	}
-	if !Section(t, tc, Ns, resp.Ns) {
+	if err := Section(tc, Ns, resp.Ns); err != nil {
-		t.Logf("%v\n", resp)
+		return err
-		return
 
 	}
-	if !Section(t, tc, Extra, resp.Extra) {
+	if err := Section(tc, Extra, resp.Extra); err != nil {
-		t.Logf("%v\n", resp)
+		return err
-		return
 	}
-	return
+	return nil
 }
 
 // ErrorHandler returns a Handler that returns ServerFailure error when called.

plugin/test/scrape.go

@@ -28,7 +28,6 @@ import (
 	"mime"
 	"net/http"
 	"strconv"
-	"testing"
 
 	"github.com/matttproud/golang_protobuf_extensions/pbutil"
 	"github.com/prometheus/common/expfmt"
@@ -67,7 +66,7 @@ type (
 )
 
 // Scrape returns the all the vars a []*metricFamily.
-func Scrape(t *testing.T, url string) []*MetricFamily {
+func Scrape(url string) []*MetricFamily {
 	mfChan := make(chan *dto.MetricFamily, 1024)
 
 	go fetchMetricFamilies(url, mfChan)
@@ -81,7 +80,7 @@ func Scrape(t *testing.T, url string) []*MetricFamily {
 
 // ScrapeMetricAsInt provide a sum of all metrics collected for the name and label provided.
 // if the metric is not a numeric value, it will be counted a 0.
-func ScrapeMetricAsInt(t *testing.T, addr string, name string, label string, nometricvalue int) int {
+func ScrapeMetricAsInt(addr string, name string, label string, nometricvalue int) int {
 
 	valueToInt := func(m metric) int {
 		v := m.Value
@@ -92,7 +91,7 @@ func ScrapeMetricAsInt(t *testing.T, addr string, name string, label string, nom
 		return r
 	}
 
-	met := Scrape(t, fmt.Sprintf("http://%s/metrics", addr))
+	met := Scrape(fmt.Sprintf("http://%s/metrics", addr))
 	found := false
 	tot := 0
 	for _, mf := range met {

test/ds_file_test.go

@@ -53,6 +53,8 @@ func TestLookupDS(t *testing.T) {
 			t.Fatalf("Expected to receive reply, but didn't for %s %d", tc.Qname, tc.Qtype)
 		}
 
-		mtest.SortAndCheck(t, resp, tc)
+		if err := mtest.SortAndCheck(resp, tc); err != nil {
+			t.Error(err)
+		}
 	}
 }

test/metrics_test.go

@@ -8,10 +8,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/coredns/coredns/plugin/cache"
 	"github.com/coredns/coredns/plugin/metrics"
-	mtest "github.com/coredns/coredns/plugin/metrics/test"
 	"github.com/coredns/coredns/plugin/metrics/vars"
+	"github.com/coredns/coredns/plugin/test"
 
 	"github.com/miekg/dns"
 )
@@ -56,8 +55,8 @@ func TestMetricsRefused(t *testing.T) {
 		t.Fatalf("Could not send message: %s", err)
 	}
 
-	data := mtest.Scrape(t, "http://"+metrics.ListenAddr+"/metrics")
+	data := test.Scrape("http://" + metrics.ListenAddr + "/metrics")
-	got, labels := mtest.MetricValue(metricName, data)
+	got, labels := test.MetricValue(metricName, data)
 
 	if got != "1" {
 		t.Errorf("Expected value %s for refused, but got %s", "1", got)
@@ -70,67 +69,6 @@ func TestMetricsRefused(t *testing.T) {
 	}
 }
 
-// TODO(miek): disabled for now - fails in weird ways in travis.
-func TestMetricsCache(t *testing.T) {
-	cacheSizeMetricName := "coredns_cache_size"
-	cacheHitMetricName := "coredns_cache_hits_total"
-
-	corefile := `example.net:0 {
-	proxy . 8.8.8.8:53
-	prometheus localhost:0
-	cache
-}
-`
-	srv, err := CoreDNSServer(corefile)
-	if err != nil {
-		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
-	}
-	defer srv.Stop()
-
-	udp, _ := CoreDNSServerPorts(srv, 0)
-
-	// send an initial query to set properly the cache size metric
-	m := new(dns.Msg)
-	m.SetQuestion("example.net.", dns.TypeA)
-
-	if _, err = dns.Exchange(m, udp); err != nil {
-		t.Fatalf("Could not send message: %s", err)
-	}
-
-	beginCacheSizeSuccess := mtest.ScrapeMetricAsInt(t, metrics.ListenAddr, cacheSizeMetricName, cache.Success, 0)
-	beginCacheHitSuccess := mtest.ScrapeMetricAsInt(t, metrics.ListenAddr, cacheHitMetricName, cache.Success, 0)
-
-	m = new(dns.Msg)
-	m.SetQuestion("www.example.net.", dns.TypeA)
-
-	if _, err = dns.Exchange(m, udp); err != nil {
-		t.Fatalf("Could not send message: %s", err)
-	}
-
-	// Get the value for the cache size metric where the one of the labels values matches "success".
-	got := mtest.ScrapeMetricAsInt(t, metrics.ListenAddr, cacheSizeMetricName, cache.Success, 0)
-
-	if got-beginCacheSizeSuccess != 1 {
-		t.Errorf("Expected value %d for %s, but got %d", 1, cacheSizeMetricName, got-beginCacheSizeSuccess)
-	}
-
-	// Second request for the same response to test hit counter
-	if _, err = dns.Exchange(m, udp); err != nil {
-		t.Fatalf("Could not send message: %s", err)
-	}
-	// Third request for the same response to test hit counter for the second time
-	if _, err = dns.Exchange(m, udp); err != nil {
-		t.Fatalf("Could not send message: %s", err)
-	}
-
-	// Get the value for the cache hit counter where the one of the labels values matches "success".
-	got = mtest.ScrapeMetricAsInt(t, metrics.ListenAddr, cacheHitMetricName, cache.Success, 0)
-
-	if got-beginCacheHitSuccess != 2 {
-		t.Errorf("Expected value %d for %s, but got %d", 2, cacheHitMetricName, got-beginCacheHitSuccess)
-	}
-}
-
 func TestMetricsAuto(t *testing.T) {
 	tmpdir, err := ioutil.TempDir(os.TempDir(), "coredns")
 	if err != nil {
@@ -172,9 +110,9 @@ func TestMetricsAuto(t *testing.T) {
 
 	metricName := "coredns_dns_request_count_total" //{zone, proto, family}
 
-	data := mtest.Scrape(t, "http://"+metrics.ListenAddr+"/metrics")
+	data := test.Scrape("http://" + metrics.ListenAddr + "/metrics")
 	// Get the value for the metrics where the one of the labels values matches "example.org."
-	got, _ := mtest.MetricValueLabel(metricName, "example.org.", data)
+	got, _ := test.MetricValueLabel(metricName, "example.org.", data)
 
 	if got != "1" {
 		t.Errorf("Expected value %s for %s, but got %s", "1", metricName, got)
@@ -187,8 +125,8 @@ func TestMetricsAuto(t *testing.T) {
 		t.Fatalf("Could not send message: %s", err)
 	}
 
-	data = mtest.Scrape(t, "http://"+metrics.ListenAddr+"/metrics")
+	data = test.Scrape("http://" + metrics.ListenAddr + "/metrics")
-	got, _ = mtest.MetricValueLabel(metricName, "example.org.", data)
+	got, _ = test.MetricValueLabel(metricName, "example.org.", data)
 
 	if got != "1" {
 		t.Errorf("Expected value %s for %s, but got %s", "1", metricName, got)
@@ -230,7 +168,7 @@ google.com:0 {
 		t.Fatalf("Could not send message: %s", err)
 	}
 
-	beginCacheSize := mtest.ScrapeMetricAsInt(t, addrMetrics, cacheSizeMetricName, "", 0)
+	beginCacheSize := test.ScrapeMetricAsInt(addrMetrics, cacheSizeMetricName, "", 0)
 
 	// send an query, different from initial to ensure we have another add to the cache
 	m = new(dns.Msg)
@@ -240,7 +178,7 @@ google.com:0 {
 		t.Fatalf("Could not send message: %s", err)
 	}
 
-	endCacheSize := mtest.ScrapeMetricAsInt(t, addrMetrics, cacheSizeMetricName, "", 0)
+	endCacheSize := test.ScrapeMetricAsInt(addrMetrics, cacheSizeMetricName, "", 0)
 	if err != nil {
 		t.Errorf("Unexpected metric data retrieved for %s : %s", cacheSizeMetricName, err)
 	}