Skip to content

Commit aba3268

Browse files
quirogasquirogas
authored
test(internal/surfer): increase test coverage for utils (#4166)
Refactors and expand the unit tests for the gcloud utility functions to increase test coverage. ### Coverage Comparison (`utils` package) | Function | Old Coverage | New Coverage | Increment | | :--- | :--- | :--- | :--- | | `GetGcloudType` | 88.9% | **100.0%** | +11.1% | | `IsStandardMethod` | 0.0% | **100.0%** | +100.0% | | `IsResourceMethod` | 80.0% | **100.0%** | +20.0% | | `IsCollectionMethod` | 80.0% | **100.0%** | +20.0% | | `GetParentFromSegments` | 80.0% | **100.0%** | +20.0% | | `getResourceFromMethod` | 50.0% | **100.0%** | +50.0% | | `GetResourceForMethod` | 84.2% | **100.0%** | +15.8% | | `GetPluralResourceNameForMethod` | 80.0% | **100.0%** | +20.0% | | `GetSingularResourceNameForMethod` | 80.0% | **100.0%** | +20.0% | Overall utility functions test coverage increased to 97%. Fixes #3632 Updates #3666 --------- Signed-off-by: Santiago Quiroga <22756465+quirogas@users.noreply.github.com>
1 parent 89ff969 commit aba3268

6 files changed

Lines changed: 243 additions & 96 deletions

File tree

‎codecov.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,4 @@ ignore:
3030
- internal/legacylibrarian/legacycontainer/java/pom
3131
- internal/legacylibrarian/legacycontainer/java/release
3232
- internal/legacylibrarian/legacyimages
33-
# TODO(https://github.com/googleapis/librarian/issues/3632): improve coverage
3433
- internal/surfer/gcloud

‎internal/surfer/gcloud/utils/field_test.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ func TestGetGcloudType(t *testing.T) {
3939
{"Message", api.MESSAGE_TYPE, "arg_object"},
4040
} {
4141
t.Run(test.name, func(t *testing.T) {
42+
t.Parallel()
4243
got := GetGcloudType(test.typez)
4344
if got != test.want {
44-
t.Errorf("GetGcloudType(%v) = %q, want %q", test.typez, got, test.want)
45+
t.Errorf("GetGcloudType() = %q, want %q", got, test.want)
4546
}
4647
})
4748
}
@@ -62,10 +63,21 @@ func TestIsSafeName(t *testing.T) {
6263
{"", true},
6364
} {
6465
t.Run(test.name, func(t *testing.T) {
66+
t.Parallel()
6567
got := IsSafeName(test.name)
6668
if got != test.want {
67-
t.Errorf("IsSafeName(%q) = %v, want %v", test.name, got, test.want)
69+
t.Errorf("IsSafeName() = %v, want %v", got, test.want)
6870
}
6971
})
7072
}
7173
}
74+
75+
func TestGetGcloudType_Panic(t *testing.T) {
76+
t.Parallel()
77+
defer func() {
78+
if r := recover(); r == nil {
79+
t.Errorf("GetGcloudType() did not panic for unsupported type")
80+
}
81+
}()
82+
GetGcloudType(api.Typez(999))
83+
}

