Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enhance defaultHWAddrFunc() and tests to hit 100% coverage #57

Merged
merged 3 commits into from Sep 9, 2022
Merged

Enhance defaultHWAddrFunc() and tests to hit 100% coverage #57

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b46831
Enhance defaultHWAddrFunc() and tests to hit 100% coverage
theckman Oct 6, 2018
96f118c
Merge remote-tracking branch 'remotes/origin/master' into 100%_coverage
cameracker Sep 7, 2022
20095e6
Merge remote-tracking branch 'remotes/origin/master' into 100%_coverage
cameracker Sep 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion generator.go
View file
Open in desktop
Expand Up @@ -558,9 +558,11 @@ func newFromHash(h hash.Hash, ns UUID, name string) UUID {
return u
}

var netInterfaces = net.Interfaces
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the call site for netInterfaces, we use ifaces, err := netInterfaces() and rely on type inference. I would prefer adding an explicit type here, in the style of these hooks: https://golang.org/pkg/internal/poll/#pkg-variables, to better communicate the signature of what is being hooked, so the reader doesn't have to go through yet another indirection.

Perhaps var netInterfaces func() ([]net.Interface, error) = net.Interfaces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like how it clutters up the code. It would be easier for my editor to give me the function header definition over trying to read that line.

I was trying to hit 100% without making the code too messy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to push back a little here. I like having the type inline, be it a little bit more cluttered as it may. Editors might help alleviate the extra indirection via highlighting, but I'd like to avoid it altogether.

Copy link
Member Author

@theckman theckman Jan 11, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally getting back around to working on this PR, and this change results in a golint failure.

generator.go:287:19: should omit type func() ([]net.Interface, error) from declaration of var netInterfaces; it will be inferred from the right-hand side (golint)
var netInterfaces func() ([]net.Interface, error) = net.Interfaces
                  ^

As a result I'm just going to omit it at this time.


// Returns the hardware address.
func defaultHWAddrFunc() (net.HardwareAddr, error) {
ifaces, err := net.Interfaces()
ifaces, err := netInterfaces()
if err != nil {
return []byte{}, err
}
Expand Down
98 changes: 98 additions & 0 deletions generator_test.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import (
"bytes"
"crypto/rand"
"encoding/binary"
"errors"
"fmt"
"net"
"strings"
Expand Down Expand Up @@ -808,6 +809,103 @@ func TestPrecision_Duration(t *testing.T) {
}
}

func TestDefaultHWAddrFunc(t *testing.T) {
tests := []struct {
n string
fn func() ([]net.Interface, error)
hw net.HardwareAddr
e string
}{
{
n: "Error",
fn: func() ([]net.Interface, error) {
return nil, errors.New("controlled failure")
},
e: "controlled failure",
},
{
n: "NoValidHWAddrReturned",
fn: func() ([]net.Interface, error) {
s := []net.Interface{
{
Index: 1,
MTU: 1500,
Name: "test0",
HardwareAddr: net.HardwareAddr{1, 2, 3, 4},
},
{
Index: 2,
MTU: 1500,
Name: "lo0",
HardwareAddr: net.HardwareAddr{5, 6, 7, 8},
},
}

return s, nil
},
e: "uuid: no HW address found",
},
{
n: "ValidHWAddrReturned",
fn: func() ([]net.Interface, error) {
s := []net.Interface{
{
Index: 1,
MTU: 1500,
Name: "test0",
HardwareAddr: net.HardwareAddr{1, 2, 3, 4},
},
{
Index: 2,
MTU: 1500,
Name: "lo0",
HardwareAddr: net.HardwareAddr{5, 6, 7, 8, 9, 0},
},
}

return s, nil
},
hw: net.HardwareAddr{5, 6, 7, 8, 9, 0},
},
}

for _, tt := range tests {
t.Run(tt.n, func(t *testing.T) {
// set the netInterfaces variable (function) for the test
// and then set it back to default in the deferred function
netInterfaces = tt.fn
defer func() {
netInterfaces = net.Interfaces
}()

var hw net.HardwareAddr
acln0 marked this conversation as resolved.
Show resolved Hide resolved
var err error

hw, err = defaultHWAddrFunc()

if len(tt.e) > 0 {
if err == nil {
t.Fatalf("defaultHWAddrFunc() error = <nil>, should contain %q", tt.e)
}

if !strings.Contains(err.Error(), tt.e) {
t.Fatalf("defaultHWAddrFunc() error = %q, should contain %q", err.Error(), tt.e)
}

return
}

if err != nil && tt.e == "" {
t.Fatalf("defaultHWAddrFunc() error = %q, want <nil>", err.Error())
}

if !bytes.Equal(hw, tt.hw) {
t.Fatalf("hw = %#v, want %#v", hw, tt.hw)
}
})
}
}

func BenchmarkGenerator(b *testing.B) {
b.Run("NewV1", func(b *testing.B) {
for i := 0; i < b.N; i++ {
Expand Down