Skip to content

Commit 5c9a577

Browse files
ci: simplify diffMap map reads and fix diffStrSlice edge case
- diffMap: use single map lookup with ok idiom instead of reading twice - diffStrSlice: compare element-wise instead of joining with comma, so ["a,b"] and ["a", "b"] are correctly reported as different Signed-off-by: huanghaoyuanhhy <haoyuan.huang@zilliz.com>
1 parent 193bf3a commit 5c9a577

2 files changed

Lines changed: 25 additions & 11 deletions

File tree

cmd/etcd-schema-verify/diff.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package main
33
import (
44
"fmt"
55
"sort"
6-
"strings"
76
)
87

98
func diffCollections(src, dst *CollectionDump) []Diff {
@@ -226,13 +225,10 @@ func diffMap(diffs *[]Diff, prefix string, src, dst map[string]string) {
226225
sort.Strings(sorted)
227226

228227
for _, k := range sorted {
229-
sv := src[k]
230-
dv := dst[k]
228+
sv, inSrc := src[k]
229+
dv, inDst := dst[k]
231230
path := prefix + "." + k
232231

233-
_, inSrc := src[k]
234-
_, inDst := dst[k]
235-
236232
switch {
237233
case !inSrc:
238234
*diffs = append(*diffs, Diff{Path: path, Src: "<missing>", Dst: dv})
@@ -245,10 +241,15 @@ func diffMap(diffs *[]Diff, prefix string, src, dst map[string]string) {
245241
}
246242

247243
func diffStrSlice(diffs *[]Diff, path string, src, dst []string) {
248-
s := strings.Join(src, ",")
249-
d := strings.Join(dst, ",")
250-
if s != d {
251-
*diffs = append(*diffs, Diff{Path: path, Src: fmt.Sprintf("[%s]", s), Dst: fmt.Sprintf("[%s]", d)})
244+
if len(src) != len(dst) {
245+
*diffs = append(*diffs, Diff{Path: path, Src: fmt.Sprintf("%v", src), Dst: fmt.Sprintf("%v", dst)})
246+
return
247+
}
248+
for i := range src {
249+
if src[i] != dst[i] {
250+
*diffs = append(*diffs, Diff{Path: path, Src: fmt.Sprintf("%v", src), Dst: fmt.Sprintf("%v", dst)})
251+
return
252+
}
252253
}
253254
}
254255

cmd/etcd-schema-verify/diff_test.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,25 @@ func TestDiffStrSlice(t *testing.T) {
330330
assert.Empty(t, diffs)
331331
})
332332

333-
t.Run("Different", func(t *testing.T) {
333+
t.Run("DifferentLength", func(t *testing.T) {
334334
var diffs []Diff
335335
diffStrSlice(&diffs, "f", []string{"a"}, []string{"a", "b"})
336336
assert.Len(t, diffs, 1)
337337
})
338338

339+
t.Run("DifferentValues", func(t *testing.T) {
340+
var diffs []Diff
341+
diffStrSlice(&diffs, "f", []string{"a", "b"}, []string{"a", "c"})
342+
assert.Len(t, diffs, 1)
343+
})
344+
345+
t.Run("CommaEdgeCase", func(t *testing.T) {
346+
// ["a,b"] and ["a", "b"] must be treated as different
347+
var diffs []Diff
348+
diffStrSlice(&diffs, "f", []string{"a,b"}, []string{"a", "b"})
349+
assert.Len(t, diffs, 1)
350+
})
351+
339352
t.Run("BothNil", func(t *testing.T) {
340353
var diffs []Diff
341354
diffStrSlice(&diffs, "f", nil, nil)

0 commit comments

Comments
 (0)