Skip to content

Commit

Permalink
fix: clewan up wrong unit test checks.
Browse files Browse the repository at this point in the history 
Signed-off-by: Asklv <[email protected]>
  • Loading branch information
@IRONICBo
IRONICBo committed Jul 25, 2024
1 parent 735eba1 commit 1d3a874
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 40 deletions.
19 changes: 11 additions & 8 deletions pkg/apiserver/registry/stats/nodelatencystats/rest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package nodelatencystats

import (
"context"
"time"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
Expand Down Expand Up @@ -101,8 +102,8 @@ func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOpti
ColumnDefinitions: []metav1.TableColumnDefinition{
{Name: "Node Name", Type: "string", Format: "name", Description: "Name of Node from which latency was measured."},
{Name: "Num Latency Entries", Type: "integer", Format: "int64", Description: "Number of peers for which latency measurements are available."},
{Name: "Max Latency", Type: "integer", Format: "int64", Description: "Name of the peer with the highest latency and the latency value(ns)."},
{Name: "Avg Latency", Type: "integer", Format: "int64", Description: "Average latency value across all peers(ns)."},
{Name: "Max Latency", Type: "string", Format: "", Description: "Name of the peer with the highest latency and the latency value."},
{Name: "Avg Latency", Type: "string", Format: "", Description: "Average latency value across all peers."},
},
}
if m, err := meta.ListAccessor(obj); err == nil {
Expand Down Expand Up @@ -135,15 +136,17 @@ func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOpti
maxLatency = currentLatency
}

if targetIPLatencyCount == 1 {
avgLatency = currentLatency
} else {
avgLatency += (currentLatency - avgLatency) / targetIPLatencyCount
}
// Due to int64 max value is enough for the sum of all latencies,
// we don't need to check overflow in this case.
avgLatency += currentLatency
}
}

return []interface{}{name, peerNodeLatencyEntriesCount, maxLatency, avgLatency}, nil
if targetIPLatencyCount > 0 {
avgLatency = avgLatency / targetIPLatencyCount
}

return []interface{}{name, peerNodeLatencyEntriesCount, time.Duration(maxLatency).String(), time.Duration(avgLatency).String()}, nil
})
return table, err
}
Expand Down
84 changes: 52 additions & 32 deletions pkg/apiserver/registry/stats/nodelatencystats/rest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package nodelatencystats
import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -46,8 +47,8 @@ func TestRESTCreate(t *testing.T) {
}

r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := context.Background()

obj, err := r.Create(ctx, summary, nil, nil)
assert.NoError(t, err)
assert.Equal(t, expectedObj, obj)
Expand All @@ -59,7 +60,6 @@ func TestRESTGet(t *testing.T) {
summary *statsv1alpha1.NodeLatencyStats
nodeName string
expectedObj runtime.Object
expectedErr bool
err error
}{
{
Expand All @@ -73,8 +73,7 @@ func TestRESTGet(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
},
expectedErr: false,
err: nil,
err: nil,
},
{
name: "get summary not found",
Expand All @@ -84,20 +83,20 @@ func TestRESTGet(t *testing.T) {
},
nodeName: "node2",
expectedObj: nil,
expectedErr: true,
err: errors.NewNotFound(statsv1alpha1.Resource("nodelatencystats"), "node2"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := context.Background()

_, err := r.Create(ctx, tt.summary, nil, nil)
assert.Nil(t, err)
require.NoError(t, err)

obj, err := r.Get(ctx, tt.nodeName, nil)
if tt.expectedErr {
assert.Equal(t, tt.err.(*errors.StatusError).ErrStatus.Code, err.(*errors.StatusError).ErrStatus.Code)
if tt.err != nil {
assert.EqualError(t, tt.err, err.Error())
} else {
require.NoError(t, err)
assert.Equal(t, tt.expectedObj, obj)
Expand All @@ -112,7 +111,6 @@ func TestRESTDelete(t *testing.T) {
summary *statsv1alpha1.NodeLatencyStats
nodeName string
expectedObj runtime.Object
expectedErr bool
err error
}{
{
Expand All @@ -126,8 +124,7 @@ func TestRESTDelete(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
},
expectedErr: false,
err: nil,
err: nil,
},
{
name: "delete summary not found",
Expand All @@ -137,21 +134,19 @@ func TestRESTDelete(t *testing.T) {
},
nodeName: "node2",
expectedObj: nil,
expectedErr: true,
err: errors.NewNotFound(statsv1alpha1.Resource("nodelatencystats"), "node2"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := context.Background()

_, err := r.Create(ctx, tt.summary, nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
obj, deleted, err := r.Delete(ctx, tt.nodeName, nil, nil)
if tt.expectedErr {
assert.Equal(t, tt.err.(*errors.StatusError).ErrStatus.Code, err.(*errors.StatusError).ErrStatus.Code)
if tt.err != nil {
assert.EqualError(t, tt.err, err.Error())
} else {
require.NoError(t, err)
assert.True(t, deleted)
Expand Down Expand Up @@ -180,33 +175,58 @@ func TestRESTList(t *testing.T) {
}

r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := context.Background()

_, err := r.Create(ctx, summary, nil, nil)
assert.Nil(t, err)
require.NoError(t, err)
objs, err := r.List(ctx, options)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, expectedObj, objs)
}

func TestRESTConvertToTable(t *testing.T) {
mockTime := time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC)
summary := &statsv1alpha1.NodeLatencyStats{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: []statsv1alpha1.PeerNodeLatencyStats{
{
NodeName: "node2",
TargetIPLatencyStats: []statsv1alpha1.TargetIPLatencyStats{
{
TargetIP: "192.168.0.1",
LastSendTime: metav1.Time{Time: mockTime},
LastRecvTime: metav1.Time{Time: mockTime},
LastMeasuredRTTNanoseconds: 0,
},
},
},
},
}
expectedObj := &statsv1alpha1.NodeLatencyStats{
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: nil,
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
PeerNodeLatencyStats: []statsv1alpha1.PeerNodeLatencyStats{
{
NodeName: "node2",
TargetIPLatencyStats: []statsv1alpha1.TargetIPLatencyStats{
{
TargetIP: "192.168.0.1",
LastSendTime: metav1.Time{Time: mockTime},
LastRecvTime: metav1.Time{Time: mockTime},
LastMeasuredRTTNanoseconds: 0,
},
},
},
},
}
expectedCells := []interface{}{"node1", 1, "0s", "0s"}

r := NewREST()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx := context.Background()

_, err := r.Create(ctx, summary, nil, nil)
assert.Nil(t, err)
require.NoError(t, err)
obj, err := r.ConvertToTable(ctx, summary, nil)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, expectedObj, obj.Rows[0].Object.Object)
assert.Equal(t, expectedCells, obj.Rows[0].Cells)
}

0 comments on commit 1d3a874

Please sign in to comment.