Pull request: 2639 use testify require vol.2

Merge in DNS/adguard-home from 2639-testify-require-2 to master

Updates #2639.

Squashed commit of the following:

commit 31cc29a166e2e48a73956853cbc6d6dd681ab6da
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Feb 9 18:48:31 2021 +0300

    all: deal with t.Run

commit 484f477fbfedd03aca4d322bc1cc9e131f30e1ce
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Feb 9 17:44:02 2021 +0300

    all: fix readability, imp tests

commit 1231a825b353c16e43eae1b660dbb4c87805f564
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Feb 9 16:06:29 2021 +0300

    all: imp tests

internal/aghtest/os.go

@@ -0,0 +1,45 @@
+package aghtest
+
+import (
+	"io/ioutil"
+	"os"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// PrepareTestDir returns the full path to temporary created directory and
+// registers the appropriate cleanup for *t.
+func PrepareTestDir(t *testing.T) (dir string) {
+	t.Helper()
+
+	wd, err := os.Getwd()
+	require.Nil(t, err)
+
+	dir, err = ioutil.TempDir(wd, "agh-test")
+	require.Nil(t, err)
+	require.NotEmpty(t, dir)
+
+	t.Cleanup(func() {
+		// TODO(e.burkov): Replace with t.TempDir methods after updating
+		// go version to 1.15.
+		start := time.Now()
+		for {
+			err := os.RemoveAll(dir)
+			if err == nil {
+				break
+			}
+
+			if runtime.GOOS != "windows" || time.Since(start) >= 500*time.Millisecond {
+				break
+			}
+			time.Sleep(5 * time.Millisecond)
+		}
+		assert.Nil(t, err)
+	})
+
+	return dir
+}

internal/querylog/qlog_test.go

@@ -2,11 +2,8 @@ package querylog
 
 import (
 	"fmt"
-	"io/ioutil"
 	"math/rand"
 	"net"
-	"os"
-	"runtime"
 	"sort"
 	"testing"
 	"time"
@@ -24,38 +21,6 @@ func TestMain(m *testing.M) {
 	aghtest.DiscardLogOutput(m)
 }
 
-func prepareTestDir(t *testing.T) string {
-	t.Helper()
-
-	wd, err := os.Getwd()
-	require.Nil(t, err)
-
-	dir, err := ioutil.TempDir(wd, "agh-tests")
-	require.Nil(t, err)
-	require.NotEmpty(t, dir)
-
-	t.Cleanup(func() {
-		// TODO(e.burkov): Replace with t.TempDir methods after updating
-		// go version to 1.15.
-		start := time.Now()
-		for {
-			err := os.RemoveAll(dir)
-			if err == nil {
-				break
-			}
-
-			if runtime.GOOS != "windows" || time.Since(start) >= 500*time.Millisecond {
-				break
-			}
-			time.Sleep(5 * time.Millisecond)
-		}
-
-		assert.Nil(t, err)
-	})
-
-	return dir
-}
-
 // TestQueryLog tests adding and loading (with filtering) entries from disk and
 // memory.
 func TestQueryLog(t *testing.T) {
@@ -64,7 +29,7 @@ func TestQueryLog(t *testing.T) {
 		FileEnabled: true,
 		Interval:    1,
 		MemSize:     100,
-		BaseDir:     prepareTestDir(t),
+		BaseDir:     aghtest.PrepareTestDir(t),
 	})
 
 	// Add disk entries.
@@ -166,7 +131,7 @@ func TestQueryLogOffsetLimit(t *testing.T) {
 		Enabled:  true,
 		Interval: 1,
 		MemSize:  100,
-		BaseDir:  prepareTestDir(t),
+		BaseDir:  aghtest.PrepareTestDir(t),
 	})
 
 	const (
@@ -240,7 +205,7 @@ func TestQueryLogMaxFileScanEntries(t *testing.T) {
 		FileEnabled: true,
 		Interval:    1,
 		MemSize:     100,
-		BaseDir:     prepareTestDir(t),
+		BaseDir:     aghtest.PrepareTestDir(t),
 	})
 
 	const entNum = 10
@@ -268,7 +233,7 @@ func TestQueryLogFileDisabled(t *testing.T) {
 		FileEnabled: false,
 		Interval:    1,
 		MemSize:     2,
-		BaseDir:     prepareTestDir(t),
+		BaseDir:     aghtest.PrepareTestDir(t),
 	})
 
 	addEntry(l, "example1.org", net.IPv4(1, 1, 1, 1), net.IPv4(2, 2, 2, 1))

