diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2020-11-17 14:26:48 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2020-11-17 14:29:07 +0300 |
commit | c2c82c07b445f9fc2913125a59e7ca8b0fb72e28 (patch) | |
tree | 6fe082142541b7ad728c0d70cd1b0d2674470022 | |
parent | b8a027e4ea96235545000da5ff35d05a91b4370b (diff) |
reduce duplication in praefect config validation tests
Praefect's config validation tests duplicate the meaning of a valid
config. This commit removes that duplication by letting each test
case make changes on a valid config instead of defining a fully
valid config each time.
-rw-r--r-- | internal/praefect/config/config_test.go | 161 |
1 files changed, 57 insertions, 104 deletions
diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index fb65e80f7..862dea798 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -27,113 +27,73 @@ func TestConfigValidation(t *testing.T) { {Storage: "internal-3.1", Address: "localhost:33458", Token: "secret-token-2"}, } - fromValidConfig := func(changeFunc func(*Config)) Config { - cfg := Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ - {Name: "default", Nodes: vs1Nodes}, - {Name: "secondary", Nodes: vs2Nodes}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, - } - - changeFunc(&cfg) - - return cfg - } - testCases := []struct { - desc string - config Config - errMsg string + desc string + changeConfig func(*Config) + errMsg string }{ { - desc: "Valid config with ListenAddr", - config: fromValidConfig(func(*Config) {}), + desc: "Valid config with ListenAddr", + changeConfig: func(*Config) {}, }, { desc: "Valid config with local elector", - config: fromValidConfig(func(cfg *Config) { + changeConfig: func(cfg *Config) { cfg.Failover.ElectionStrategy = ElectionStrategyLocal - }), + }, }, { desc: "Valid config with per repository elector", - config: fromValidConfig(func(cfg *Config) { + changeConfig: func(cfg *Config) { cfg.Failover.ElectionStrategy = ElectionStrategyPerRepository - }), + }, }, { desc: "Invalid election strategy", - config: fromValidConfig(func(cfg *Config) { + changeConfig: func(cfg *Config) { cfg.Failover.ElectionStrategy = "invalid-strategy" - }), + }, errMsg: `invalid election strategy: "invalid-strategy"`, }, { desc: "Valid config with TLSListenAddr", - config: Config{ - TLSListenAddr: "tls://localhost:4321", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ - {Name: "default", Nodes: vs1Nodes}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + changeConfig: func(cfg *Config) { + cfg.ListenAddr = "" + cfg.TLSListenAddr = "tls://localhost:4321" }, }, { desc: "Valid config with SocketPath", - config: Config{ - SocketPath: "/tmp/praefect.socket", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ - {Name: "default", Nodes: vs1Nodes}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + changeConfig: func(cfg *Config) { + cfg.ListenAddr = "" + cfg.SocketPath = "/tmp/praefect.socket" }, }, { desc: "Invalid replication batch size", - config: Config{ - SocketPath: "/tmp/praefect.socket", - Replication: Replication{BatchSize: 0}, - VirtualStorages: []*VirtualStorage{ - {Name: "default", Nodes: vs1Nodes}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + changeConfig: func(cfg *Config) { + cfg.Replication = Replication{BatchSize: 0} }, errMsg: "replication batch size was 0 but must be >=1", }, { desc: "No ListenAddr or SocketPath or TLSListenAddr", - config: Config{ - ListenAddr: "", - TLSListenAddr: "", - SocketPath: "", - VirtualStorages: []*VirtualStorage{ - {Name: "default", Nodes: vs1Nodes}, - }, - Replication: DefaultReplicationConfig(), - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + changeConfig: func(cfg *Config) { + cfg.ListenAddr = "" }, errMsg: "no listen address or socket path configured", }, { desc: "No virtual storages", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = nil }, errMsg: "no virtual storages configured", }, { desc: "duplicate storage", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ { Name: "default", Nodes: append(vs1Nodes, &Node{ @@ -141,17 +101,14 @@ func TestConfigValidation(t *testing.T) { Address: vs1Nodes[1].Address, }), }, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `virtual storage "default": internal gitaly storages are not unique`, }, { desc: "Node storage has no name", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ { Name: "default", Nodes: []*Node{ @@ -162,17 +119,14 @@ func TestConfigValidation(t *testing.T) { }, }, }, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `virtual storage "default": all gitaly nodes must have a storage`, }, { desc: "Node storage has no address", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ { Name: "default", Nodes: []*Node{ @@ -183,59 +137,46 @@ func TestConfigValidation(t *testing.T) { }, }, }, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `virtual storage "default": all gitaly nodes must have an address`, }, { desc: "Virtual storage has no name", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ {Name: "", Nodes: vs1Nodes}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `virtual storages must have a name`, }, { desc: "Virtual storage not unique", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ {Name: "default", Nodes: vs1Nodes}, {Name: "default", Nodes: vs2Nodes}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `virtual storage "default": virtual storages must have unique names`, }, { desc: "Virtual storage has no nodes", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ {Name: "default", Nodes: vs1Nodes}, {Name: "secondary", Nodes: nil}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `virtual storage "secondary": no primary gitaly backends configured`, }, { desc: "Node storage has address duplicate", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ + changeConfig: func(cfg *Config) { + cfg.VirtualStorages = []*VirtualStorage{ {Name: "default", Nodes: vs1Nodes}, {Name: "secondary", Nodes: append(vs2Nodes, vs1Nodes[1])}, - }, - Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } }, errMsg: `multiple storages have the same address`, }, @@ -243,7 +184,19 @@ func TestConfigValidation(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - err := tc.config.Validate() + config := Config{ + ListenAddr: "localhost:1234", + Replication: DefaultReplicationConfig(), + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + {Name: "secondary", Nodes: vs2Nodes}, + }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } + + tc.changeConfig(&config) + + err := config.Validate() if tc.errMsg == "" { require.NoError(t, err) return |