Pull request: home: imp upgrade test

Merge in DNS/adguard-home from imp-upgrade-test to master

Updates #2639.
Updates #2646.

Squashed commit of the following:

commit f7bd8e020fa8fc285cdd6ecdf36711574f2e6e38
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Feb 9 16:38:40 2021 +0300

    home: imp test more

commit 5b64131e04d568871cf401fa3fc2c7980d69bee0
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Feb 9 16:09:22 2021 +0300

    home: imp upgrade test

internal/home/upgrade_test.go

@@ -3,183 +3,108 @@ package home
 import (
 	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestUpgrade1to2(t *testing.T) {
+// any is a convenient alias for interface{}.
-	// let's create test config for 1 schema version
+type any = interface{}
-	diskConfig := createTestDiskConfig(1)
 
-	// update config
+// object is a convenient alias for map[string]interface{}.
-	err := upgradeSchema1to2(&diskConfig)
+type object = map[string]any
-	if err != nil {
-		t.Fatalf("Can't upgrade schema version from 1 to 2")
-	}
 
-	// ensure that schema version was bumped
+func TestUpgradeSchema1to2(t *testing.T) {
-	compareSchemaVersion(t, diskConfig["schema_version"], 2)
+	diskConf := testDiskConf(1)
 
-	// old coredns entry should be removed
+	err := upgradeSchema1to2(&diskConf)
-	_, ok := diskConfig["coredns"]
+	require.Nil(t, err)
-	if ok {
-		t.Fatalf("Core DNS config was not removed after upgrade schema version from 1 to 2")
-	}
 
-	// pull out new dns config
+	require.Equal(t, diskConf["schema_version"], 2)
-	dnsMap, ok := diskConfig["dns"]
-	if !ok {
-		t.Fatalf("No DNS config after upgrade schema version from 1 to 2")
-	}
 
-	// cast dns configurations to maps and compare them
+	_, ok := diskConf["coredns"]
-	oldDNSConfig := castInterfaceToMap(t, createTestDNSConfig(1))
+	require.False(t, ok)
-	newDNSConfig := castInterfaceToMap(t, dnsMap)
+
-	compareConfigs(t, &oldDNSConfig, &newDNSConfig)
+	dnsMap, ok := diskConf["dns"]
+	require.True(t, ok)
+
+	oldDNSConf := convertToObject(t, testDNSConf(1))
+	newDNSConf := convertToObject(t, dnsMap)
+	assert.Equal(t, oldDNSConf, newDNSConf)
 
-	// exclude dns config and schema version from disk config comparison
 	oldExcludedEntries := []string{"coredns", "schema_version"}
 	newExcludedEntries := []string{"dns", "schema_version"}
-	oldDiskConfig := createTestDiskConfig(1)
+	oldDiskConf := testDiskConf(1)
-	compareConfigsWithoutEntries(t, &oldDiskConfig, &diskConfig, oldExcludedEntries, newExcludedEntries)
+	assertEqualExcept(t, oldDiskConf, diskConf, oldExcludedEntries, newExcludedEntries)
 }
 
-func TestUpgrade2to3(t *testing.T) {
+func TestUpgradeSchema2to3(t *testing.T) {
-	// let's create test config
+	diskConf := testDiskConf(2)
-	diskConfig := createTestDiskConfig(2)
 
-	// upgrade schema from 2 to 3
+	err := upgradeSchema2to3(&diskConf)
-	err := upgradeSchema2to3(&diskConfig)
+	require.Nil(t, err)
-	if err != nil {
-		t.Fatalf("Can't update schema version from 2 to 3: %s", err)
-	}
 
-	// check new schema version
+	require.Equal(t, diskConf["schema_version"], 3)
-	compareSchemaVersion(t, diskConfig["schema_version"], 3)
 
-	// pull out new dns configuration
+	dnsMap, ok := diskConf["dns"]
-	dnsMap, ok := diskConfig["dns"]
+	require.True(t, ok)
-	if !ok {
-		t.Fatalf("No dns config in new configuration")
-	}
 
-	// cast dns configuration to map
+	newDNSConf := convertToObject(t, dnsMap)
-	newDNSConfig := castInterfaceToMap(t, dnsMap)
+	bootstrapDNS := newDNSConf["bootstrap_dns"]
-
-	// check if bootstrap DNS becomes an array
-	bootstrapDNS := newDNSConfig["bootstrap_dns"]
 	switch v := bootstrapDNS.(type) {
 	case []string:
-		if len(v) != 1 {
+		require.Len(t, v, 1)
-			t.Fatalf("Wrong count of bootsrap DNS servers: %d", len(v))
+		require.Equal(t, "8.8.8.8:53", v[0])
-		}
-
-		if v[0] != "8.8.8.8:53" {
-			t.Fatalf("Bootsrap DNS server is not 8.8.8.8:53 : %s", v[0])
-		}
 	default:
-		t.Fatalf("Wrong type for bootsrap DNS: %T", v)
+		t.Fatalf("wrong type for bootsrap dns: %T", v)
 	}
 
-	// exclude bootstrap DNS from DNS configs comparison
 	excludedEntries := []string{"bootstrap_dns"}
-	oldDNSConfig := castInterfaceToMap(t, createTestDNSConfig(2))
+	oldDNSConf := convertToObject(t, testDNSConf(2))
-	compareConfigsWithoutEntries(t, &oldDNSConfig, &newDNSConfig, excludedEntries, excludedEntries)
+	assertEqualExcept(t, oldDNSConf, newDNSConf, excludedEntries, excludedEntries)
 
-	// excluded dns config and schema version from disk config comparison
 	excludedEntries = []string{"dns", "schema_version"}
-	oldDiskConfig := createTestDiskConfig(2)
+	oldDiskConf := testDiskConf(2)
-	compareConfigsWithoutEntries(t, &oldDiskConfig, &diskConfig, excludedEntries, excludedEntries)
+	assertEqualExcept(t, oldDiskConf, diskConf, excludedEntries, excludedEntries)
 }
 
-func castInterfaceToMap(t *testing.T, oldConfig interface{}) (newConfig map[string]interface{}) {
+func convertToObject(t *testing.T, oldConf any) (newConf object) {
-	newConfig = make(map[string]interface{})
+	t.Helper()
-	switch v := oldConfig.(type) {
+
-	case map[interface{}]interface{}:
+	switch v := oldConf.(type) {
+	case map[any]any:
+		newConf = make(object, len(v))
 		for key, value := range v {
-			newConfig[fmt.Sprint(key)] = value
+			newConf[fmt.Sprint(key)] = value
 		}
-	case map[string]interface{}:
+	case object:
+		newConf = make(object, len(v))
 		for key, value := range v {
-			newConfig[key] = value
+			newConf[key] = value
 		}
 	default:
-		t.Fatalf("DNS configuration is not a map")
+		t.Fatalf("dns configuration is not a map, got %T", oldConf)
 	}
-	return
+
+	return newConf
 }
 
-// compareConfigsWithoutEntry removes entries from configs and returns result of compareConfigs
+// assertEqualExcept removes entries from configs and compares them.
-func compareConfigsWithoutEntries(t *testing.T, oldConfig, newConfig *map[string]interface{}, oldKey, newKey []string) {
+func assertEqualExcept(t *testing.T, oldConf, newConf object, oldKeys, newKeys []string) {
-	for _, k := range oldKey {
+	t.Helper()
-		delete(*oldConfig, k)
+
+	for _, k := range oldKeys {
+		delete(oldConf, k)
 	}
-	for _, k := range newKey {
+	for _, k := range newKeys {
-		delete(*newConfig, k)
+		delete(newConf, k)
 	}
-	compareConfigs(t, oldConfig, newConfig)
+
+	assert.Equal(t, oldConf, newConf)
 }
 
-// compares configs before and after schema upgrade
+func testDiskConf(schemaVersion int) (diskConf object) {
-func compareConfigs(t *testing.T, oldConfig, newConfig *map[string]interface{}) {
+	filters := []filter{
-	if len(*oldConfig) != len(*newConfig) {
-		t.Fatalf("wrong config entries count! Before upgrade: %d; After upgrade: %d", len(*oldConfig), len(*oldConfig))
-	}
-
-	// Check old and new entries
-	for k, v := range *newConfig {
-		switch value := v.(type) {
-		case string:
-			if value != (*oldConfig)[k] {
-				t.Fatalf("wrong value for string %s. Before update: %s; After update: %s", k, (*oldConfig)[k], value)
-			}
-		case int:
-			if value != (*oldConfig)[k] {
-				t.Fatalf("wrong value for int %s. Before update: %d; After update: %d", k, (*oldConfig)[k], value)
-			}
-		case []string:
-			for i, line := range value {
-				if len((*oldConfig)[k].([]string)) != len(value) {
-					t.Fatalf("wrong array length for %s. Before update: %d; After update: %d", k, len((*oldConfig)[k].([]string)), len(value))
-				}
-				if (*oldConfig)[k].([]string)[i] != line {
-					t.Fatalf("wrong data for string array %s. Before update: %s; After update: %s", k, (*oldConfig)[k].([]string)[i], line)
-				}
-			}
-		case bool:
-			if v != (*oldConfig)[k].(bool) {
-				t.Fatalf("wrong boolean value for %s", k)
-			}
-		case []filter:
-			if len((*oldConfig)[k].([]filter)) != len(value) {
-				t.Fatalf("wrong filters count. Before update: %d; After update: %d", len((*oldConfig)[k].([]filter)), len(value))
-			}
-			for i, newFilter := range value {
-				oldFilter := (*oldConfig)[k].([]filter)[i]
-				if oldFilter.Enabled != newFilter.Enabled || oldFilter.Name != newFilter.Name || oldFilter.RulesCount != newFilter.RulesCount {
-					t.Fatalf("old filter %s not equals new filter %s", oldFilter.Name, newFilter.Name)
-				}
-			}
-		default:
-			t.Fatalf("uknown data type for %s: %T", k, value)
-		}
-	}
-}
-
-// compareSchemaVersion check if newSchemaVersion equals schemaVersion
-func compareSchemaVersion(t *testing.T, newSchemaVersion interface{}, schemaVersion int) {
-	switch v := newSchemaVersion.(type) {
-	case int:
-		if v != schemaVersion {
-			t.Fatalf("Wrong schema version in new config file")
-		}
-	default:
-		t.Fatalf("Schema version is not an integer after update")
-	}
-}
-
-func createTestDiskConfig(schemaVersion int) (diskConfig map[string]interface{}) {
-	diskConfig = make(map[string]interface{})
-	diskConfig["language"] = "en"
-	diskConfig["filters"] = []filter{
 		{
 			URL:        "https://filters.adtidy.org/android/filters/111_optimized.txt",
 			Name:       "Latvian filter",
@@ -191,40 +116,51 @@ func createTestDiskConfig(schemaVersion int) (diskConfig map[string]interface{})
 			RulesCount: 200,
 		},
 	}
-	diskConfig["user_rules"] = []string{}
+	diskConf = object{
-	diskConfig["schema_version"] = schemaVersion
+		"language":       "en",
-	diskConfig["bind_host"] = "0.0.0.0"
+		"filters":        filters,
-	diskConfig["bind_port"] = 80
+		"user_rules":     []string{},
-	diskConfig["auth_name"] = "name"
+		"schema_version": schemaVersion,
-	diskConfig["auth_pass"] = "pass"
+		"bind_host":      "0.0.0.0",
-	dnsConfig := createTestDNSConfig(schemaVersion)
+		"bind_port":      80,
-	if schemaVersion > 1 {
+		"auth_name":      "name",
-		diskConfig["dns"] = dnsConfig
+		"auth_pass":      "pass",
-	} else {
-		diskConfig["coredns"] = dnsConfig
 	}
-	return diskConfig
+
+	dnsConf := testDNSConf(schemaVersion)
+	if schemaVersion > 1 {
+		diskConf["dns"] = dnsConf
+	} else {
+		diskConf["coredns"] = dnsConf
+	}
+
+	return diskConf
 }
 
-func createTestDNSConfig(schemaVersion int) map[interface{}]interface{} {
+// testDNSConf creates a DNS config for test the way gopkg.in/yaml.v2 would
-	dnsConfig := make(map[interface{}]interface{})
+// unmarshal it.  In YAML, keys aren't guaranteed to always only be strings.
-	dnsConfig["port"] = 53
+func testDNSConf(schemaVersion int) (dnsConf map[any]any) {
-	dnsConfig["blocked_response_ttl"] = 10
+	dnsConf = map[any]any{
-	dnsConfig["querylog_enabled"] = true
+		"port":                 53,
-	dnsConfig["ratelimit"] = 20
+		"blocked_response_ttl": 10,
-	dnsConfig["bootstrap_dns"] = "8.8.8.8:53"
+		"querylog_enabled":     true,
-	if schemaVersion > 2 {
+		"ratelimit":            20,
-		dnsConfig["bootstrap_dns"] = []string{"8.8.8.8:53"}
+		"bootstrap_dns":        "8.8.8.8:53",
+		"parental_sensitivity": 13,
+		"ratelimit_whitelist":  []string{},
+		"upstream_dns":         []string{"tls://1.1.1.1", "tls://1.0.0.1", "8.8.8.8"},
+		"filtering_enabled":    true,
+		"refuse_any":           true,
+		"parental_enabled":     true,
+		"bind_host":            "0.0.0.0",
+		"protection_enabled":   true,
+		"safesearch_enabled":   true,
+		"safebrowsing_enabled": true,
 	}
-	dnsConfig["parental_sensitivity"] = 13
+
-	dnsConfig["ratelimit_whitelist"] = []string{}
+	if schemaVersion > 2 {
-	dnsConfig["upstream_dns"] = []string{"tls://1.1.1.1", "tls://1.0.0.1", "8.8.8.8"}
+		dnsConf["bootstrap_dns"] = []string{"8.8.8.8:53"}
-	dnsConfig["filtering_enabled"] = true
+	}
-	dnsConfig["refuse_any"] = true
+
-	dnsConfig["parental_enabled"] = true
+	return dnsConf
-	dnsConfig["bind_host"] = "0.0.0.0"
-	dnsConfig["protection_enabled"] = true
-	dnsConfig["safesearch_enabled"] = true
-	dnsConfig["safebrowsing_enabled"] = true
-	return dnsConfig
 }