Skip to content

Commit 3dd86b4

Browse files
kschochekschoche
committed
add unit tests and update failing test
1 parent f01aefc commit 3dd86b4

File tree

3 files changed

+275
-8
lines changed

3 files changed

+275
-8
lines changed

‎pkg/apis/clickhouse-keeper.altinity.com/v1/type_cluster_test.go‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
package v1
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
apiChi "github.com/altinity/clickhouse-operator/pkg/apis/clickhouse.altinity.com/v1"
9+
)
10+
11+
// Test_KeeperClusterSettingsSource_PreferReplicaOverShard tests that when both
12+
// shard and replica settings are explicitly specified, replica settings
13+
// take precedence as they are more fine-grained control points.
14+
func Test_KeeperClusterSettingsSource_PreferReplicaOverShard(t *testing.T) {
15+
testCases := []struct {
16+
name string
17+
shardsExplicit bool
18+
replicasExplicit bool
19+
expectShardAsSource bool
20+
description string
21+
}{
22+
{
23+
name: "neither_shards_nor_replicas_explicit",
24+
shardsExplicit: false,
25+
replicasExplicit: false,
26+
expectShardAsSource: true,
27+
description: "When neither shards nor replicas are explicitly specified, use shard as settings source",
28+
},
29+
{
30+
name: "only_shards_explicit",
31+
shardsExplicit: true,
32+
replicasExplicit: false,
33+
expectShardAsSource: true,
34+
description: "When only shards are explicitly specified, use shard as settings source",
35+
},
36+
{
37+
name: "only_replicas_explicit",
38+
shardsExplicit: false,
39+
replicasExplicit: true,
40+
expectShardAsSource: false,
41+
description: "When only replicas are explicitly specified, use replica as settings source",
42+
},
43+
{
44+
name: "both_shards_and_replicas_explicit",
45+
shardsExplicit: true,
46+
replicasExplicit: true,
47+
expectShardAsSource: false,
48+
description: "When both shards and replicas are explicitly specified, prefer replica settings as they are more fine-grained",
49+
},
50+
}
51+
52+
for _, tc := range testCases {
53+
t.Run(tc.name, func(tt *testing.T) {
54+
cluster := &Cluster{
55+
Layout: &ChkClusterLayout{
56+
ShardsExplicitlySpecified: tc.shardsExplicit,
57+
ReplicasExplicitlySpecified: tc.replicasExplicit,
58+
},
59+
}
60+
61+
// Test isShardToBeUsedToInheritSettingsFrom
62+
actualShardSource := cluster.isShardToBeUsedToInheritSettingsFrom()
63+
require.Equal(tt, tc.expectShardAsSource, actualShardSource,
64+
"%s: isShardToBeUsedToInheritSettingsFrom() returned %v, expected %v",
65+
tc.description, actualShardSource, tc.expectShardAsSource)
66+
67+
// Test SelectSettingsSourceFrom
68+
shard := &apiChi.ChiShard{Name: "test-shard"}
69+
replica := &apiChi.ChiReplica{Name: "test-replica"}
70+
71+
src := cluster.SelectSettingsSourceFrom(shard, replica)
72+
if tc.expectShardAsSource {
73+
require.Equal(tt, shard, src,
74+
"%s: SelectSettingsSourceFrom() should return shard", tc.description)
75+
} else {
76+
require.Equal(tt, replica, src,
77+
"%s: SelectSettingsSourceFrom() should return replica", tc.description)
78+
}
79+
})
80+
}
81+
}
82+
83+
// Test_KeeperClusterReplicaExplicitlySpecified tests that isReplicaExplicitlySpecified
84+
// returns true whenever replicas are explicitly specified, regardless of shard specification
85+
func Test_KeeperClusterReplicaExplicitlySpecified(t *testing.T) {
86+
testCases := []struct {
87+
name string
88+
shardsExplicit bool
89+
replicasExplicit bool
90+
expected bool
91+
}{
92+
{
93+
name: "replicas_not_explicit",
94+
shardsExplicit: false,
95+
replicasExplicit: false,
96+
expected: false,
97+
},
98+
{
99+
name: "replicas_explicit_shards_not",
100+
shardsExplicit: false,
101+
replicasExplicit: true,
102+
expected: true,
103+
},
104+
{
105+
name: "replicas_explicit_shards_explicit",
106+
shardsExplicit: true,
107+
replicasExplicit: true,
108+
expected: true,
109+
},
110+
{
111+
name: "replicas_not_explicit_shards_explicit",
112+
shardsExplicit: true,
113+
replicasExplicit: false,
114+
expected: false,
115+
},
116+
}
117+
118+
for _, tc := range testCases {
119+
t.Run(tc.name, func(tt *testing.T) {
120+
cluster := &Cluster{
121+
Layout: &ChkClusterLayout{
122+
ShardsExplicitlySpecified: tc.shardsExplicit,
123+
ReplicasExplicitlySpecified: tc.replicasExplicit,
124+
},
125+
}
126+
127+
actual := cluster.isReplicaExplicitlySpecified()
128+
require.Equal(tt, tc.expected, actual,
129+
"isReplicaExplicitlySpecified() returned %v, expected %v",
130+
actual, tc.expected)
131+
})
132+
}
133+
}

