Skip to content

Commit 6072760

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #709 from joelanford/declcfg-improve-tests
declcfg: improve and fix model tests to eliminate possibility of false negatives
2 parents 73d60bf + 95c5e3b commit 6072760

File tree

2 files changed

+68
-65
lines changed

2 files changed

+68
-65
lines changed

‎internal/model/model.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,6 @@ type RelatedImage struct {
269269

270270
func (i RelatedImage) Validate() error {
271271
result := newValidationError("invalid related image")
272-
if i.Name == "" {
273-
result.subErrors = append(result.subErrors, fmt.Errorf("name must be set"))
274-
}
275272
if i.Image == "" {
276273
result.subErrors = append(result.subErrors, fmt.Errorf("image must be set"))
277274
}

‎internal/model/model_test.go‎

Lines changed: 68 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestChannelHead(t *testing.T) {
9797
"anakin.v0.0.1": {Name: "anakin.v0.0.1", Replaces: "anakin.v0.0.3"},
9898
"anakin.v0.0.3": head,
9999
}},
100-
assertion: require.Error,
100+
assertion: hasError(`no channel head found in graph`),
101101
},
102102
{
103103
name: "Error/MultipleChannelHeads",
@@ -106,7 +106,7 @@ func TestChannelHead(t *testing.T) {
106106
"anakin.v0.0.3": head,
107107
"anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.1"},
108108
}},
109-
assertion: require.Error,
109+
assertion: hasError(`multiple channel heads found in graph: anakin.v0.0.3, anakin.v0.0.4`),
110110
},
111111
}
112112
for _, s := range specs {
@@ -118,6 +118,32 @@ func TestChannelHead(t *testing.T) {
118118
}
119119
}
120120

