From fa33568fab9889d4467c0c5593d2bea8486e3342 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Thu, 4 Feb 2021 15:12:34 +0300 Subject: [PATCH] Pull request: all: imp errcheck in tests Merge in DNS/adguard-home from imp-errcheck to master Squashed commit of the following: commit ed046b8ef59a092a27c623cd14b3fc2ef305fc3d Author: Ainar Garipov Date: Thu Feb 4 15:00:04 2021 +0300 stats: imp cleanup more commit e53a9240d3e3eec2417c768b98c267a8cd54d992 Merge: da734317 676f8c76 Author: Ainar Garipov Date: Thu Feb 4 14:59:40 2021 +0300 Merge branch 'master' into imp-errcheck commit da734317035543b52e5a9030813084bdc92ba90a Author: Ainar Garipov Date: Thu Feb 4 14:37:26 2021 +0300 stats: imp cleanup commit 8b4ad150129111a09be6fa2944a21bd06ab8e5a1 Merge: 385c8a6c 5081ead0 Author: Ainar Garipov Date: Thu Feb 4 14:34:26 2021 +0300 Merge branch 'master' into imp-errcheck commit 385c8a6c91e3bf07a457da370c8cc77820b91600 Author: Ainar Garipov Date: Fri Jan 29 20:41:57 2021 +0300 all: imp errcheck in tests --- internal/dhcpd/dhcpd_test.go | 9 ++++++-- internal/dhcpd/v4_test.go | 9 +++++--- internal/dhcpd/v6_test.go | 9 +++++--- internal/dnsfilter/dnsfilter_test.go | 5 +++- internal/dnsforward/dnsforward_test.go | 12 ++++++---- internal/dnsforward/dnsrewrite_test.go | 32 +++++++++++++++----------- internal/home/auth_test.go | 1 - internal/home/middlewares_test.go | 5 +++- internal/stats/stats_test.go | 11 ++++++--- scripts/make/go-lint.sh | 5 ++-- 10 files changed, 65 insertions(+), 33 deletions(-) diff --git a/internal/dhcpd/dhcpd_test.go b/internal/dhcpd/dhcpd_test.go index f65e6823..f5c34154 100644 --- a/internal/dhcpd/dhcpd_test.go +++ b/internal/dhcpd/dhcpd_test.go @@ -45,12 +45,17 @@ func TestDB(t *testing.T) { l.HWAddr, _ = net.ParseMAC("aa:aa:aa:aa:aa:aa") exp1 := time.Now().Add(time.Hour) l.Expiry = exp1 - s.srv4.(*v4Server).addLease(&l) + + srv4, ok := s.srv4.(*v4Server) + assert.True(t, ok) + + srv4.addLease(&l) l2 := Lease{} l2.IP = net.IP{192, 168, 10, 101} l2.HWAddr, _ = net.ParseMAC("aa:aa:aa:aa:aa:bb") - s.srv4.AddStaticLease(l2) + err = s.srv4.AddStaticLease(l2) + assert.Nil(t, err) _ = os.Remove("leases.db") s.dbStore() diff --git a/internal/dhcpd/v4_test.go b/internal/dhcpd/v4_test.go index 74eab484..8edb3113 100644 --- a/internal/dhcpd/v4_test.go +++ b/internal/dhcpd/v4_test.go @@ -69,7 +69,8 @@ func TestV4StaticLeaseAddReplaceDynamic(t *testing.T) { notify: notify4, } sIface, err := v4Create(conf) - s := sIface.(*v4Server) + s, ok := sIface.(*v4Server) + assert.True(t, ok) assert.Nil(t, err) // add dynamic lease @@ -121,7 +122,8 @@ func TestV4StaticLeaseGet(t *testing.T) { notify: notify4, } sIface, err := v4Create(conf) - s := sIface.(*v4Server) + s, ok := sIface.(*v4Server) + assert.True(t, ok) assert.Nil(t, err) s.conf.dnsIPAddrs = []net.IP{{192, 168, 10, 1}} @@ -184,7 +186,8 @@ func TestV4DynamicLeaseGet(t *testing.T) { }, } sIface, err := v4Create(conf) - s := sIface.(*v4Server) + s, ok := sIface.(*v4Server) + assert.True(t, ok) assert.Nil(t, err) s.conf.dnsIPAddrs = []net.IP{{192, 168, 10, 1}} diff --git a/internal/dhcpd/v6_test.go b/internal/dhcpd/v6_test.go index 1c54163a..9cdf3ee4 100644 --- a/internal/dhcpd/v6_test.go +++ b/internal/dhcpd/v6_test.go @@ -64,7 +64,8 @@ func TestV6StaticLeaseAddReplaceDynamic(t *testing.T) { notify: notify6, } sIface, err := v6Create(conf) - s := sIface.(*v6Server) + s, ok := sIface.(*v6Server) + assert.True(t, ok) assert.Nil(t, err) // add dynamic lease @@ -113,7 +114,8 @@ func TestV6GetLease(t *testing.T) { notify: notify6, } sIface, err := v6Create(conf) - s := sIface.(*v6Server) + s, ok := sIface.(*v6Server) + assert.True(t, ok) assert.Nil(t, err) s.conf.dnsIPAddrs = []net.IP{net.ParseIP("2000::1")} s.sid = dhcpv6.Duid{ @@ -173,7 +175,8 @@ func TestV6GetDynamicLease(t *testing.T) { notify: notify6, } sIface, err := v6Create(conf) - s := sIface.(*v6Server) + s, ok := sIface.(*v6Server) + assert.True(t, ok) assert.Nil(t, err) s.conf.dnsIPAddrs = []net.IP{net.ParseIP("2000::1")} s.sid = dhcpv6.Duid{ diff --git a/internal/dnsfilter/dnsfilter_test.go b/internal/dnsfilter/dnsfilter_test.go index 257953ad..6ab4fbc4 100644 --- a/internal/dnsfilter/dnsfilter_test.go +++ b/internal/dnsfilter/dnsfilter_test.go @@ -683,7 +683,10 @@ func TestWhitelist(t *testing.T) { ID: 0, Data: []byte(whiteRules), }} d := newForTest(nil, filters) - d.SetFilters(filters, whiteFilters, false) + + err := d.SetFilters(filters, whiteFilters, false) + assert.Nil(t, err) + t.Cleanup(d.Close) // Matched by white filter. diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index 86356c6b..9b72dd52 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -1113,8 +1113,10 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) { assert.Equal(t, dns.TypePTR, resp.Answer[0].Header().Rrtype) assert.Equal(t, "1.0.0.127.in-addr.arpa.", resp.Answer[0].Header().Name) - ptr := resp.Answer[0].(*dns.PTR) - assert.Equal(t, "localhost.", ptr.Ptr) + ptr, ok := resp.Answer[0].(*dns.PTR) + if assert.True(t, ok) { + assert.Equal(t, "localhost.", ptr.Ptr) + } s.Close() } @@ -1158,8 +1160,10 @@ func TestPTRResponseFromHosts(t *testing.T) { assert.Equal(t, dns.TypePTR, resp.Answer[0].Header().Rrtype) assert.Equal(t, "1.0.0.127.in-addr.arpa.", resp.Answer[0].Header().Name) - ptr := resp.Answer[0].(*dns.PTR) - assert.Equal(t, "host.", ptr.Ptr) + ptr, ok := resp.Answer[0].(*dns.PTR) + if assert.True(t, ok) { + assert.Equal(t, "host.", ptr.Ptr) + } s.Close() } diff --git a/internal/dnsforward/dnsrewrite_test.go b/internal/dnsforward/dnsrewrite_test.go index b3f5ecf3..4029038a 100644 --- a/internal/dnsforward/dnsrewrite_test.go +++ b/internal/dnsforward/dnsrewrite_test.go @@ -130,9 +130,11 @@ func TestServer_FilterDNSRewrite(t *testing.T) { assert.Nil(t, err) assert.Equal(t, dns.RcodeSuccess, d.Res.Rcode) if assert.Len(t, d.Res.Answer, 1) { - ans := d.Res.Answer[0].(*dns.MX) - assert.Equal(t, mx.Exchange, ans.Mx) - assert.Equal(t, mx.Preference, ans.Preference) + ans, ok := d.Res.Answer[0].(*dns.MX) + if assert.True(t, ok) { + assert.Equal(t, mx.Exchange, ans.Mx) + assert.Equal(t, mx.Preference, ans.Preference) + } } }) @@ -145,11 +147,13 @@ func TestServer_FilterDNSRewrite(t *testing.T) { assert.Nil(t, err) assert.Equal(t, dns.RcodeSuccess, d.Res.Rcode) if assert.Len(t, d.Res.Answer, 1) { - ans := d.Res.Answer[0].(*dns.SVCB) - assert.Equal(t, dns.SVCB_ALPN, ans.Value[0].Key()) - assert.Equal(t, svcb.Params["alpn"], ans.Value[0].String()) - assert.Equal(t, svcb.Target, ans.Target) - assert.Equal(t, svcb.Priority, ans.Priority) + ans, ok := d.Res.Answer[0].(*dns.SVCB) + if assert.True(t, ok) { + assert.Equal(t, dns.SVCB_ALPN, ans.Value[0].Key()) + assert.Equal(t, svcb.Params["alpn"], ans.Value[0].String()) + assert.Equal(t, svcb.Target, ans.Target) + assert.Equal(t, svcb.Priority, ans.Priority) + } } }) @@ -162,11 +166,13 @@ func TestServer_FilterDNSRewrite(t *testing.T) { assert.Nil(t, err) assert.Equal(t, dns.RcodeSuccess, d.Res.Rcode) if assert.Len(t, d.Res.Answer, 1) { - ans := d.Res.Answer[0].(*dns.HTTPS) - assert.Equal(t, dns.SVCB_ALPN, ans.Value[0].Key()) - assert.Equal(t, svcb.Params["alpn"], ans.Value[0].String()) - assert.Equal(t, svcb.Target, ans.Target) - assert.Equal(t, svcb.Priority, ans.Priority) + ans, ok := d.Res.Answer[0].(*dns.HTTPS) + if assert.True(t, ok) { + assert.Equal(t, dns.SVCB_ALPN, ans.Value[0].Key()) + assert.Equal(t, svcb.Params["alpn"], ans.Value[0].String()) + assert.Equal(t, svcb.Target, ans.Target) + assert.Equal(t, svcb.Priority, ans.Priority) + } } }) } diff --git a/internal/home/auth_test.go b/internal/home/auth_test.go index 4a4a21c8..e44b7e83 100644 --- a/internal/home/auth_test.go +++ b/internal/home/auth_test.go @@ -79,7 +79,6 @@ func TestAuth(t *testing.T) { assert.Equal(t, checkSessionNotFound, a.checkSession(sessStr)) a.Close() - os.Remove(fn) } // implements http.ResponseWriter diff --git a/internal/home/middlewares_test.go b/internal/home/middlewares_test.go index 4d6a33d0..53b7a933 100644 --- a/internal/home/middlewares_test.go +++ b/internal/home/middlewares_test.go @@ -42,7 +42,10 @@ func TestLimitRequestBody(t *testing.T) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var b []byte b, *err = ioutil.ReadAll(r.Body) - w.Write(b) + _, werr := w.Write(b) + if werr != nil { + panic(werr) + } }) } diff --git a/internal/stats/stats_test.go b/internal/stats/stats_test.go index c4fbe191..06163c6c 100644 --- a/internal/stats/stats_test.go +++ b/internal/stats/stats_test.go @@ -34,6 +34,10 @@ func TestStats(t *testing.T) { Filename: "./stats.db", LimitDays: 1, } + t.Cleanup(func() { + assert.Nil(t, os.Remove(conf.Filename)) + }) + s, _ := createObject(conf) e := Entry{} @@ -86,7 +90,6 @@ func TestStats(t *testing.T) { s.clear() s.Close() - os.Remove(conf.Filename) } func TestLargeNumbers(t *testing.T) { @@ -102,7 +105,10 @@ func TestLargeNumbers(t *testing.T) { LimitDays: 1, UnitID: newID, } - os.Remove(conf.Filename) + t.Cleanup(func() { + assert.Nil(t, os.Remove(conf.Filename)) + }) + s, _ := createObject(conf) e := Entry{} @@ -128,7 +134,6 @@ func TestLargeNumbers(t *testing.T) { assert.EqualValues(t, int(hour)*n, d.NumDNSQueries) s.Close() - os.Remove(conf.Filename) } // this code is a chunk copied from getData() that generates aggregate data per day diff --git a/scripts/make/go-lint.sh b/scripts/make/go-lint.sh index 59c4cbf1..e1aeb9ef 100644 --- a/scripts/make/go-lint.sh +++ b/scripts/make/go-lint.sh @@ -140,7 +140,8 @@ ineffassign . unparam ./... -git ls-files -- '*.go' '*.md' '*.yaml' '*.yml' 'Makefile'\ +git ls-files -- '*.go' '*.md' '*.mod' '*.sh' '*.yaml' '*.yml'\ + 'Makefile'\ | xargs misspell --error looppointer ./... @@ -157,7 +158,7 @@ nilness ./... # errcheck ./... exit_on_output sh -c ' errcheck --asserts --ignoregenerated ./... |\ - { grep -e "defer" -e "_test\.go:" -v || exit 0; } + { grep -e "defer" -v || exit 0; } ' staticcheck ./...