Skip to content

Commit 5bc90f5

Browse files
committed
changed to use cmp.Diff where appropriate
Signed-off-by: Etai Lev Ran <elevran@gmail.com>
1 parent 8f5d587 commit 5bc90f5

File tree

5 files changed

+48
-66
lines changed

5 files changed

+48
-66
lines changed

pkg/epp/datalayer/attributemap_test.go

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,25 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package datalayer_test
17+
package datalayer
1818

1919
import (
2020
"testing"
2121

22+
"github.com/google/go-cmp/cmp"
2223
"github.com/stretchr/testify/assert"
23-
24-
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer"
2524
)
2625

2726
type dummy struct {
2827
Text string
2928
}
3029

31-
func (d *dummy) Clone() datalayer.Cloneable {
30+
func (d *dummy) Clone() Cloneable {
3231
return &dummy{Text: d.Text}
3332
}
3433

3534
func TestExpectPutThenGetToMatch(t *testing.T) {
36-
attrs := datalayer.NewAttributes()
35+
attrs := NewAttributes()
3736
original := &dummy{"foo"}
3837
attrs.Put("a", original)
3938

@@ -47,34 +46,26 @@ func TestExpectPutThenGetToMatch(t *testing.T) {
4746
}
4847

4948
func TestExpectKeysToMatchAdded(t *testing.T) {
50-
attrs := datalayer.NewAttributes()
49+
attrs := NewAttributes()
5150
attrs.Put("x", &dummy{"1"})
5251
attrs.Put("y", &dummy{"2"})
5352

5453
keys := attrs.Keys()
5554
assert.Len(t, keys, 2)
56-
57-
found := map[string]bool{}
58-
for _, k := range keys {
59-
found[k] = true
60-
}
61-
62-
assert.True(t, found["x"])
63-
assert.True(t, found["y"])
55+
assert.ElementsMatch(t, keys, []string{"x", "y"})
6456
}
6557

6658
func TestCloneReturnsCopy(t *testing.T) {
67-
original := datalayer.NewAttributes()
59+
original := NewAttributes()
6860
original.Put("k", &dummy{"value"})
6961

7062
cloned := original.Clone()
7163

72-
gotOrig, _ := original.Get("k")
73-
gotClone, _ := cloned.Get("k")
74-
75-
assert.NotSame(t, gotOrig, gotClone, "expected cloned value to be a different instance")
64+
kOrig, _ := original.Get("k")
65+
kClone, _ := cloned.Get("k")
7666

77-
dv, ok := gotClone.(*dummy)
78-
assert.True(t, ok)
79-
assert.Equal(t, "value", dv.Text)
67+
assert.NotSame(t, kOrig, kClone, "expected cloned value to be a different instance")
68+
if diff := cmp.Diff(kOrig, kClone); diff != "" {
69+
t.Errorf("Unexpected output (-want +got): %v", diff)
70+
}
8071
}

pkg/epp/datalayer/datasource.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,15 @@ func GetSources() []DataSource {
106106
// ValidateExtractorType checks if an extractor can handle
107107
// the DataSource's output. It should be called by a DataSource
108108
// when an extractor is added.
109-
func ValidateExtractorType(output, input reflect.Type) error {
110-
if output == nil || input == nil {
109+
func ValidateExtractorType(collectorOutput, extractorInput reflect.Type) error {
110+
if collectorOutput == nil || extractorInput == nil {
111111
return errors.New("extractor input type or data source output type can't be nil")
112112
}
113-
if output == input ||
114-
(input.Kind() == reflect.Interface && input.NumMethod() == 0) ||
115-
(input.Kind() == reflect.Interface && output.Implements(input)) {
113+
if collectorOutput == extractorInput ||
114+
(extractorInput.Kind() == reflect.Interface && extractorInput.NumMethod() == 0) ||
115+
(extractorInput.Kind() == reflect.Interface && collectorOutput.Implements(extractorInput)) {
116116
return nil
117117
}
118118
return fmt.Errorf("extractor input type %v cannot handle data source output type %v",
119-
input, output)
119+
extractorInput, collectorOutput)
120120
}

pkg/epp/datalayer/datasource_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,25 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package datalayer_test
17+
package datalayer
1818

1919
import (
2020
"reflect"
2121
"testing"
2222

2323
"github.com/stretchr/testify/assert"
24-
25-
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer"
2624
)
2725

2826
type mockDataSource struct {
2927
name string
3028
}
3129

32-
func (m *mockDataSource) Name() string { return m.name }
33-
func (m *mockDataSource) AddExtractor(_ datalayer.Extractor) error { return nil }
34-
func (m *mockDataSource) Collect(_ datalayer.Endpoint) {}
30+
func (m *mockDataSource) Name() string { return m.name }
31+
func (m *mockDataSource) AddExtractor(_ Extractor) error { return nil }
32+
func (m *mockDataSource) Collect(_ Endpoint) {}
3533

3634
func TestRegisterAndGetSource(t *testing.T) {
37-
reg := datalayer.DataSourceRegistry{}
35+
reg := DataSourceRegistry{}
3836
ds := &mockDataSource{name: "test"}
3937

4038
err := reg.Register(ds)
@@ -56,7 +54,7 @@ func TestRegisterAndGetSource(t *testing.T) {
5654
}
5755

5856
func TestGetNamedSourceWhenNotFound(t *testing.T) {
59-
reg := datalayer.DataSourceRegistry{}
57+
reg := DataSourceRegistry{}
6058
_, found := reg.GetNamedSource("missing")
6159
assert.False(t, found, "expected source to be missing")
6260
}
@@ -78,7 +76,7 @@ func TestValidateExtractorType(t *testing.T) {
7876
}
7977

8078
for _, tt := range tests {
81-
err := datalayer.ValidateExtractorType(tt.output, tt.input)
79+
err := ValidateExtractorType(tt.output, tt.input)
8280
if tt.valid {
8381
assert.NoError(t, err, "%s: expected valid extractor type", tt.name)
8482
} else {

pkg/epp/datalayer/metrics_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,18 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package datalayer_test
17+
package datalayer
1818

1919
import (
2020
"testing"
2121
"time"
2222

2323
"github.com/google/go-cmp/cmp"
2424
"github.com/stretchr/testify/assert"
25-
26-
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer"
2725
)
2826

2927
func TestMetricsClone(t *testing.T) {
30-
m := &datalayer.Metrics{
28+
m := &Metrics{
3129
ActiveModels: map[string]int{"modelA": 1},
3230
WaitingModels: map[string]int{"modelB": 2},
3331
MaxActiveModels: 5,
@@ -39,8 +37,6 @@ func TestMetricsClone(t *testing.T) {
3937
}
4038

4139
clone := m.Clone()
42-
43-
assert.NotNil(t, clone)
4440
if diff := cmp.Diff(m, clone); diff != "" {
4541
t.Errorf("Unexpected output (-want +got): %v", diff)
4642
}
@@ -53,14 +49,14 @@ func TestMetricsClone(t *testing.T) {
5349
}
5450

5551
func TestMetricsCloneOfNil(t *testing.T) {
56-
var m *datalayer.Metrics
52+
var m *Metrics
5753
assert.Nil(t, m.Clone())
5854
}
5955

6056
func TestMetricsToString(t *testing.T) {
61-
m := datalayer.NewMetrics()
57+
m := NewMetrics()
6258
assert.NotEmpty(t, m.String())
6359

64-
var none *datalayer.Metrics
60+
var none *Metrics
6561
assert.Equal(t, "", none.String())
6662
}

pkg/epp/datalayer/podinfo_test.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,16 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package datalayer_test
17+
package datalayer
1818

1919
import (
2020
"testing"
2121

22+
"github.com/google/go-cmp/cmp"
2223
"github.com/stretchr/testify/assert"
2324
corev1 "k8s.io/api/core/v1"
2425
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2526
"k8s.io/apimachinery/pkg/types"
26-
27-
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer"
2827
)
2928

3029
const (
@@ -49,35 +48,33 @@ var (
4948
PodIP: podip,
5049
},
5150
}
51+
expected = &PodInfo{
52+
NamespacedName: types.NamespacedName{Name: name, Namespace: namespace},
53+
Address: podip,
54+
Labels: labels,
55+
}
5256
)
5357

5458
func TestToPodInfo(t *testing.T) {
55-
podinfo := datalayer.ToPodInfo(pod)
56-
57-
assert.Equal(t, name, podinfo.NamespacedName.Name)
58-
assert.Equal(t, namespace, podinfo.NamespacedName.Namespace)
59-
assert.Equal(t, podip, podinfo.Address)
60-
assert.Equal(t, labels, podinfo.Labels)
59+
podinfo := ToPodInfo(pod)
60+
if diff := cmp.Diff(expected, podinfo); diff != "" {
61+
t.Errorf("Unexpected output (-want +got): %v", diff)
62+
}
6163
}
6264

6365
func TestPodInfoClone(t *testing.T) {
64-
podinfo := &datalayer.PodInfo{
65-
NamespacedName: types.NamespacedName{Name: name, Namespace: namespace},
66-
Address: podip,
67-
Labels: labels,
66+
clone := expected.Clone()
67+
assert.NotSame(t, expected, clone)
68+
if diff := cmp.Diff(expected, clone); diff != "" {
69+
t.Errorf("Unexpected output (-want +got): %v", diff)
6870
}
6971

70-
clone := podinfo.Clone()
71-
72-
assert.Equal(t, podinfo, clone)
73-
assert.NotSame(t, podinfo, clone)
74-
assert.Equal(t, podinfo.Labels, clone.Labels)
7572
clone.Labels["env"] = "staging"
76-
assert.Equal(t, "prod", podinfo.Labels["env"], "mutating clone should not affect original")
73+
assert.Equal(t, "prod", expected.Labels["env"], "mutating clone should not affect original")
7774
}
7875

7976
func TestPodInfoString(t *testing.T) {
80-
podinfo := datalayer.ToPodInfo(pod)
77+
podinfo := ToPodInfo(pod)
8178

8279
s := podinfo.String()
8380
assert.Contains(t, s, name)

0 commit comments

Comments
 (0)