‎pkg/apis/clickhouse.altinity.com/v1/type_cluster_test.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package v1
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
// Test_ClusterSettingsSource_PreferReplicaOverShard tests that when both
10+
// shard and replica settings are explicitly specified, replica settings
11+
// take precedence as they are more fine-grained control points.
12+
func Test_ClusterSettingsSource_PreferReplicaOverShard(t *testing.T) {
13+
testCases := []struct {
14+
name string
15+
shardsExplicit bool
16+
replicasExplicit bool
17+
expectShardAsSource bool
18+
description string
19+
}{
20+
{
21+
name: "neither_shards_nor_replicas_explicit",
22+
shardsExplicit: false,
23+
replicasExplicit: false,
24+
expectShardAsSource: true,
25+
description: "When neither shards nor replicas are explicitly specified, use shard as settings source",
26+
},
27+
{
28+
name: "only_shards_explicit",
29+
shardsExplicit: true,
30+
replicasExplicit: false,
31+
expectShardAsSource: true,
32+
description: "When only shards are explicitly specified, use shard as settings source",
33+
},
34+
{
35+
name: "only_replicas_explicit",
36+
shardsExplicit: false,
37+
replicasExplicit: true,
38+
expectShardAsSource: false,
39+
description: "When only replicas are explicitly specified, use replica as settings source",
40+
},
41+
{
42+
name: "both_shards_and_replicas_explicit",
43+
shardsExplicit: true,
44+
replicasExplicit: true,
45+
expectShardAsSource: false,
46+
description: "When both shards and replicas are explicitly specified, prefer replica settings as they are more fine-grained",
47+
},
48+
}
49+
50+
for _, tc := range testCases {
51+
t.Run(tc.name, func(tt *testing.T) {
52+
cluster := &Cluster{
53+
Layout: &ChiClusterLayout{
54+
ShardsExplicitlySpecified: tc.shardsExplicit,
55+
ReplicasExplicitlySpecified: tc.replicasExplicit,
56+
},
57+
}
58+
59+
// Test isShardToBeUsedToInheritSettingsFrom
60+
actualShardSource := cluster.isShardToBeUsedToInheritSettingsFrom()
61+
require.Equal(tt, tc.expectShardAsSource, actualShardSource,
62+
"%s: isShardToBeUsedToInheritSettingsFrom() returned %v, expected %v",
63+
tc.description, actualShardSource, tc.expectShardAsSource)
64+
65+
// Test SelectSettingsSourceFrom
66+
shard := &ChiShard{Name: "test-shard"}
67+
replica := &ChiReplica{Name: "test-replica"}
68+
69+
src := cluster.SelectSettingsSourceFrom(shard, replica)
70+
if tc.expectShardAsSource {
71+
require.Equal(tt, shard, src,
72+
"%s: SelectSettingsSourceFrom() should return shard", tc.description)
73+
} else {
74+
require.Equal(tt, replica, src,
75+
"%s: SelectSettingsSourceFrom() should return replica", tc.description)
76+
}
77+
})
78+
}
79+
}
80+
81+
// Test_ClusterReplicaExplicitlySpecified tests that isReplicaExplicitlySpecified
82+
// returns true whenever replicas are explicitly specified, regardless of shard specification
83+
func Test_ClusterReplicaExplicitlySpecified(t *testing.T) {
84+
testCases := []struct {
85+
name string
86+
shardsExplicit bool
87+
replicasExplicit bool
88+
expected bool
89+
}{
90+
{
91+
name: "replicas_not_explicit",
92+
shardsExplicit: false,
93+
replicasExplicit: false,
94+
expected: false,
95+
},
96+
{
97+
name: "replicas_explicit_shards_not",
98+
shardsExplicit: false,
99+
replicasExplicit: true,
100+
expected: true,
101+
},
102+
{
103+
name: "replicas_explicit_shards_explicit",
104+
shardsExplicit: true,
105+
replicasExplicit: true,
106+
expected: true,
107+
},
108+
{
109+
name: "replicas_not_explicit_shards_explicit",
110+
shardsExplicit: true,
111+
replicasExplicit: false,
112+
expected: false,
113+
},
114+
}
115+
116+
for _, tc := range testCases {
117+
t.Run(tc.name, func(tt *testing.T) {
118+
cluster := &Cluster{
119+
Layout: &ChiClusterLayout{
120+
ShardsExplicitlySpecified: tc.shardsExplicit,
121+
ReplicasExplicitlySpecified: tc.replicasExplicit,
122+
},
123+
}
124+
125+
actual := cluster.isReplicaExplicitlySpecified()
126+
require.Equal(tt, tc.expected, actual,
127+
"isReplicaExplicitlySpecified() returned %v, expected %v",
128+
actual, tc.expected)
129+
})
130+
}
131+
}

