Skip to content

Commit

Permalink
✅ Unit tests: linter fixes, testify cleanup
Browse files Browse the repository at this point in the history 
Use testify when possible, fix linter warnings, remove dead code
  • Loading branch information
@jfbus
jfbus committed Jan 7, 2025
1 parent 886b728 commit cf8d568
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 374 deletions.
83 changes: 27 additions & 56 deletions cloud-controller-manager/osc/instances_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

osc "github.com/outscale/osc-sdk-go/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
)

Expand Down Expand Up @@ -82,16 +83,11 @@ func TestMapToAWSInstanceIDs(t *testing.T) {

for _, test := range tests {
awsID, err := test.Kubernetes.MapToAWSInstanceID()
if err != nil {
if !test.ExpectError {
t.Errorf("unexpected error parsing %s: %v", test.Kubernetes, err)
}
if test.ExpectError {
require.Error(t, err)
} else {
if test.ExpectError {
t.Errorf("expected error parsing %s", test.Kubernetes)
} else if test.Aws != awsID {
t.Errorf("unexpected value parsing %s, got %s", test.Kubernetes, awsID)
}
require.NoError(t, err)
assert.Equal(t, test.Aws, awsID)
}
}

Expand All @@ -100,60 +96,44 @@ func TestMapToAWSInstanceIDs(t *testing.T) {
node.Spec.ProviderID = string(test.Kubernetes)

awsInstanceIds, err := mapToAWSInstanceIDs([]*v1.Node{node})
if err != nil {
if !test.ExpectError {
t.Errorf("unexpected error parsing %s: %v", test.Kubernetes, err)
}
if test.ExpectError {
require.Error(t, err)
} else {
if test.ExpectError {
t.Errorf("expected error parsing %s", test.Kubernetes)
} else if len(awsInstanceIds) != 1 {
t.Errorf("unexpected value parsing %s, got %s", test.Kubernetes, awsInstanceIds)
} else if awsInstanceIds[0] != test.Aws {
t.Errorf("unexpected value parsing %s, got %s", test.Kubernetes, awsInstanceIds)
}
require.NoError(t, err)
require.Len(t, awsInstanceIds, 1)
assert.Equal(t, test.Aws, awsInstanceIds[0])
}

awsInstanceIds = mapToAWSInstanceIDsTolerant([]*v1.Node{node})
if test.ExpectError {
if len(awsInstanceIds) != 0 {
t.Errorf("unexpected results parsing %s: %s", test.Kubernetes, awsInstanceIds)
}
require.Empty(t, awsInstanceIds)
} else {
if len(awsInstanceIds) != 1 {
t.Errorf("unexpected value parsing %s, got %s", test.Kubernetes, awsInstanceIds)
} else if awsInstanceIds[0] != test.Aws {
t.Errorf("unexpected value parsing %s, got %s", test.Kubernetes, awsInstanceIds)
}
require.Len(t, awsInstanceIds, 1)
assert.Equal(t, test.Aws, awsInstanceIds[0])
}
}
}

func TestSnapshotMeetsCriteria(t *testing.T) {
snapshot := &allInstancesSnapshot{timestamp: time.Now().Add(-3601 * time.Second)}

if !snapshot.MeetsCriteria(cacheCriteria{}) {
t.Errorf("Snapshot should always meet empty criteria")
}
assert.True(t, snapshot.MeetsCriteria(cacheCriteria{}),
"Snapshot should always meet empty criteria")

if snapshot.MeetsCriteria(cacheCriteria{MaxAge: time.Hour}) {
t.Errorf("Snapshot did not honor MaxAge")
}
assert.False(t, snapshot.MeetsCriteria(cacheCriteria{MaxAge: time.Hour}),
"Snapshot did not honor MaxAge")

if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}) {
t.Errorf("Snapshot did not honor HasInstances with missing instances")
}
assert.False(t, snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}),
"Snapshot did not honor HasInstances with missing instances")

snapshot.instances = make(map[InstanceID]*osc.Vm)
snapshot.instances[InstanceID("i-12345678")] = &osc.Vm{}

if !snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}) {
t.Errorf("Snapshot did not honor HasInstances with matching instances")
}
assert.True(t, snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}),
"Snapshot did not honor HasInstances with matching instances")

if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678"), InstanceID("i-00000000")}}) {
t.Errorf("Snapshot did not honor HasInstances with partially matching instances")
}
assert.False(t, snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678"), InstanceID("i-00000000")}}),
"Snapshot did not honor HasInstances with partially matching instances")
}