121+
func hasError(expectedError string) require.ErrorAssertionFunc {
122+
return func(t require.TestingT, actualError error, args ...interface{}) {
123+
if stdt, ok := t.(*testing.T); ok {
124+
stdt.Helper()
125+
}
126+
errsToCheck := []error{actualError}
127+
for len(errsToCheck) > 0 {
128+
var err error
129+
err, errsToCheck = errsToCheck[0], errsToCheck[1:]
130+
if err == nil {
131+
continue
132+
}
133+
if verr, ok := err.(*validationError); ok {
134+
if verr.message == expectedError {
135+
return
136+
}
137+
errsToCheck = append(errsToCheck, verr.subErrors...)
138+
} else if expectedError == err.Error() {
139+
return
140+
}
141+
}
142+
t.Errorf("expected error to be or contain suberror %q, got %v", expectedError, actualError)
143+
t.FailNow()
144+
}
145+
}
146+
121147
func TestValidators(t *testing.T) {
122148
type spec struct {
123149
name string
@@ -144,14 +170,14 @@ func TestValidators(t *testing.T) {
144170
v: Model{
145171
"foo": pkg,
146172
},
147-
assertion: require.Error,
173+
assertion: hasError(`package key "foo" does not match package name "anakin"`),
148174
},
149175
{
150176
name: "Model/Error/InvalidPackage",
151177
v: Model{
152178
pkgIncorrectDefaultChannel.Name: pkgIncorrectDefaultChannel,
153179
},
154-
assertion: require.Error,
180+
assertion: hasError(`invalid package "anakin"`),
155181
},
156182
{
157183
name: "Package/Success/Valid",
@@ -161,23 +187,23 @@ func TestValidators(t *testing.T) {
161187
{
162188
name: "Package/Error/NoName",
163189
v: &Package{},
164-
assertion: require.Error,
165-
},
166-
{
167-
name: "Package/Error/InvalidIcon",
168-
v: &Package{
169-
Name: "anakin",
170-
Icon: &Icon{Data: mustBase64Decode(svgData)},
171-
},
172-
assertion: require.Error,
190+
assertion: hasError("package name must not be empty"),
173191
},
192+
//{
193+
// name: "Package/Error/InvalidIcon",
194+
// v: &Package{
195+
// Name: "anakin",
196+
// Icon: &Icon{Data: mustBase64Decode(svgData)},
197+
// },
198+
// assertion: hasError("icon mediatype must be set if icon is defined"),
199+
//},
174200
{
175201
name: "Package/Error/NoChannels",
176202
v: &Package{
177203
Name: "anakin",
178204
Icon: &Icon{Data: mustBase64Decode(svgData), MediaType: "image/svg+xml"},
179205
},
180-
assertion: require.Error,
206+
assertion: hasError("package must contain at least one channel"),
181207
},
182208
{
183209
name: "Package/Error/NoDefaultChannel",
@@ -186,7 +212,7 @@ func TestValidators(t *testing.T) {
186212
Icon: &Icon{Data: mustBase64Decode(svgData), MediaType: "image/svg+xml"},
187213
Channels: map[string]*Channel{"light": ch},
188214
},
189-
assertion: require.Error,
215+
assertion: hasError("default channel must be set"),
190216
},
191217
{
192218
name: "Package/Error/ChannelKeyNameMismatch",
@@ -196,7 +222,7 @@ func TestValidators(t *testing.T) {
196222
DefaultChannel: ch,
197223
Channels: map[string]*Channel{"dark": ch},
198224
},
199-
assertion: require.Error,
225+
assertion: hasError(`channel key "dark" does not match channel name "light"`),
200226
},
201227
{
202228
name: "Package/Error/InvalidChannel",
@@ -206,7 +232,7 @@ func TestValidators(t *testing.T) {
206232
DefaultChannel: ch,
207233
Channels: map[string]*Channel{"light": {Name: "light"}},
208234
},
209-
assertion: require.Error,
235+
assertion: hasError(`invalid channel "light"`),
210236
},
211237
{
212238
name: "Package/Error/InvalidChannelPackageLink",
@@ -216,12 +242,12 @@ func TestValidators(t *testing.T) {
216242
DefaultChannel: ch,
217243
Channels: map[string]*Channel{"light": ch},
218244
},
219-
assertion: require.Error,
245+
assertion: hasError(`channel "light" not correctly linked to parent package`),
220246
},
221247
{
222248
name: "Package/Error/DefaultChannelNotInChannelMap",
223249
v: pkgIncorrectDefaultChannel,
224-
assertion: require.Error,
250+
assertion: hasError(`default channel "not-found" not found in channels list`),
225251
},
226252
{
227253
name: "Icon/Success/ValidSVG",
@@ -258,31 +284,31 @@ func TestValidators(t *testing.T) {
258284
// Data: nil,
259285
// MediaType: "image/svg+xml",
260286
// },
261-
// assertion: require.Error,
287+
// assertion: hasError(`icon data must be set if icon is defined`),
262288
//},
263289
//{
264290
// name: "Icon/Error/NoMediaType",
265291
// v: &Icon{
266292
// Data: mustBase64Decode(svgData),
267293
// MediaType: "",
268294
// },
269-
// assertion: require.Error,
295+
// assertion: hasError(`icon mediatype must be set if icon is defined`),
270296
//},
271297
//{
272298
// name: "Icon/Error/DataIsNotImage",
273299
// v: &Icon{
274300
// Data: []byte("{}"),
275301
// MediaType: "application/json",
276302
// },
277-
// assertion: require.Error,
303+
// assertion: hasError(`icon data is not an image`),
278304
//},
279305
//{
280306
// name: "Icon/Error/DataDoesNotMatchMediaType",
281307
// v: &Icon{
282308
// Data: mustBase64Decode(svgData),
283309
// MediaType: "image/jpeg",
284310
// },
285-
// assertion: require.Error,
311+
// assertion: hasError(`icon media type "image/jpeg" does not match detected media type "image/svg+xml"`),
286312
//},
287313
{
288314
name: "Channel/Success/Valid",
@@ -292,22 +318,22 @@ func TestValidators(t *testing.T) {
292318
{
293319
name: "Channel/Error/NoName",
294320
v: &Channel{},
295-
assertion: require.Error,
321+
assertion: hasError(`channel name must not be empty`),
296322
},
297323
{
298324
name: "Channel/Error/NoPackage",
299325
v: &Channel{
300326
Name: "light",
301327
},
302-
assertion: require.Error,
328+
assertion: hasError(`package must be set`),
303329
},
304330
{
305331
name: "Channel/Error/NoBundles",
306332
v: &Channel{
307333
Package: pkg,
308334
Name: "light",
309335
},
310-
assertion: require.Error,
336+
assertion: hasError(`channel must contain at least one bundle`),
311337
},
312338
{
313339
name: "Channel/Error/InvalidHead",
@@ -319,7 +345,7 @@ func TestValidators(t *testing.T) {
319345
"anakin.v0.0.1": {Name: "anakin.v0.0.1"},
320346
},
321347
},
322-
assertion: require.Error,
348+
assertion: hasError(`multiple channel heads found in graph: anakin.v0.0.0, anakin.v0.0.1`),
323349
},
324350
{
325351
name: "Channel/Error/BundleKeyNameMismatch",
@@ -330,7 +356,7 @@ func TestValidators(t *testing.T) {
330356
"foo": {Name: "bar"},
331357
},
332358
},
333-
assertion: require.Error,
359+
assertion: hasError(`bundle key "foo" does not match bundle name "bar"`),
334360
},
335361
{
336362
name: "Channel/Error/InvalidBundle",
@@ -341,7 +367,7 @@ func TestValidators(t *testing.T) {
341367
"anakin.v0.0.0": {Name: "anakin.v0.0.0"},
342368
},
343369
},
344-
assertion: require.Error,
370+
assertion: hasError(`invalid bundle "anakin.v0.0.0"`),
345371
},
346372
{
347373
name: "Channel/Error/InvalidBundleChannelLink",
@@ -357,7 +383,7 @@ func TestValidators(t *testing.T) {
357383
},
358384
},
359385
},
360-
assertion: require.Error,
386+
assertion: hasError(`bundle "anakin.v0.0.0" not correctly linked to parent channel`),
361387
},
362388
{
363389
name: "Bundle/Success/Valid",
@@ -411,7 +437,7 @@ func TestValidators(t *testing.T) {
411437
assertion: require.NoError,
412438
},
413439
{
414-
name: "Bundle/Error/NoBundleImage/NoBundleData",
440+
name: "Bundle/Error/NoBundleImage",
415441
v: &Bundle{
416442
Package: pkg,
417443
Channel: ch,
@@ -423,27 +449,27 @@ func TestValidators(t *testing.T) {
423449
property.MustBuildChannel("light", "anakin.v0.0.1"),
424450
},
425451
},
426-
assertion: require.Error,
452+
assertion: hasError(`bundle image must be set`),
427453
},
428454
{
429455
name: "Bundle/Error/NoName",
430456
v: &Bundle{},
431-
assertion: require.Error,
457+
assertion: hasError(`name must be set`),
432458
},
433459
{
434460
name: "Bundle/Error/NoChannel",
435461
v: &Bundle{
436462
Name: "anakin.v0.1.0",
437463
},
438-
assertion: require.Error,
464+
assertion: hasError(`channel must be set`),
439465
},
440466
{
441467
name: "Bundle/Error/NoPackage",
442468
v: &Bundle{
443469
Channel: ch,
444470
Name: "anakin.v0.1.0",
445471
},
446-
assertion: require.Error,
472+
assertion: hasError(`package must be set`),
447473
},
448474
{
449475
name: "Bundle/Error/WrongPackage",
@@ -452,7 +478,7 @@ func TestValidators(t *testing.T) {
452478
Channel: ch,
453479
Name: "anakin.v0.1.0",
454480
},
455-
assertion: require.Error,
481+
assertion: hasError(`package does not match channel's package`),
456482
},
457483
{
458484
name: "Bundle/Error/InvalidProperty",
@@ -461,9 +487,9 @@ func TestValidators(t *testing.T) {
461487
Channel: ch,
462488
Name: "anakin.v0.1.0",
463489
Replaces: "anakin.v0.0.1",
464-
Properties: []property.Property{{Value: json.RawMessage("")}},
490+
Properties: []property.Property{{Type: "broken", Value: json.RawMessage("")}},
465491
},
466-
assertion: require.Error,
492+
assertion: hasError(`parse property[0] of type "broken": unexpected end of JSON input`),
467493
},
468494
{
469495
name: "Bundle/Error/EmptySkipsValue",
@@ -475,19 +501,7 @@ func TestValidators(t *testing.T) {
475501
Properties: []property.Property{{Type: "custom", Value: json.RawMessage("{}")}},
476502
Skips: []string{""},
477503
},
478-
assertion: require.Error,
479-
},
480-
{
481-
name: "Bundle/Error/SkipsNotInPackage",
482-
v: &Bundle{
483-
Package: pkg,
484-
Channel: ch,
485-
Name: "anakin.v0.1.0",
486-
Replaces: "anakin.v0.0.1",
487-
Properties: []property.Property{{Type: "custom", Value: json.RawMessage("{}")}},
488-
Skips: []string{"foobar"},
489-
},
490-
assertion: require.Error,
504+
assertion: hasError(`skip[0] is empty`),
491505
},
492506
{
493507
name: "Bundle/Error/MissingPackage",
@@ -504,7 +518,7 @@ func TestValidators(t *testing.T) {
504518
property.MustBuildChannel("dark", "anakin.v0.0.1"),
505519
},
506520
},
507-
assertion: require.Error,
521+
assertion: hasError(`must be exactly one property with type "olm.package"`),
508522
},
509523
{
510524
name: "Bundle/Error/MultiplePackages",
@@ -523,7 +537,7 @@ func TestValidators(t *testing.T) {
523537
property.MustBuildChannel("dark", "anakin.v0.0.1"),
524538
},
525539
},
526-
assertion: require.Error,
540+
assertion: hasError(`must be exactly one property with type "olm.package"`),
527541
},
528542
{
529543
name: "RelatedImage/Success/Valid",
@@ -533,21 +547,13 @@ func TestValidators(t *testing.T) {
533547
},
534548
assertion: require.NoError,
535549
},
536-
{
537-
name: "RelatedImage/Error/NoName",
538-
v: RelatedImage{
539-
Name: "",
540-
Image: "",
541-
},
542-
assertion: require.Error,
543-
},
544550
{
545551
name: "RelatedImage/Error/NoImage",
546552
v: RelatedImage{
547553
Name: "foo",
548554
Image: "",
549555
},
550-
assertion: require.Error,
556+
assertion: hasError(`image must be set`),
551557
},
552558
}
553559
for _, s := range specs {

0 commit comments

Comments
 (0)