Skip to content

Commit

Permalink
chore: fix tests and add new ws validators for nil funcs
Browse files Browse the repository at this point in the history
  • Loading branch information
denopink committed Mar 29, 2024
1 parent bb339b2 commit 4ea0f1e
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 30 deletions.
3 changes: 3 additions & 0 deletions cmd/xmidt-agent/ws.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// SPDX-FileCopyrightText: 2023 Comcast Cable Communications Management, LLC
// SPDX-License-Identifier: Apache-2.0

package main

import (
Expand Down
6 changes: 4 additions & 2 deletions cmd/xmidt-agent/xmidt_agent.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# SPDX-FileCopyrightText: 2023 Comcast Cable Communications Management, LLC
# SPDX-License-Identifier: Apache-2.0

websocket:
enable_defaults: true
registration_api: api/v2/device
url_path: api/v2/device
xmidt_credentials:
url: https://localhost:8080/issue
file_name: crt.pem
Expand Down
48 changes: 30 additions & 18 deletions internal/websocket/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestEndToEnd(t *testing.T) {
err = wrp.NewDecoderBytes(got, wrp.Msgpack).Decode(&msg)
require.NoError(err)
require.Equal(wrp.SimpleEventMessageType, msg.Type)
require.Equal("client", msg.Source)
require.Equal("server", msg.Source)

c.Close(websocket.StatusNormalClosure, "")
}))
Expand Down Expand Up @@ -79,14 +79,21 @@ func TestEndToEnd(t *testing.T) {
func(event.Disconnect) {
disconnectCnt.Add(1)
})),
ws.WithIPv6(),
ws.RetryPolicy(&retry.Config{
Interval: time.Second,
Multiplier: 2.0,
Jitter: 1.0 / 3.0,
MaxInterval: 341*time.Second + 333*time.Millisecond,
}),
ws.WithIPv6(),
ws.WithIPv4(),
ws.NowFunc(time.Now),
ws.ConnectTimeout(30*time.Second),
ws.FetchURLTimeout(30*time.Second),
ws.MaxMessageBytes(256*1024),
ws.CredentialsDecorator(func(h http.Header) error {
return nil
}),
)
require.NoError(err)
require.NotNil(got)
Expand Down Expand Up @@ -196,14 +203,19 @@ func TestEndToEndBadData(t *testing.T) {
func(event.Disconnect) {
disconnectCnt.Add(1)
})),
ws.WithIPv6(),
ws.RetryPolicy(&retry.Config{
Interval: time.Second,
Multiplier: 2.0,
Jitter: 1.0 / 3.0,
MaxInterval: 341*time.Second + 333*time.Millisecond,
Interval: 50 * time.Millisecond,
Multiplier: 2.0,
MaxElapsedTime: 300 * time.Millisecond,
}),
ws.WithIPv4(),
ws.NowFunc(time.Now),
ws.ConnectTimeout(30*time.Second),
ws.FetchURLTimeout(30*time.Second),
ws.MaxMessageBytes(256*1024),
ws.CredentialsDecorator(func(h http.Header) error {
return nil
}),
)
require.NoError(err)
require.NotNil(got)
Expand Down Expand Up @@ -244,7 +256,7 @@ func TestEndToEndConnectionIssues(t *testing.T) {
require.NoError(err)
defer c.CloseNow()

ctx, cancel := context.WithTimeout(r.Context(), 200*time.Millisecond)
ctx, cancel := context.WithTimeout(r.Context(), 2000000*time.Millisecond)
defer cancel()

msg := wrp.Message{
Expand All @@ -269,11 +281,6 @@ func TestEndToEndConnectionIssues(t *testing.T) {
return s.URL, nil
}),
ws.DeviceID("mac:112233445566"),
ws.RetryPolicy(&retry.Config{
Interval: 50 * time.Millisecond,
Multiplier: 2.0,
MaxElapsedTime: 300 * time.Millisecond,
}),
ws.AddMessageListener(
event.MsgListenerFunc(
func(m wrp.Message) {
Expand All @@ -289,14 +296,19 @@ func TestEndToEndConnectionIssues(t *testing.T) {
func(event.Disconnect) {
disconnectCnt.Add(1)
})),
ws.WithIPv6(),
ws.RetryPolicy(&retry.Config{
Interval: time.Second,
Multiplier: 2.0,
Jitter: 1.0 / 3.0,
MaxInterval: 341*time.Second + 333*time.Millisecond,
Interval: 50 * time.Millisecond,
Multiplier: 2.0,
MaxElapsedTime: 300 * time.Millisecond,
}),
ws.WithIPv4(),
ws.NowFunc(time.Now),
ws.ConnectTimeout(30*time.Second),
ws.FetchURLTimeout(30*time.Second),
ws.MaxMessageBytes(256*1024),
ws.CredentialsDecorator(func(h http.Header) error {
return nil
}),
)
require.NoError(err)
require.NotNil(got)
Expand Down
44 changes: 43 additions & 1 deletion internal/websocket/internal_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

package websocket

import "fmt"
import (
"fmt"
)

func validateDeviceID() Option {
return optionFunc(
Expand Down Expand Up @@ -34,3 +36,43 @@ func validateIPMode() Option {
return nil
})
}

func validateFetchURL() Option {
return optionFunc(
func(ws *Websocket) error {
if ws.urlFetcher == nil {
return fmt.Errorf("%w: nil FetchURL", ErrMisconfiguredWS)
}

Check warning on line 45 in internal/websocket/internal_options.go

View check run for this annotation

Codecov / codecov/patch

internal/websocket/internal_options.go#L44-L45

Added lines #L44 - L45 were not covered by tests
return nil
})
}

func validateCredentialsDecorator() Option {
return optionFunc(
func(ws *Websocket) error {
if ws.credDecorator == nil {
return fmt.Errorf("%w: nil CredentialsDecorator", ErrMisconfiguredWS)
}

Check warning on line 55 in internal/websocket/internal_options.go

View check run for this annotation

Codecov / codecov/patch

internal/websocket/internal_options.go#L54-L55

Added lines #L54 - L55 were not covered by tests
return nil
})
}

func validateNowFunc() Option {
return optionFunc(
func(ws *Websocket) error {
if ws.nowFunc == nil {
return fmt.Errorf("%w: nil NowFunc", ErrMisconfiguredWS)
}

Check warning on line 65 in internal/websocket/internal_options.go

View check run for this annotation

Codecov / codecov/patch

internal/websocket/internal_options.go#L64-L65

Added lines #L64 - L65 were not covered by tests
return nil
})
}

func validRetryPolicy() Option {
return optionFunc(
func(ws *Websocket) error {
if ws.retryPolicyFactory == nil {
return fmt.Errorf("%w: nil RetryPolicy", ErrMisconfiguredWS)
}

Check warning on line 75 in internal/websocket/internal_options.go

View check run for this annotation

Codecov / codecov/patch

internal/websocket/internal_options.go#L74-L75

Added lines #L74 - L75 were not covered by tests
return nil
})
}
2 changes: 1 addition & 1 deletion internal/websocket/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func CredentialsDecorator(f func(http.Header) error) Option {
return optionFunc(
func(ws *Websocket) error {
if f == nil {
return fmt.Errorf("%w: negative FetchURLTimeout", ErrMisconfiguredWS)
return fmt.Errorf("%w: nil CredentialsDecorator", ErrMisconfiguredWS)
}

Check warning on line 83 in internal/websocket/options.go

View check run for this annotation

Codecov / codecov/patch

internal/websocket/options.go#L82-L83

Added lines #L82 - L83 were not covered by tests

ws.credDecorator = f
Expand Down
4 changes: 4 additions & 0 deletions internal/websocket/ws.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ func New(opts ...Option) (*Websocket, error) {
validateDeviceID(),
validateURL(),
validateIPMode(),
validateFetchURL(),
validateCredentialsDecorator(),
validateNowFunc(),
validRetryPolicy(),
)

for _, opt := range opts {
Expand Down
55 changes: 47 additions & 8 deletions internal/websocket/ws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/stretchr/testify/assert"
mock "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/xmidt-org/retry"
"github.com/xmidt-org/wrp-go/v3"
"github.com/xmidt-org/xmidt-agent/internal/websocket/event"
)
Expand Down Expand Up @@ -53,6 +54,8 @@ func TestNew(t *testing.T) {
h.Add("Credentials-Decorator", "some value")
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
),
check: func(assert *assert.Assertions, c *Websocket) {
// URL Related
Expand Down Expand Up @@ -84,6 +87,11 @@ func TestNew(t *testing.T) {
wsDefaults,
DeviceID("mac:112233445566"),
FetchURL(fetcher),
CredentialsDecorator(func(h http.Header) error {
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
),
check: func(assert *assert.Assertions, c *Websocket) {
u, err := c.urlFetcher(context.Background())
Expand Down Expand Up @@ -141,6 +149,10 @@ func TestNew(t *testing.T) {
NowFunc(func() time.Time {
return time.Unix(1234, 0)
}),
CredentialsDecorator(func(h http.Header) error {
return nil
}),
RetryPolicy(retry.Config{}),
),
check: func(assert *assert.Assertions, c *Websocket) {
if assert.NotNil(c.nowFunc) {
Expand Down Expand Up @@ -196,6 +208,11 @@ func TestMessageListener(t *testing.T) {
DeviceID("mac:112233445566"),
AddMessageListener(&m),
WithIPv6(),
CredentialsDecorator(func(h http.Header) error {
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
)

assert.NoError(err)
Expand All @@ -219,6 +236,11 @@ func TestConnectListener(t *testing.T) {
DeviceID("mac:112233445566"),
AddConnectListener(&m),
WithIPv6(),
CredentialsDecorator(func(h http.Header) error {
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
)

assert.NoError(err)
Expand All @@ -242,6 +264,11 @@ func TestDisconnectListener(t *testing.T) {
DeviceID("mac:112233445566"),
AddDisconnectListener(&m),
WithIPv6(),
CredentialsDecorator(func(h http.Header) error {
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
)

assert.NoError(err)
Expand All @@ -265,6 +292,11 @@ func TestHeartbeatListener(t *testing.T) {
DeviceID("mac:112233445566"),
AddHeartbeatListener(&m),
WithIPv6(),
CredentialsDecorator(func(h http.Header) error {
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
)

assert.NoError(err)
Expand All @@ -277,6 +309,13 @@ func TestHeartbeatListener(t *testing.T) {
}

func TestNextMode(t *testing.T) {
defaults := []Option{
CredentialsDecorator(func(h http.Header) error {
return nil
}),
NowFunc(time.Now),
RetryPolicy(retry.Config{}),
}
tests := []struct {
description string
opts []Option
Expand All @@ -287,32 +326,32 @@ func TestNextMode(t *testing.T) {
description: "IPv4 to IPv6",
mode: ipv4,
expected: ipv6,
opts: []Option{
opts: append(defaults,
WithIPv6(true),
WithIPv4(true),
},
),
}, {
description: "IPv6 to IPv4",
mode: ipv6,
expected: ipv4,
opts: []Option{
opts: append(defaults,
WithIPv6(true),
WithIPv4(true),
},
),
}, {
description: "IPv4 to IPv4",
opts: []Option{
opts: append(defaults,
WithIPv4(true),
WithIPv6(false),
},
),
mode: ipv4,
expected: ipv4,
}, {
description: "IPv6 to IPv6",
opts: []Option{
opts: append(defaults,
WithIPv4(false),
WithIPv6(true),
},
),
mode: ipv6,
expected: ipv6,
},
Expand Down

0 comments on commit 4ea0f1e

Please sign in to comment.