internal/querylog/qlogfile_test.go

@@ -12,15 +12,20 @@ import (
 	"testing"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 // prepareTestFiles prepares several test query log files, each with the
 // specified lines count.
-func prepareTestFiles(t *testing.T, dir string, filesNum, linesNum int) []string {
+func prepareTestFiles(t *testing.T, filesNum, linesNum int) []string {
 	t.Helper()
 
+	if filesNum == 0 {
+		return []string{}
+	}
+
 	const strV = "\"%s\""
 	const nl = "\n"
 	const format = `{"IP":` + strV + `,"T":` + strV + `,` +
@@ -31,6 +36,8 @@ func prepareTestFiles(t *testing.T, dir string, filesNum, linesNum int) []string
 	lineTime, _ := time.Parse(time.RFC3339Nano, "2020-02-18T22:36:35.920973+03:00")
 	lineIP := uint32(0)
 
+	dir := aghtest.PrepareTestDir(t)
+
 	files := make([]string, filesNum)
 	for j := range files {
 		f, err := ioutil.TempFile(dir, "*.txt")
@@ -56,10 +63,10 @@ func prepareTestFiles(t *testing.T, dir string, filesNum, linesNum int) []string
 
 // prepareTestFile prepares a test query log file with the specified number of
 // lines.
-func prepareTestFile(t *testing.T, dir string, linesCount int) string {
+func prepareTestFile(t *testing.T, linesCount int) string {
 	t.Helper()
 
-	return prepareTestFiles(t, dir, 1, linesCount)[0]
+	return prepareTestFiles(t, 1, linesCount)[0]
 }
 
 // newTestQLogFile creates new *QLogFile for tests and registers the required
@@ -67,7 +74,7 @@ func prepareTestFile(t *testing.T, dir string, linesCount int) string {
 func newTestQLogFile(t *testing.T, linesNum int) (file *QLogFile) {
 	t.Helper()
 
-	testFile := prepareTestFile(t, prepareTestDir(t), linesNum)
+	testFile := prepareTestFile(t, linesNum)
 
 	// Create the new QLogFile instance.
 	file, err := NewQLogFile(testFile)
@@ -275,7 +282,7 @@ func TestQLogFile(t *testing.T) {
 }
 
 func NewTestQLogFileData(t *testing.T, data string) (file *QLogFile) {
-	f, err := ioutil.TempFile(prepareTestDir(t), "*.txt")
+	f, err := ioutil.TempFile(aghtest.PrepareTestDir(t), "*.txt")
 	require.Nil(t, err)
 	t.Cleanup(func() {
 		assert.Nil(t, f.Close())

internal/querylog/qlogreader_test.go

@@ -15,7 +15,7 @@ import (
 func newTestQLogReader(t *testing.T, filesNum, linesNum int) (reader *QLogReader) {
 	t.Helper()
 
-	testFiles := prepareTestFiles(t, prepareTestDir(t), filesNum, linesNum)
+	testFiles := prepareTestFiles(t, filesNum, linesNum)
 
 	// Create the new QLogReader instance.
 	reader, err := NewQLogReader(testFiles)

internal/stats/stats_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestMain(m *testing.M) {
@@ -34,28 +35,30 @@ func TestStats(t *testing.T) {
 		Filename:  "./stats.db",
 		LimitDays: 1,
 	}
+
+	s, err := createObject(conf)
+	require.Nil(t, err)
 	t.Cleanup(func() {
+		s.clear()
+		s.Close()
 		assert.Nil(t, os.Remove(conf.Filename))
 	})
 
-	s, _ := createObject(conf)
+	s.Update(Entry{
-
+		Domain: "domain",
-	e := Entry{}
+		Client: "127.0.0.1",
-
+		Result: RFiltered,
-	e.Domain = "domain"
+		Time:   123456,
-	e.Client = "127.0.0.1"
+	})
-	e.Result = RFiltered
+	s.Update(Entry{
-	e.Time = 123456
+		Domain: "domain",
-	s.Update(e)
+		Client: "127.0.0.1",
-
+		Result: RNotFiltered,
-	e.Domain = "domain"
+		Time:   123456,
-	e.Client = "127.0.0.1"
+	})
-	e.Result = RNotFiltered
-	e.Time = 123456
-	s.Update(e)
 
 	d, ok := s.getData()
-	assert.True(t, ok)
+	require.True(t, ok)
 
 	a := []uint64{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}
 	assert.True(t, UIntArrayEquals(d.DNSQueries, a))
@@ -70,12 +73,15 @@ func TestStats(t *testing.T) {
 	assert.True(t, UIntArrayEquals(d.ReplacedParental, a))
 
 	m := d.TopQueried
+	require.NotEmpty(t, m)
 	assert.EqualValues(t, 1, m[0]["domain"])
 
 	m = d.TopBlocked
+	require.NotEmpty(t, m)
 	assert.EqualValues(t, 1, m[0]["domain"])
 
 	m = d.TopClients
+	require.NotEmpty(t, m)
 	assert.EqualValues(t, 2, m[0]["127.0.0.1"])
 
 	assert.EqualValues(t, 2, d.NumDNSQueries)
@@ -86,81 +92,69 @@ func TestStats(t *testing.T) {
 	assert.EqualValues(t, 0.123456, d.AvgProcessingTime)
 
 	topClients := s.GetTopClientsIP(2)
+	require.NotEmpty(t, topClients)
 	assert.True(t, net.IP{127, 0, 0, 1}.Equal(topClients[0]))
-
-	s.clear()
-	s.Close()
 }
 
 func TestLargeNumbers(t *testing.T) {
-	var hour int32 = 1
+	var hour int32 = 0
 	newID := func() uint32 {
-		// use "atomic" to make Go race detector happy
+		// Use "atomic" to make go race detector happy.
 		return uint32(atomic.LoadInt32(&hour))
 	}
 
-	// log.SetLevel(log.DEBUG)
 	conf := Config{
 		Filename:  "./stats.db",
 		LimitDays: 1,
 		UnitID:    newID,
 	}
+	s, err := createObject(conf)
+	require.Nil(t, err)
 	t.Cleanup(func() {
+		s.Close()
 		assert.Nil(t, os.Remove(conf.Filename))
 	})
 
-	s, _ := createObject(conf)
+	// Number of distinct clients and domains every hour.
-	e := Entry{}
+	const n = 1000
 
-	n := 1000 // number of distinct clients and domains every hour
+	for h := 0; h < 12; h++ {
-	for h := 0; h != 12; h++ {
+		atomic.AddInt32(&hour, 1)
-		if h != 0 {
+		for i := 0; i < n; i++ {
-			atomic.AddInt32(&hour, 1)
+			s.Update(Entry{
-		}
+				Domain: fmt.Sprintf("domain%d", i),
-		for i := 0; i != n; i++ {
+				Client: net.IP{
-			e.Domain = fmt.Sprintf("domain%d", i)
+					127,
-			ip := net.IP{127, 0, 0, 1}
+					0,
-			ip[2] = byte((i & 0xff00) >> 8)
+					byte((i & 0xff00) >> 8),
-			ip[3] = byte(i & 0xff)
+					byte(i & 0xff),
-			e.Client = ip.String()
+				}.String(),
-			e.Result = RNotFiltered
+				Result: RNotFiltered,
-			e.Time = 123456
+				Time:   123456,
-			s.Update(e)
+			})
 		}
 	}
 
 	d, ok := s.getData()
-	assert.True(t, ok)
+	require.True(t, ok)
-	assert.EqualValues(t, int(hour)*n, d.NumDNSQueries)
+	assert.EqualValues(t, hour*n, d.NumDNSQueries)
-
-	s.Close()
 }
 
-// this code is a chunk copied from getData() that generates aggregate data per day
+func TestStatsCollector(t *testing.T) {
-func aggregateDataPerDay(firstID uint32) int {
+	ng := func(_ *unitDB) uint64 {
-	firstDayID := (firstID + 24 - 1) / 24 * 24 // align_ceil(24)
+		return 0
-	a := []uint64{}
+	}
-	var sum uint64
+	units := make([]*unitDB, 720)
-	id := firstDayID
+
-	nextDayID := firstDayID + 24
+	t.Run("hours", func(t *testing.T) {
-	for i := firstDayID - firstID; int(i) != 720; i++ {
+		statsData := statsCollector(units, 0, Hours, ng)
-		sum++
+		assert.Len(t, statsData, 720)
-		if id == nextDayID {
+	})
-			a = append(a, sum)
+
-			sum = 0
+	t.Run("days", func(t *testing.T) {
-			nextDayID += 24
+		for i := 0; i != 25; i++ {
+			statsData := statsCollector(units, uint32(i), Days, ng)
+			require.Lenf(t, statsData, 30, "i=%d", i)
 		}
-		id++
+	})
-	}
-	if id <= nextDayID {
-		a = append(a, sum)
-	}
-	return len(a)
-}
-
-func TestAggregateDataPerTimeUnit(t *testing.T) {
-	for i := 0; i != 25; i++ {
-		alen := aggregateDataPerDay(uint32(i))
-		assert.Equalf(t, 30, alen, "i=%d", i)
-	}
 }

internal/stats/unit.go

@@ -528,6 +528,57 @@ func (s *statsCtx) loadUnits(limit uint32) ([]*unitDB, uint32) {
 	return units, firstID
 }
 
+// numsGetter is a signature for statsCollector argument.
+type numsGetter func(u *unitDB) (num uint64)
+
+// statsCollector collects statisctics for the given *unitDB slice by specified
+// timeUnit using ng to retrieve data.
+func statsCollector(units []*unitDB, firstID uint32, timeUnit TimeUnit, ng numsGetter) (nums []uint64) {
+	if timeUnit == Hours {
+		for _, u := range units {
+			nums = append(nums, ng(u))
+		}
+	} else {
+		// Per time unit counters: 720 hours may span 31 days, so we
+		// skip data for the first day in this case.
+		// align_ceil(24)
+		firstDayID := (firstID + 24 - 1) / 24 * 24
+
+		var sum uint64
+		id := firstDayID
+		nextDayID := firstDayID + 24
+		for i := int(firstDayID - firstID); i != len(units); i++ {
+			sum += ng(units[i])
+			if id == nextDayID {
+				nums = append(nums, sum)
+				sum = 0
+				nextDayID += 24
+			}
+			id++
+		}
+		if id <= nextDayID {
+			nums = append(nums, sum)
+		}
+	}
+	return nums
+}
+
+// pairsGetter is a signature for topsCollector argument.
+type pairsGetter func(u *unitDB) (pairs []countPair)
+
+// topsCollector collects statistics about highest values fro the given *unitDB
+// slice using pg to retrieve data.
+func topsCollector(units []*unitDB, max int, pg pairsGetter) []map[string]uint64 {
+	m := map[string]uint64{}
+	for _, u := range units {
+		for _, it := range pg(u) {
+			m[it.Name] += it.Count
+		}
+	}
+	a2 := convertMapToSlice(m, max)
+	return convertTopSlice(a2)
+}
+
 /* Algorithm:
 . Prepare array of N units, where N is the value of "limit" configuration setting
  . Load data for the most recent units from file
@@ -568,65 +619,25 @@ func (s *statsCtx) getData() (statsResponse, bool) {
 		return statsResponse{}, false
 	}
 
-	// per time unit counters:
+	dnsQueries := statsCollector(units, firstID, timeUnit, func(u *unitDB) (num uint64) { return u.NTotal })
-	// 720 hours may span 31 days, so we skip data for the first day in this case
-	firstDayID := (firstID + 24 - 1) / 24 * 24 // align_ceil(24)
-
-	statsCollector := func(numsGetter func(u *unitDB) (num uint64)) (nums []uint64) {
-		if timeUnit == Hours {
-			for _, u := range units {
-				nums = append(nums, numsGetter(u))
-			}
-		} else {
-			var sum uint64
-			id := firstDayID
-			nextDayID := firstDayID + 24
-			for i := int(firstDayID - firstID); i != len(units); i++ {
-				sum += numsGetter(units[i])
-				if id == nextDayID {
-					nums = append(nums, sum)
-					sum = 0
-					nextDayID += 24
-				}
-				id++
-			}
-			if id <= nextDayID {
-				nums = append(nums, sum)
-			}
-		}
-		return nums
-	}
-
-	topsCollector := func(max int, pairsGetter func(u *unitDB) (pairs []countPair)) []map[string]uint64 {
-		m := map[string]uint64{}
-		for _, u := range units {
-			for _, it := range pairsGetter(u) {
-				m[it.Name] += it.Count
-			}
-		}
-		a2 := convertMapToSlice(m, max)
-		return convertTopSlice(a2)
-	}
-
-	dnsQueries := statsCollector(func(u *unitDB) (num uint64) { return u.NTotal })
 	if timeUnit != Hours && len(dnsQueries) != int(limit/24) {
 		log.Fatalf("len(dnsQueries) != limit: %d %d", len(dnsQueries), limit)
 	}
 
 	data := statsResponse{
 		DNSQueries:           dnsQueries,
-		BlockedFiltering:     statsCollector(func(u *unitDB) (num uint64) { return u.NResult[RFiltered] }),
+		BlockedFiltering:     statsCollector(units, firstID, timeUnit, func(u *unitDB) (num uint64) { return u.NResult[RFiltered] }),
-		ReplacedSafebrowsing: statsCollector(func(u *unitDB) (num uint64) { return u.NResult[RSafeBrowsing] }),
+		ReplacedSafebrowsing: statsCollector(units, firstID, timeUnit, func(u *unitDB) (num uint64) { return u.NResult[RSafeBrowsing] }),
-		ReplacedParental:     statsCollector(func(u *unitDB) (num uint64) { return u.NResult[RParental] }),
+		ReplacedParental:     statsCollector(units, firstID, timeUnit, func(u *unitDB) (num uint64) { return u.NResult[RParental] }),
-		TopQueried:           topsCollector(maxDomains, func(u *unitDB) (pairs []countPair) { return u.Domains }),
+		TopQueried:           topsCollector(units, maxDomains, func(u *unitDB) (pairs []countPair) { return u.Domains }),
-		TopBlocked:           topsCollector(maxDomains, func(u *unitDB) (pairs []countPair) { return u.BlockedDomains }),
+		TopBlocked:           topsCollector(units, maxDomains, func(u *unitDB) (pairs []countPair) { return u.BlockedDomains }),
-		TopClients:           topsCollector(maxClients, func(u *unitDB) (pairs []countPair) { return u.Clients }),
+		TopClients:           topsCollector(units, maxClients, func(u *unitDB) (pairs []countPair) { return u.Clients }),
 	}
 
-	// total counters:
+	// Total counters:
-
+	sum := unitDB{
-	sum := unitDB{}
+		NResult: make([]uint64, rLast),
-	sum.NResult = make([]uint64, rLast)
+	}
 	timeN := 0
 	for _, u := range units {
 		sum.NTotal += u.NTotal

internal/sysutil/net_linux_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const nl = "\n"
@@ -48,7 +49,7 @@ func TestDHCPCDStaticConfig(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			r := bytes.NewReader(tc.data)
 			has, err := dhcpcdStaticConfig(r, "wlan0")
-			assert.Nil(t, err)
+			require.Nil(t, err)
 			assert.Equal(t, tc.want, has)
 		})
 	}
@@ -85,26 +86,36 @@ func TestIfacesStaticConfig(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			r := bytes.NewReader(tc.data)
 			has, err := ifacesStaticConfig(r, "enp0s3")
-			assert.Nil(t, err)
+			require.Nil(t, err)
 			assert.Equal(t, tc.want, has)
 		})
 	}
 }
 
 func TestSetStaticIPdhcpcdConf(t *testing.T) {
-	dhcpcdConf := nl + `interface wlan0` + nl +
+	testCases := []struct {
-		`static ip_address=192.168.0.2/24` + nl +
+		name       string
-		`static routers=192.168.0.1` + nl +
+		dhcpcdConf string
-		`static domain_name_servers=192.168.0.2` + nl + nl
+		routers    net.IP
+	}{{
+		name: "with_gateway",
+		dhcpcdConf: nl + `interface wlan0` + nl +
+			`static ip_address=192.168.0.2/24` + nl +
+			`static routers=192.168.0.1` + nl +
+			`static domain_name_servers=192.168.0.2` + nl + nl,
+		routers: net.IP{192, 168, 0, 1},
+	}, {
+		name: "without_gateway",
+		dhcpcdConf: nl + `interface wlan0` + nl +
+			`static ip_address=192.168.0.2/24` + nl +
+			`static domain_name_servers=192.168.0.2` + nl + nl,
+		routers: nil,
+	}}
 
-	s := updateStaticIPdhcpcdConf("wlan0", "192.168.0.2/24", net.IP{192, 168, 0, 1}, net.IP{192, 168, 0, 2})
+	for _, tc := range testCases {
-	assert.Equal(t, dhcpcdConf, s)
+		t.Run(tc.name, func(t *testing.T) {
-
+			s := updateStaticIPdhcpcdConf("wlan0", "192.168.0.2/24", tc.routers, net.IP{192, 168, 0, 2})
-	// without gateway
+			assert.Equal(t, tc.dhcpcdConf, s)
-	dhcpcdConf = nl + `interface wlan0` + nl +
+		})
-		`static ip_address=192.168.0.2/24` + nl +
+	}
-		`static domain_name_servers=192.168.0.2` + nl + nl
-
-	s = updateStaticIPdhcpcdConf("wlan0", "192.168.0.2/24", nil, net.IP{192, 168, 0, 2})
-	assert.Equal(t, dhcpcdConf, s)
 }

internal/util/autohosts_test.go

@@ -11,114 +11,162 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/aghtest"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestMain(m *testing.M) {
 	aghtest.DiscardLogOutput(m)
 }
 
-func prepareTestDir() string {
+func prepareTestFile(t *testing.T) (f *os.File) {
-	const dir = "./agh-test"
+	t.Helper()
-	_ = os.RemoveAll(dir)
+
-	_ = os.MkdirAll(dir, 0o755)
+	dir := aghtest.PrepareTestDir(t)
-	return dir
+
+	f, err := ioutil.TempFile(dir, "")
+	require.Nil(t, err)
+	require.NotNil(t, f)
+	t.Cleanup(func() {
+		assert.Nil(t, f.Close())
+	})
+
+	return f
+}
+
+func assertWriting(t *testing.T, f *os.File, strs ...string) {
+	t.Helper()
+
+	for _, str := range strs {
+		n, err := f.WriteString(str)
+		require.Nil(t, err)
+		assert.Equal(t, n, len(str))
+	}
 }
 
 func TestAutoHostsResolution(t *testing.T) {
-	ah := AutoHosts{}
+	ah := &AutoHosts{}
 
-	dir := prepareTestDir()
+	f := prepareTestFile(t)
-	defer func() { _ = os.RemoveAll(dir) }()
-
-	f, _ := ioutil.TempFile(dir, "")
-	defer func() { _ = os.Remove(f.Name()) }()
-	defer f.Close()
-
-	_, _ = f.WriteString("  127.0.0.1   host  localhost # comment \n")
-	_, _ = f.WriteString("  ::1   localhost#comment  \n")
 
+	assertWriting(t, f,
+		"  127.0.0.1   host  localhost # comment \n",
+		"  ::1   localhost#comment  \n",
+	)
 	ah.Init(f.Name())
 
-	// Existing host
+	t.Run("existing_host", func(t *testing.T) {
-	ips := ah.Process("localhost", dns.TypeA)
+		ips := ah.Process("localhost", dns.TypeA)
-	assert.NotNil(t, ips)
+		require.Len(t, ips, 1)
-	assert.Len(t, ips, 1)
+		assert.Equal(t, net.IPv4(127, 0, 0, 1), ips[0])
-	assert.Equal(t, net.ParseIP("127.0.0.1"), ips[0])
+	})
 
-	// Unknown host
+	t.Run("unknown_host", func(t *testing.T) {
-	ips = ah.Process("newhost", dns.TypeA)
+		ips := ah.Process("newhost", dns.TypeA)
-	assert.Nil(t, ips)
+		assert.Nil(t, ips)
 
-	// Unknown host (comment)
+		// Comment.
-	ips = ah.Process("comment", dns.TypeA)
+		ips = ah.Process("comment", dns.TypeA)
-	assert.Nil(t, ips)
+		assert.Nil(t, ips)
+	})
 
-	// Test hosts file
+	t.Run("hosts_file", func(t *testing.T) {
-	table := ah.List()
+		names, ok := ah.List()["127.0.0.1"]
-	names, ok := table["127.0.0.1"]
+		require.True(t, ok)
-	assert.True(t, ok)
+		assert.Equal(t, []string{"host", "localhost"}, names)
-	assert.Equal(t, []string{"host", "localhost"}, names)
+	})
 
-	// Test PTR
+	t.Run("ptr", func(t *testing.T) {
-	a, _ := dns.ReverseAddr("127.0.0.1")
+		testCases := []struct {
-	a = strings.TrimSuffix(a, ".")
+			wantIP   string
-	hosts := ah.ProcessReverse(a, dns.TypePTR)
+			wantLen  int
-	if assert.Len(t, hosts, 2) {
+			wantHost string
-		assert.Equal(t, hosts[0], "host")
+		}{
-	}
+			{wantIP: "127.0.0.1", wantLen: 2, wantHost: "host"},
+			{wantIP: "::1", wantLen: 1, wantHost: "localhost"},
+		}
 
-	a, _ = dns.ReverseAddr("::1")
+		for _, tc := range testCases {
-	a = strings.TrimSuffix(a, ".")
+			a, err := dns.ReverseAddr(tc.wantIP)
-	hosts = ah.ProcessReverse(a, dns.TypePTR)
+			require.Nil(t, err)
-	if assert.Len(t, hosts, 1) {
+
-		assert.Equal(t, hosts[0], "localhost")
+			a = strings.TrimSuffix(a, ".")
-	}
+			hosts := ah.ProcessReverse(a, dns.TypePTR)
+			require.Len(t, hosts, tc.wantLen)
+			assert.Equal(t, tc.wantHost, hosts[0])
+		}
+	})
 }
 
 func TestAutoHostsFSNotify(t *testing.T) {
-	ah := AutoHosts{}
+	ah := &AutoHosts{}
 
-	dir := prepareTestDir()
+	f := prepareTestFile(t)
-	defer func() { _ = os.RemoveAll(dir) }()
 
-	f, _ := ioutil.TempFile(dir, "")
+	assertWriting(t, f, "  127.0.0.1   host  localhost  \n")
-	defer func() { _ = os.Remove(f.Name()) }()
-	defer f.Close()
-
-	// Init
-	_, _ = f.WriteString("  127.0.0.1   host  localhost  \n")
 	ah.Init(f.Name())
 
-	// Unknown host
+	t.Run("unknown_host", func(t *testing.T) {
-	ips := ah.Process("newhost", dns.TypeA)
+		ips := ah.Process("newhost", dns.TypeA)
-	assert.Nil(t, ips)
+		assert.Nil(t, ips)
+	})
 
-	// Stat monitoring for changes
+	// Start monitoring for changes.
 	ah.Start()
-	defer ah.Close()
+	t.Cleanup(ah.Close)
 
-	// Update file
+	assertWriting(t, f, "127.0.0.2   newhost\n")
-	_, _ = f.WriteString("127.0.0.2   newhost\n")
+	require.Nil(t, f.Sync())
-	_ = f.Sync()
 
-	// wait until fsnotify has triggerred and processed the file-modification event
+	// Wait until fsnotify has triggerred and processed the
+	// file-modification event.
 	time.Sleep(50 * time.Millisecond)
 
-	// Check if we are notified about changes
+	t.Run("notified", func(t *testing.T) {
-	ips = ah.Process("newhost", dns.TypeA)
+		ips := ah.Process("newhost", dns.TypeA)
-	assert.NotNil(t, ips)
+		assert.NotNil(t, ips)
-	assert.Len(t, ips, 1)
+		require.Len(t, ips, 1)
-	assert.True(t, net.IP{127, 0, 0, 2}.Equal(ips[0]))
+		assert.True(t, net.IP{127, 0, 0, 2}.Equal(ips[0]))
+	})
 }
 
-func TestIP(t *testing.T) {
+func TestDNSReverseAddr(t *testing.T) {
-	assert.True(t, net.IP{127, 0, 0, 1}.Equal(DNSUnreverseAddr("1.0.0.127.in-addr.arpa")))
+	testCases := []struct {
-	assert.Equal(t, "::abcd:1234", DNSUnreverseAddr("4.3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa").String())
+		name string
-	assert.Equal(t, "::abcd:1234", DNSUnreverseAddr("4.3.2.1.d.c.B.A.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa").String())
+		have string
+		want net.IP
+	}{{
+		name: "good_ipv4",
+		have: "1.0.0.127.in-addr.arpa",
+		want: net.IP{127, 0, 0, 1},
+	}, {
+		name: "good_ipv6",
+		have: "4.3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa",
+		want: net.ParseIP("::abcd:1234"),
+	}, {
+		name: "good_ipv6_case",
+		have: "4.3.2.1.d.c.B.A.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa",
+		want: net.ParseIP("::abcd:1234"),
+	}, {
+		name: "bad_ipv4_dot",
+		have: "1.0.0.127.in-addr.arpa.",
+	}, {
+		name: "wrong_ipv4",
+		have: ".0.0.127.in-addr.arpa",
+	}, {
+		name: "wrong_ipv6",
+		have: ".3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa",
+	}, {
+		name: "bad_ipv6_dot",
+		have: "4.3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0..ip6.arpa",
+	}, {
+		name: "bad_ipv6_space",
+		have: "4.3.2.1.d.c.b. .0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa",
+	}}
 
-	assert.Nil(t, DNSUnreverseAddr("1.0.0.127.in-addr.arpa."))
+	for _, tc := range testCases {
-	assert.Nil(t, DNSUnreverseAddr(".0.0.127.in-addr.arpa"))
+		t.Run(tc.name, func(t *testing.T) {
-	assert.Nil(t, DNSUnreverseAddr(".3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa"))
+			ip := DNSUnreverseAddr(tc.have)
-	assert.Nil(t, DNSUnreverseAddr("4.3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0..ip6.arpa"))
+			assert.True(t, tc.want.Equal(ip))
-	assert.Nil(t, DNSUnreverseAddr("4.3.2.1.d.c.b. .0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa"))
+		})
+	}
 }

internal/util/helpers_test.go

@@ -4,12 +4,14 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSplitNext(t *testing.T) {
 	s := " a,b , c "
+
 	assert.Equal(t, "a", SplitNext(&s, ','))
 	assert.Equal(t, "b", SplitNext(&s, ','))
 	assert.Equal(t, "c", SplitNext(&s, ','))
-	assert.Empty(t, s)
+	require.Empty(t, s)
 }

internal/util/network_test.go

@@ -2,22 +2,15 @@ package util
 
 import (
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestGetValidNetInterfacesForWeb(t *testing.T) {
 	ifaces, err := GetValidNetInterfacesForWeb()
-	if err != nil {
+	require.Nilf(t, err, "Cannot get net interfaces: %s", err)
-		t.Fatalf("Cannot get net interfaces: %s", err)
+	require.NotEmpty(t, ifaces, "No net interfaces found")
-	}
-	if len(ifaces) == 0 {
-		t.Fatalf("No net interfaces found")
-	}
-
 	for _, iface := range ifaces {
-		if len(iface.Addresses) == 0 {
+		require.NotEmptyf(t, iface.Addresses, "No addresses found for %s", iface.Name)
-			t.Fatalf("No addresses found for %s", iface.Name)
-		}
-
-		t.Logf("%v", iface)
 	}
 }