func TestOlderThan(t *testing.T) {
Expand Down Expand Up @@ -184,21 +164,12 @@ func TestSnapshotFindInstances(t *testing.T) {
}

instances := snapshot.FindInstances([]InstanceID{InstanceID("i-12345678"), InstanceID("i-23456789"), InstanceID("i-00000000")})
if len(instances) != 2 {
t.Errorf("findInstances returned %d results, expected 2", len(instances))
}
require.Len(t, instances, 2)

for _, id := range []InstanceID{InstanceID("i-12345678"), InstanceID("i-23456789")} {
i := instances[id]
if i == nil {
t.Errorf("findInstances did not return %s", id)
continue
}
if i.GetVmId() != string(id) {
t.Errorf("findInstances did not return expected instanceId for %s", id)
}
if i != snapshot.instances[id] {
t.Errorf("findInstances did not return expected instance (reference equality) for %s", id)
}
require.NotNil(t, i)
assert.Equal(t, string(id), i.GetVmId())
assert.Equal(t, snapshot.instances[id], i)
}
}
75 changes: 14 additions & 61 deletions cloud-controller-manager/osc/osc_fakes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,23 +445,23 @@ func (m *FakeMetadata) Available() bool {
func (m *FakeMetadata) GetMetadata(key string) (string, error) {
networkInterfacesPrefix := "network/interfaces/macs/"
i := m.aws.selfInstance
if key == "placement/availability-zone" {
az := ""
switch {
case key == "placement/availability-zone":
if i.Placement != nil {
az = i.Placement.GetSubregionName()
return i.Placement.GetSubregionName(), nil
}
return az, nil
} else if key == "instance-id" {
return "", nil
case key == "instance-id":
return i.GetVmId(), nil
} else if key == "local-hostname" {
case key == "local-hostname":
return i.GetPrivateDnsName(), nil
} else if key == "public-hostname" {
case key == "public-hostname":
return i.GetPublicDnsName(), nil
} else if key == "local-ipv4" {
case key == "local-ipv4":
return i.GetPrivateIp(), nil
} else if key == "public-ipv4" {
case key == "public-ipv4":
return i.GetPublicIp(), nil
} else if strings.HasPrefix(key, networkInterfacesPrefix) {
case strings.HasPrefix(key, networkInterfacesPrefix):
if key == networkInterfacesPrefix {
return strings.Join(m.aws.networkInterfacesMacs, "/\n") + "/\n", nil
}
Expand Down Expand Up @@ -501,7 +501,7 @@ func (fakeElb *FakeELB) CreateLoadBalancer(input *elb.CreateLoadBalancerInput) (
lb := elb.LoadBalancerDescription{
Subnets: input.Subnets,
AvailabilityZones: input.AvailabilityZones,
DNSName: aws.String(fmt.Sprintf("%v", *input.LoadBalancerName)),
DNSName: input.LoadBalancerName,
HealthCheck: &elb.HealthCheck{},
LoadBalancerName: input.LoadBalancerName,
}
Expand Down Expand Up @@ -642,53 +642,6 @@ func (fakeElb *FakeELB) ModifyLoadBalancerAttributes(*elb.ModifyLoadBalancerAttr
panic("Not implemented")
}

// expectDescribeLoadBalancers is not implemented but is required for interface
// conformance
func (fakeElb *FakeELB) expectDescribeLoadBalancers(loadBalancerName string) {
panic("Not implemented")
}

func instanceMatchesFilter(instance *ec2.Instance, filter *ec2.Filter) bool {
name := *filter.Name
if name == "private-dns-name" {
if instance.PrivateDnsName == nil {
return false
}
return contains(filter.Values, *instance.PrivateDnsName)
}

if name == "instance-state-name" {
return contains(filter.Values, *instance.State.Name)
}

if name == "tag-key" {
for _, instanceTag := range instance.Tags {
if contains(filter.Values, aws.StringValue(instanceTag.Key)) {
return true
}
}
return false
}

if strings.HasPrefix(name, "tag:") {
tagName := name[4:]
for _, instanceTag := range instance.Tags {
if aws.StringValue(instanceTag.Key) == tagName && contains(filter.Values, aws.StringValue(instanceTag.Value)) {
return true
}
}
return false
}

panic("Unknown filter name: " + name)
}

func contains(haystack []*string, needle string) bool {
for _, s := range haystack {
// (deliberately panic if s == nil)
if needle == *s {
return true
}
}
return false
}
var _ Services = (*FakeOscServices)(nil)
var _ LoadBalancer = (*FakeELB)(nil)
var _ Compute = (*FakeComputeImpl)(nil)
Loading

0 comments on commit cf8d568

Please sign in to comment.