‎internal/surfer/gcloud/utils/method_test.go‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ func TestIsCreate(t *testing.T) {
3737
t.Run(test.name, func(t *testing.T) {
3838
t.Parallel()
3939
got := IsCreate(test.method)
40-
if diff := cmp.Diff(test.want, got); diff != "" {
41-
t.Errorf("mismatch (-want +got):\n%s", diff)
40+
if got != test.want {
41+
t.Errorf("got %v, want %v", got, test.want)
4242
}
4343
})
4444
}
@@ -59,8 +59,8 @@ func TestIsGet(t *testing.T) {
5959
t.Run(test.name, func(t *testing.T) {
6060
t.Parallel()
6161
got := IsGet(test.method)
62-
if diff := cmp.Diff(test.want, got); diff != "" {
63-
t.Errorf("mismatch (-want +got):\n%s", diff)
62+
if got != test.want {
63+
t.Errorf("got %v, want %v", got, test.want)
6464
}
6565
})
6666
}
@@ -81,8 +81,8 @@ func TestIsList(t *testing.T) {
8181
t.Run(test.name, func(t *testing.T) {
8282
t.Parallel()
8383
got := IsList(test.method)
84-
if diff := cmp.Diff(test.want, got); diff != "" {
85-
t.Errorf("mismatch (-want +got):\n%s", diff)
84+
if got != test.want {
85+
t.Errorf("got %v, want %v", got, test.want)
8686
}
8787
})
8888
}
@@ -104,8 +104,8 @@ func TestIsUpdate(t *testing.T) {
104104
t.Run(test.name, func(t *testing.T) {
105105
t.Parallel()
106106
got := IsUpdate(test.method)
107-
if diff := cmp.Diff(test.want, got); diff != "" {
108-
t.Errorf("mismatch (-want +got):\n%s", diff)
107+
if got != test.want {
108+
t.Errorf("got %v, want %v", got, test.want)
109109
}
110110
})
111111
}
@@ -126,8 +126,8 @@ func TestIsDelete(t *testing.T) {
126126
t.Run(test.name, func(t *testing.T) {
127127
t.Parallel()
128128
got := IsDelete(test.method)
129-
if diff := cmp.Diff(test.want, got); diff != "" {
130-
t.Errorf("mismatch (-want +got):\n%s", diff)
129+
if got != test.want {
130+
t.Errorf("got %v, want %v", got, test.want)
131131
}
132132
})
133133
}
@@ -164,8 +164,8 @@ func TestGetCommandName(t *testing.T) {
164164
if err != nil {
165165
t.Fatalf("GetCommandName() error = %v", err)
166166
}
167-
if diff := cmp.Diff(test.want, got); diff != "" {
168-
t.Errorf("mismatch (-want +got):\n%s", diff)
167+
if got != test.want {
168+
t.Errorf("got %q, want %q", got, test.want)
169169
}
170170
})
171171
}
@@ -206,9 +206,10 @@ func TestFindResourceMessage(t *testing.T) {
206206
},
207207
} {
208208
t.Run(test.name, func(t *testing.T) {
209+
t.Parallel()
209210
got := FindResourceMessage(test.outputType)
210211
if diff := cmp.Diff(test.want, got); diff != "" {
211-
t.Errorf("FindResourceMessage mismatch (-want +got):\n%s", diff)
212+
t.Errorf("mismatch (-want +got):\n%s", diff)
212213
}
213214
})
214215
}
@@ -264,12 +265,17 @@ func TestIsResourceMethod(t *testing.T) {
264265
PathTemplate: &api.PathTemplate{Segments: []api.PathSegment{*api.NewPathSegment().WithLiteral("instances")}},
265266
}}},
266267
}, false},
268+
{"Nil PathInfo", &api.Method{Name: "CustomMethod", PathInfo: nil}, false},
269+
{"Empty Bindings", &api.Method{Name: "CustomMethod", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{}}}, false},
270+
{"Nil PathTemplate", &api.Method{Name: "CustomMethod", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{PathTemplate: nil}}}}, false},
271+
{"Empty Segments", &api.Method{Name: "CustomMethod", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{PathTemplate: &api.PathTemplate{Segments: []api.PathSegment{}}}}}}, false},
267272
} {
268273
test := test
269274
t.Run(test.name, func(t *testing.T) {
270275
t.Parallel()
271-
if got := IsResourceMethod(test.method); got != test.want {
272-
t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(test.want, got))
276+
got := IsResourceMethod(test.method)
277+
if got != test.want {
278+
t.Errorf("got %v, want %v", got, test.want)
273279
}
274280
})
275281
}
@@ -295,12 +301,40 @@ func TestIsCollectionMethod(t *testing.T) {
295301
PathTemplate: &api.PathTemplate{Segments: []api.PathSegment{*api.NewPathSegment().WithLiteral("instances")}},
296302
}}},
297303
}, true},
304+
{"Nil PathInfo", &api.Method{Name: "CustomMethod", PathInfo: nil}, false},
305+
{"Empty Bindings", &api.Method{Name: "CustomMethod", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{}}}, false},
306+
{"Nil PathTemplate", &api.Method{Name: "CustomMethod", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{PathTemplate: nil}}}}, false},
307+
{"Empty Segments", &api.Method{Name: "CustomMethod", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{PathTemplate: &api.PathTemplate{Segments: []api.PathSegment{}}}}}}, false},
298308
} {
299309
test := test
300310
t.Run(test.name, func(t *testing.T) {
301311
t.Parallel()
302-
if got := IsCollectionMethod(test.method); got != test.want {
303-
t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(test.want, got))
312+
got := IsCollectionMethod(test.method)
313+
if got != test.want {
314+
t.Errorf("got %v, want %v", got, test.want)
315+
}
316+
})
317+
}
318+
}
319+
320+
func TestIsStandardMethod(t *testing.T) {
321+
for _, test := range []struct {
322+
name string
323+
method *api.Method
324+
want bool
325+
}{
326+
{"Get", &api.Method{Name: "GetInstance", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{Verb: "GET"}}}}, true},
327+
{"List", &api.Method{Name: "ListInstances", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{Verb: "GET"}}}}, true},
328+
{"Create", &api.Method{Name: "CreateInstance", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{Verb: "POST"}}}}, true},
329+
{"Update", &api.Method{Name: "UpdateInstance", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{Verb: "PATCH"}}}}, true},
330+
{"Delete", &api.Method{Name: "DeleteInstance", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{Verb: "DELETE"}}}}, true},
331+
{"Custom", &api.Method{Name: "ExportInstance", PathInfo: &api.PathInfo{Bindings: []*api.PathBinding{{Verb: "POST"}}}}, false},
332+
} {
333+
t.Run(test.name, func(t *testing.T) {
334+
t.Parallel()
335+
got := IsStandardMethod(test.method)
336+
if got != test.want {
337+
t.Errorf("got %v, want %v", got, test.want)
304338
}
305339
})
306340
}

‎internal/surfer/gcloud/utils/resource.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,17 +130,11 @@ func IsPrimaryResource(field *api.Field, method *api.Method) bool {
130130
// {Type} is the singular form of the resource noun.
131131
func getResourceNameFromType(typeStr string) string {
132132
parts := strings.Split(typeStr, "/")
133-
if len(parts) > 0 {
134-
return parts[len(parts)-1]
135-
}
136-
return ""
133+
return parts[len(parts)-1]
137134
}
138135

139136
// getResourceFromMethod extracts the resource definition from a method's input message if it exists.
140137
func getResourceFromMethod(method *api.Method) (*api.Resource, error) {
141-
if method.InputType == nil {
142-
return nil, fmt.Errorf("method %q does not have an input type", method.Name)
143-
}
144138
for _, f := range method.InputType.Fields {
145139
if f.MessageType != nil && f.MessageType.Resource != nil {
146140
return f.MessageType.Resource, nil

0 commit comments

Comments
 (0)