Skip to content

Commit 9868dd3

Browse files
zhengkezhou1zhengkezhou1
committed
more test
Signed-off-by: zhengkezhou1 [email protected]
1 parent 7d2e901 commit 9868dd3

File tree

7 files changed

+265
-44
lines changed

7 files changed

+265
-44
lines changed

‎internal/storage/v2/clickhouse/factory.go

+8-13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package clickhouse
55

66
import (
77
"context"
8-
"errors"
98

109
"go.uber.org/zap"
1110

@@ -35,12 +34,12 @@ func newCHPool(cfg *config.Configuration, logger *zap.Logger) (client.Pool, erro
3534
return chPool, err
3635
}
3736

38-
func newClientPrerequisites(c *config.Configuration, logger *zap.Logger) error {
39-
if c.CreateSchema {
37+
func newClientPrerequisites(cfg *config.Configuration, logger *zap.Logger) error {
38+
if !cfg.CreateSchema {
4039
return nil
4140
}
4241

43-
chPool, err := newCHPool(c, logger)
42+
chPool, err := newCHPool(cfg, logger)
4443
if err != nil {
4544
return err
4645
}
@@ -53,22 +52,18 @@ func newFactory() *Factory {
5352
}
5453

5554
func NewFactory(cfg *config.Configuration, logger *zap.Logger) (*Factory, error) {
56-
var errs []error
5755
err := newClientPrerequisites(cfg, logger)
5856
if err != nil {
59-
errs = append(errs, err)
57+
return nil, err
6058
}
6159

6260
connection, err := newCHConn(cfg)
63-
if connection == nil {
64-
errs = append(errs, err)
61+
if err != nil {
62+
return nil, err
6563
}
6664
chPool, err := newCHPool(cfg, logger)
67-
if chPool == nil {
68-
errs = append(errs, err)
69-
}
70-
if errs != nil {
71-
return nil, errors.Join(errs...)
65+
if err != nil {
66+
return nil, err
7267
}
7368

7469
f := &Factory{

‎internal/storage/v2/clickhouse/factory_test.go

+38-13
Original file line numberDiff line numberDiff line change
@@ -72,27 +72,38 @@ func TestTraceFactory(t *testing.T) {
7272
require.NoError(t, f.Close())
7373
}
7474

75+
func TestNewClientPrerequisites(t *testing.T) {
76+
t.Run("should not create schema when CreateSchema is false", func(t *testing.T) {
77+
cfg := config.DefaultConfiguration()
78+
cfg.CreateSchema = false
79+
err := newClientPrerequisites(&cfg, zap.NewNop())
80+
require.NoError(t, err)
81+
})
82+
}
83+
7584
func TestPurge(t *testing.T) {
76-
t.Run("Success", func(t *testing.T) {
85+
t.Run("should succeed when Exec does not return an error", func(t *testing.T) {
7786
conn := mocks.Conn{}
7887
conn.On("Exec", mock.Anything, mock.Anything).Return(nil)
7988
f := newFactory()
8089
f.connection = &conn
8190
err := f.Purge(context.Background())
8291
require.NoError(t, err)
8392
})
84-
t.Run("Connection refused", func(t *testing.T) {
93+
94+
t.Run("should return error when connection is refused", func(t *testing.T) {
8595
conn := mocks.Conn{}
8696
conn.On("Exec", mock.Anything, mock.Anything).Return(errors.New("connection refused"))
8797
f := newFactory()
8898
f.connection = &conn
8999
err := f.Purge(context.Background())
90-
require.Error(t, err, "connection refused")
100+
require.Error(t, err)
101+
require.Contains(t, err.Error(), "connection refused")
91102
})
92103
}
93104

94105
func TestClose(t *testing.T) {
95-
t.Run("Success", func(t *testing.T) {
106+
t.Run("should succeed when both connection and pool close without error", func(t *testing.T) {
96107
conn := mocks.Conn{}
97108
conn.On("Close").Return(nil)
98109
pool := mocks.Pool{}
@@ -103,53 +114,67 @@ func TestClose(t *testing.T) {
103114
err := f.Close()
104115
require.NoError(t, err)
105116
})
106-
t.Run("Close failed", func(t *testing.T) {
117+
118+
t.Run("should return error if chPool Close fails", func(t *testing.T) {
107119
conn := mocks.Conn{}
108120
conn.On("Close").Return(nil)
109121
pool := mocks.Pool{}
110122
pool.On("Close").Return(errors.New("chPool close error"))
111123
f := newFactory()
112124
f.connection = &conn
113125
f.chPool = &pool
114-
require.Error(t, f.Close(), "chPool close error")
126+
err := f.Close()
127+
require.Error(t, err)
128+
require.Contains(t, err.Error(), "chPool close error")
129+
})
115130

131+
t.Run("should return error if clickhouse connection Close fails", func(t *testing.T) {
132+
conn := mocks.Conn{}
116133
conn.On("Close").Return(errors.New("clickhouse close error"))
117-
require.Error(t, f.Close(), "clickhouse close error")
134+
pool := mocks.Pool{}
135+
f := newFactory()
136+
f.connection = &conn
137+
f.chPool = &pool
138+
err := f.Close()
139+
require.Error(t, err)
140+
require.Contains(t, err.Error(), "clickhouse close error")
118141
})
119142
}
120143

121144
func TestCreateTraceWriter(t *testing.T) {
122-
t.Run("Success", func(t *testing.T) {
145+
t.Run("should succeed when trace writer is created successfully", func(t *testing.T) {
123146
pool := mocks.Pool{}
124147
f := newFactory()
125148
f.chPool = &pool
126149
writer, err := f.CreateTraceWriter()
127150
require.NoError(t, err)
128151
require.NotNil(t, writer)
129152
})
130-
t.Run("Can't create trace writer with nil chPool", func(t *testing.T) {
153+
154+
t.Run("should return error when chPool is nil", func(t *testing.T) {
131155
f := newFactory()
132156
f.chPool = nil
133157
writer, err := f.CreateTraceWriter()
134-
require.Error(t, err, "can't create trace writer with nil chPool")
158+
require.Error(t, err)
135159
require.Empty(t, writer)
136160
})
137161
}
138162

139163
func TestCreateTraceReader(t *testing.T) {
140-
t.Run("Success", func(t *testing.T) {
164+
t.Run("should succeed when trace reader is created successfully", func(t *testing.T) {
141165
c := mocks.Conn{}
142166
f := newFactory()
143167
f.connection = &c
144168
reader, err := f.CreateTracReader()
145169
require.NoError(t, err)
146170
require.NotNil(t, reader)
147171
})
148-
t.Run("Can't create trace reader with nil clickhouse", func(t *testing.T) {
172+
173+
t.Run("should return error when connection is nil", func(t *testing.T) {
149174
f := newFactory()
150175
f.connection = nil
151176
writer, err := f.CreateTracReader()
152-
require.Error(t, err, "can't create trace reader with nil clickhouse")
177+
require.Error(t, err)
153178
require.Nil(t, writer)
154179
})
155180
}

‎internal/storage/v2/clickhouse/schema/schema.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
//go:embed schema.tmpl
1717
var schemaFile embed.FS
1818

19+
const schema = "schema.tmpl"
20+
1921
func getQueryFileAsBytes(fileName string) ([]byte, error) {
2022
tmpl, err := template.ParseFS(schemaFile, fileName)
2123
if err != nil {
@@ -58,8 +60,8 @@ func getQueriesFromBytes(queryFile []byte) ([]string, error) {
5860
return queryStrings, nil
5961
}
6062

61-
func constructSchemaQueries() ([]string, error) {
62-
queryFile, err := getQueryFileAsBytes("schema.tmpl")
63+
func constructSchemaQueries(schema string) ([]string, error) {
64+
queryFile, err := getQueryFileAsBytes(schema)
6365
if err != nil {
6466
return nil, err
6567
}
@@ -71,13 +73,14 @@ func constructSchemaQueries() ([]string, error) {
7173
}
7274

7375
func CreateSchemaIfNotPresent(pool client.Pool) error {
74-
queries, err := constructSchemaQueries()
76+
queries, err := constructSchemaQueries(schema)
7577
if err != nil {
7678
return err
7779
}
7880

7981
for _, query := range queries {
80-
if pool.Do(context.Background(), query) != nil {
82+
err := pool.Do(context.Background(), query)
83+
if err != nil {
8184
return err
8285
}
8386
}

‎internal/storage/v2/clickhouse/schema/schema_test.go

+56-6
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,71 @@
44
package schema
55

66
import (
7+
"errors"
78
"testing"
89

910
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/mock"
12+
"github.com/stretchr/testify/require"
1013

14+
"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client/mocks"
1115
"github.com/jaegertracing/jaeger/pkg/testutils"
1216
)
1317

18+
func TestGetQueryFileAsBytes(t *testing.T) {
19+
t.Run("should return error when file not exist", func(t *testing.T) {
20+
bts, err := getQueryFileAsBytes("not_exist")
21+
require.Error(t, err)
22+
assert.Nil(t, bts)
23+
})
24+
}
25+
26+
func TestGetQueriesFromBytes(t *testing.T) {
27+
t.Run("should successful when query sgement lost ;", func(t *testing.T) {
28+
queryFile := []byte(`
29+
SELECT * FROM otel_traces;
30+
SELECT * FROM xxx
31+
`)
32+
queries, err := getQueriesFromBytes(queryFile)
33+
require.NoError(t, err)
34+
require.Equal(t, "SELECT * FROM otel_traces; ", queries[0])
35+
require.Equal(t, "SELECT * FROM xxx ", queries[1])
36+
})
37+
}
38+
1439
func TestConstructSchemaQueries(t *testing.T) {
15-
queries, err := constructSchemaQueries()
16-
if err != nil {
17-
t.Fatal(err)
18-
}
40+
t.Run("should successful when everything fine", func(t *testing.T) {
41+
queries, err := constructSchemaQueries(schema)
42+
if err != nil {
43+
t.Fatal(err)
44+
}
45+
46+
assert.Contains(t, queries[0], "CREATE DATABASE IF NOT EXISTS jaeger;")
47+
assert.Contains(t, queries[1], "CREATE TABLE IF NOT EXISTS jaeger.otel_traces")
48+
})
49+
50+
t.Run("should return error when schema file not found.", func(t *testing.T) {
51+
queries, err := constructSchemaQueries("not found")
52+
require.Error(t, err)
53+
assert.Nil(t, queries)
54+
})
55+
}
56+
57+
func TestCreateSchemaIfNotPresent(t *testing.T) {
58+
t.Run("should successful when schema is created successfully", func(t *testing.T) {
59+
pool := mocks.Pool{}
60+
pool.On("Do", mock.Anything, mock.Anything).Return(nil)
61+
err := CreateSchemaIfNotPresent(&pool)
62+
assert.NoError(t, err)
63+
})
1964

20-
assert.Contains(t, queries[0], "CREATE DATABASE IF NOT EXISTS jaeger;")
21-
assert.Contains(t, queries[1], "CREATE TABLE IF NOT EXISTS jaeger.otel_traces")
65+
t.Run("should fail when unable to connect to ClickHouse server", func(t *testing.T) {
66+
pool := mocks.Pool{}
67+
pool.On("Do", mock.Anything, mock.Anything).Return(errors.New("can't connect to clickhouse server"))
68+
err := CreateSchemaIfNotPresent(&pool)
69+
require.Error(t, err)
70+
assert.Contains(t, err.Error(), "can't connect to clickhouse server")
71+
})
2272
}
2373

2474
func TestMain(m *testing.M) {

0 commit comments

Comments
 (0)