‎pkg/apis/clickhouse.altinity.com/v1/type_status_test.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"github.com/stretchr/testify/require"
66
"sync"
77
"testing"
8+
9+
"github.com/altinity/clickhouse-operator/pkg/apis/common/types"
810
)
911

1012
var normalizedChiA = &ClickHouseInstallation{}
@@ -158,28 +160,29 @@ func Test_ChiStatus_BasicOperations_SingleStatus_ConcurrencyTest(t *testing.T) {
158160
name: "CopyFrom",
159161
goRoutineA: func(s *Status) {
160162
s.PushAction("always-present-action") // CopyFrom preserves existing actions (does not clobber)
161-
s.CopyFrom(copyTestStatusFrom, CopyStatusOptions{
162-
Actions: true,
163-
Errors: true,
164-
MainFields: true,
165-
WholeStatus: true,
166-
InheritableFields: true,
163+
s.CopyFrom(copyTestStatusFrom, types.CopyStatusOptions{
164+
CopyStatusFieldGroup: types.CopyStatusFieldGroup{
165+
FieldGroupActions: true,
166+
FieldGroupErrors: true,
167+
FieldGroupMain: true,
168+
FieldGroupWholeStatus: true,
169+
FieldGroupInheritable: true,
170+
},
167171
})
168172
},
169173
goRoutineB: func(s *Status) {
170174
s.PushAction("additional-action") // this may or may not win the race, but the race will be sync
171175
},
172176
postConditionsVerification: func(tt *testing.T, s *Status) {
173177
if len(s.GetActions()) == len(copyTestStatusFrom.GetActions())+2 {
174-
require.Equal(tt, copyTestStatusFrom.GetActions(), s.GetActions())
175178
require.Contains(tt, s.GetActions(), "always-present-action")
176179
require.Contains(tt, s.GetActions(), "additional-action")
177180
for _, action := range copyTestStatusFrom.GetActions() {
178181
require.Contains(tt, s.GetActions(), action)
179182
}
180183
} else {
181184
require.Equal(tt, len(copyTestStatusFrom.GetActions())+1, len(s.GetActions()))
182-
require.Contains(tt, s.GetActions(), "additional-action")
185+
require.Contains(tt, s.GetActions(), "always-present-action")
183186
for _, action := range copyTestStatusFrom.GetActions() {
184187
require.Contains(tt, s.GetActions(), action)
185188
}

0 commit comments

Comments
 (0)