Merge pull request #252 from crawford/vet

go vet
This commit is contained in:
Alex Crawford 2014-10-23 12:03:01 -07:00
commit e9bda98b54
21 changed files with 95 additions and 69 deletions

View File

@ -5,6 +5,7 @@ go:
install:
- go get code.google.com/p/go.tools/cmd/cover
- go get code.google.com/p/go.tools/cmd/vet
script:
- ./test

View File

@ -38,7 +38,7 @@ func TestIsZero(t *testing.T) {
{struct{ A int }{A: 1}, false},
} {
if empty := IsZero(tt.c); tt.empty != empty {
t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.empty, empty)
t.Errorf("bad result (%q): want %t, got %t", tt.c, tt.empty, empty)
}
}
}
@ -373,7 +373,7 @@ users:
}
if len(cfg.Users) != 1 {
t.Fatalf("Parsed %d users, expected 1", cfg.Users)
t.Fatalf("Parsed %d users, expected 1", len(cfg.Users))
}
user := cfg.Users[0]
@ -448,7 +448,7 @@ users:
}
if len(cfg.Users) != 1 {
t.Fatalf("Parsed %d users, expected 1", cfg.Users)
t.Fatalf("Parsed %d users, expected 1", len(cfg.Users))
}
user := cfg.Users[0]
@ -474,7 +474,7 @@ users:
}
if len(cfg.Users) != 1 {
t.Fatalf("Parsed %d users, expected 1", cfg.Users)
t.Fatalf("Parsed %d users, expected 1", len(cfg.Users))
}
user := cfg.Users[0]

View File

@ -57,7 +57,7 @@ func TestFetchMetadata(t *testing.T) {
cd := configDrive{tt.root, tt.files.readFile}
filename, err := cd.FetchMetadata()
if err != nil {
t.Fatalf("bad error for %q: want %q, got %q", tt, nil, err)
t.Fatalf("bad error for %q: want %v, got %q", tt, nil, err)
}
if string(filename) != tt.filename {
t.Fatalf("bad path for %q: want %q, got %q", tt, tt.filename, filename)
@ -90,7 +90,7 @@ func TestFetchUserdata(t *testing.T) {
cd := configDrive{tt.root, tt.files.readFile}
filename, err := cd.FetchUserdata()
if err != nil {
t.Fatalf("bad error for %q: want %q, got %q", tt, nil, err)
t.Fatalf("bad error for %q: want %v, got %q", tt, nil, err)
}
if string(filename) != tt.filename {
t.Fatalf("bad path for %q: want %q, got %q", tt, tt.filename, filename)

View File

@ -86,14 +86,14 @@ func TestFetchMetadata(t *testing.T) {
expect: []byte(`{"hostname":"","public-ipv4":"192.168.1.2","public-ipv6":"fe00::","public_keys":{"0":"publickey1","1":"publickey2"}}`),
},
{
clientErr: pkg.ErrTimeout{fmt.Errorf("test error")},
expectErr: pkg.ErrTimeout{fmt.Errorf("test error")},
clientErr: pkg.ErrTimeout{Err: fmt.Errorf("test error")},
expectErr: pkg.ErrTimeout{Err: fmt.Errorf("test error")},
},
} {
service := &metadataService{
MetadataService: metadata.MetadataService{
Root: tt.root,
Client: &test.HttpClient{tt.resources, tt.clientErr},
Client: &test.HttpClient{Resources: tt.resources, Err: tt.clientErr},
MetadataPath: tt.metadataPath,
},
}

View File

@ -74,7 +74,7 @@ func TestFetchAttributes(t *testing.T) {
},
} {
service := metadataService{metadata.MetadataService{
Client: &test.HttpClient{s.resources, s.err},
Client: &test.HttpClient{Resources: s.resources, Err: s.err},
}}
for _, tt := range s.tests {
attrs, err := service.fetchAttributes(tt.path)
@ -128,7 +128,7 @@ func TestFetchAttribute(t *testing.T) {
},
} {
service := metadataService{metadata.MetadataService{
Client: &test.HttpClient{s.resources, s.err},
Client: &test.HttpClient{Resources: s.resources, Err: s.err},
}}
for _, tt := range s.tests {
attr, err := service.fetchAttribute(tt.path)
@ -174,13 +174,13 @@ func TestFetchMetadata(t *testing.T) {
expect: []byte(`{"hostname":"host","local-ipv4":"1.2.3.4","network_config":{"content_path":"path"},"public-ipv4":"5.6.7.8","public_keys":{"test1":"key"}}`),
},
{
clientErr: pkg.ErrTimeout{fmt.Errorf("test error")},
expectErr: pkg.ErrTimeout{fmt.Errorf("test error")},
clientErr: pkg.ErrTimeout{Err: fmt.Errorf("test error")},
expectErr: pkg.ErrTimeout{Err: fmt.Errorf("test error")},
},
} {
service := &metadataService{metadata.MetadataService{
Root: tt.root,
Client: &test.HttpClient{tt.resources, tt.clientErr},
Client: &test.HttpClient{Resources: tt.resources, Err: tt.clientErr},
MetadataPath: tt.metadataPath,
}}
metadata, err := service.FetchMetadata()

View File

@ -28,7 +28,7 @@ import (
func TestAvailabilityChanges(t *testing.T) {
want := true
if ac := (MetadataService{}).AvailabilityChanges(); ac != want {
t.Fatalf("bad AvailabilityChanges: want %q, got %q", want, ac)
t.Fatalf("bad AvailabilityChanges: want %t, got %t", want, ac)
}
}
@ -55,11 +55,11 @@ func TestIsAvailable(t *testing.T) {
} {
service := &MetadataService{
Root: tt.root,
Client: &test.HttpClient{tt.resources, nil},
Client: &test.HttpClient{Resources: tt.resources, Err: nil},
ApiVersion: tt.apiVersion,
}
if a := service.IsAvailable(); a != tt.expect {
t.Fatalf("bad isAvailable (%q): want %q, got %q", tt.resources, tt.expect, a)
t.Fatalf("bad isAvailable (%q): want %t, got %t", tt.resources, tt.expect, a)
}
}
}
@ -83,18 +83,18 @@ func TestFetchUserdata(t *testing.T) {
},
{
root: "/",
clientErr: pkg.ErrNotFound{fmt.Errorf("test not found error")},
clientErr: pkg.ErrNotFound{Err: fmt.Errorf("test not found error")},
userdata: []byte{},
},
{
root: "/",
clientErr: pkg.ErrTimeout{fmt.Errorf("test timeout error")},
expectErr: pkg.ErrTimeout{fmt.Errorf("test timeout error")},
clientErr: pkg.ErrTimeout{Err: fmt.Errorf("test timeout error")},
expectErr: pkg.ErrTimeout{Err: fmt.Errorf("test timeout error")},
},
} {
service := &MetadataService{
Root: tt.root,
Client: &test.HttpClient{tt.resources, tt.clientErr},
Client: &test.HttpClient{Resources: tt.resources, Err: tt.clientErr},
UserdataPath: tt.userdataPath,
}
data, err := service.FetchUserdata()

View File

@ -101,7 +101,7 @@ func TestFetchMetadata(t *testing.T) {
a := waagent{tt.root, tt.files.readFile}
metadataBytes, err := a.FetchMetadata()
if err != nil {
t.Fatalf("bad error for %q: want %q, got %q", tt, nil, err)
t.Fatalf("bad error for %q: want %v, got %q", tt, nil, err)
}
var metadata map[string]string
if len(metadataBytes) > 0 {
@ -136,7 +136,7 @@ func TestFetchUserdata(t *testing.T) {
a := waagent{tt.root, tt.files.readFile}
_, err := a.FetchUserdata()
if err != nil {
t.Fatalf("bad error for %q: want %q, got %q", tt, nil, err)
t.Fatalf("bad error for %q: want %v, got %q", tt, nil, err)
}
}
}

View File

@ -106,13 +106,13 @@ func Apply(cfg config.CloudConfig, env *Environment) error {
var writeFiles []system.File
for _, file := range cfg.WriteFiles {
writeFiles = append(writeFiles, system.File{file})
writeFiles = append(writeFiles, system.File{File: file})
}
for _, ccf := range []CloudConfigFile{
system.OEM{cfg.Coreos.OEM},
system.Update{cfg.Coreos.Update, system.DefaultReadConfig},
system.EtcHosts{cfg.ManageEtcHosts},
system.OEM{OEM: cfg.Coreos.OEM},
system.Update{Update: cfg.Coreos.Update, ReadConfig: system.DefaultReadConfig},
system.EtcHosts{EtcHosts: cfg.ManageEtcHosts},
} {
f, err := ccf.File()
if err != nil {
@ -125,13 +125,13 @@ func Apply(cfg config.CloudConfig, env *Environment) error {
var units []system.Unit
for _, u := range cfg.Coreos.Units {
units = append(units, system.Unit{u})
units = append(units, system.Unit{Unit: u})
}
for _, ccu := range []CloudConfigUnit{
system.Etcd{cfg.Coreos.Etcd},
system.Fleet{cfg.Coreos.Fleet},
system.Update{cfg.Coreos.Update, system.DefaultReadConfig},
system.Etcd{Etcd: cfg.Coreos.Etcd},
system.Fleet{Fleet: cfg.Coreos.Fleet},
system.Update{Update: cfg.Coreos.Update, ReadConfig: system.DefaultReadConfig},
} {
units = append(units, ccu.Units()...)
}

View File

@ -61,7 +61,7 @@ func (tum *TestUnitManager) UnmaskUnit(unit *system.Unit) error {
func TestProcessUnits(t *testing.T) {
tum := &TestUnitManager{}
units := []system.Unit{
system.Unit{config.Unit{
system.Unit{Unit: config.Unit{
Name: "foo",
Mask: true,
}},
@ -75,7 +75,7 @@ func TestProcessUnits(t *testing.T) {
tum = &TestUnitManager{}
units = []system.Unit{
system.Unit{config.Unit{
system.Unit{Unit: config.Unit{
Name: "bar.network",
}},
}
@ -88,7 +88,7 @@ func TestProcessUnits(t *testing.T) {
tum = &TestUnitManager{}
units = []system.Unit{
system.Unit{config.Unit{
system.Unit{Unit: config.Unit{
Name: "baz.service",
Content: "[Service]\nExecStart=/bin/true",
}},
@ -102,7 +102,7 @@ func TestProcessUnits(t *testing.T) {
tum = &TestUnitManager{}
units = []system.Unit{
system.Unit{config.Unit{
system.Unit{Unit: config.Unit{
Name: "locksmithd.service",
Runtime: true,
}},
@ -116,7 +116,7 @@ func TestProcessUnits(t *testing.T) {
tum = &TestUnitManager{}
units = []system.Unit{
system.Unit{config.Unit{
system.Unit{Unit: config.Unit{
Name: "woof",
Enable: true,
}},

View File

@ -98,7 +98,7 @@ func (e *Environment) Apply(data string) string {
func (e *Environment) DefaultEnvironmentFile() *system.EnvFile {
ef := system.EnvFile{
File: &system.File{config.File{
File: &system.File{File: config.File{
Path: "/etc/environment",
}},
Vars: map[string]string{},

View File

@ -48,7 +48,7 @@ func PersistScriptInWorkspace(script system.Script, workspace string) (string, e
relpath := strings.TrimPrefix(tmp.Name(), workspace)
file := system.File{config.File{
file := system.File{File: config.File{
Path: relpath,
RawFilePermissions: "0744",
Content: string(script),
@ -58,7 +58,7 @@ func PersistScriptInWorkspace(script system.Script, workspace string) (string, e
}
func PersistUnitNameInWorkspace(name string, workspace string) error {
file := system.File{config.File{
file := system.File{File: config.File{
Path: path.Join("scripts", "unit-name"),
RawFilePermissions: "0644",
Content: name,

View File

@ -49,7 +49,7 @@ func TestProcessDebianNetconf(t *testing.T) {
interfaces, err := ProcessDebianNetconf(tt.in)
failed := err != nil
if tt.fail != failed {
t.Fatalf("bad failure state for %q: got %b, want %b", failed, tt.fail)
t.Fatalf("bad failure state for %q: got %t, want %t", tt.in, failed, tt.fail)
}
if tt.n != -1 && tt.n != len(interfaces) {
t.Fatalf("bad number of interfaces for %q: got %d, want %q", tt.in, len(interfaces), tt.n)

View File

@ -36,11 +36,11 @@ func TestParseNameservers(t *testing.T) {
nss: []net.IP{},
},
{
dns: digitalocean.DNS{[]string{"1.2.3.4"}},
dns: digitalocean.DNS{Nameservers: []string{"1.2.3.4"}},
nss: []net.IP{net.ParseIP("1.2.3.4")},
},
{
dns: digitalocean.DNS{[]string{"bad"}},
dns: digitalocean.DNS{Nameservers: []string{"bad"}},
err: errors.New("could not parse \"bad\" as nameserver IP address"),
},
} {
@ -132,7 +132,10 @@ func TestParseInterface(t *testing.T) {
iface: &logicalInterface{
hwaddr: net.HardwareAddr([]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab}),
config: configMethodStatic{
addresses: []net.IPNet{net.IPNet{net.ParseIP("1.2.3.4"), net.IPMask(net.ParseIP("255.255.0.0"))}},
addresses: []net.IPNet{net.IPNet{
IP: net.ParseIP("1.2.3.4"),
Mask: net.IPMask(net.ParseIP("255.255.0.0")),
}},
nameservers: []net.IP{},
routes: []route{},
},
@ -165,9 +168,15 @@ func TestParseInterface(t *testing.T) {
iface: &logicalInterface{
hwaddr: net.HardwareAddr([]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab}),
config: configMethodStatic{
addresses: []net.IPNet{net.IPNet{net.ParseIP("1.2.3.4"), net.IPMask(net.ParseIP("255.255.0.0"))}},
addresses: []net.IPNet{net.IPNet{
IP: net.ParseIP("1.2.3.4"),
Mask: net.IPMask(net.ParseIP("255.255.0.0")),
}},
nameservers: []net.IP{},
routes: []route{route{net.IPNet{net.IPv4zero, net.IPMask(net.IPv4zero)}, net.ParseIP("5.6.7.8")}},
routes: []route{route{
net.IPNet{IP: net.IPv4zero, Mask: net.IPMask(net.IPv4zero)},
net.ParseIP("5.6.7.8"),
}},
},
},
},
@ -195,7 +204,10 @@ func TestParseInterface(t *testing.T) {
iface: &logicalInterface{
hwaddr: net.HardwareAddr([]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab}),
config: configMethodStatic{
addresses: []net.IPNet{net.IPNet{net.ParseIP("fe00::"), net.IPMask(net.ParseIP("ffff::"))}},
addresses: []net.IPNet{net.IPNet{
IP: net.ParseIP("fe00::"),
Mask: net.IPMask(net.ParseIP("ffff::")),
}},
nameservers: []net.IP{},
routes: []route{},
},
@ -228,9 +240,15 @@ func TestParseInterface(t *testing.T) {
iface: &logicalInterface{
hwaddr: net.HardwareAddr([]byte{0x01, 0x23, 0x45, 0x67, 0x89, 0xab}),
config: configMethodStatic{
addresses: []net.IPNet{net.IPNet{net.ParseIP("fe00::"), net.IPMask(net.ParseIP("ffff::"))}},
addresses: []net.IPNet{net.IPNet{
IP: net.ParseIP("fe00::"),
Mask: net.IPMask(net.ParseIP("ffff::")),
}},
nameservers: []net.IP{},
routes: []route{route{net.IPNet{net.IPv6zero, net.IPMask(net.IPv6zero)}, net.ParseIP("fe00:1234::")}},
routes: []route{route{
net.IPNet{IP: net.IPv6zero, Mask: net.IPMask(net.IPv6zero)},
net.ParseIP("fe00:1234::"),
}},
},
},
},

View File

@ -98,7 +98,7 @@ func TestGetURL4xx(t *testing.T) {
}
if retries > 1 {
t.Errorf("Number of retries:\n%d\nExpected number of retries:\n%s", retries, 1)
t.Errorf("Number of retries:\n%d\nExpected number of retries:\n%d", retries, 1)
}
}

View File

@ -28,11 +28,11 @@ import (
const DefaultIpv4Address = "127.0.0.1"
type EtcHosts struct {
Config config.EtcHosts
config.EtcHosts
}
func (eh EtcHosts) generateEtcHosts() (out string, err error) {
if eh.Config != "localhost" {
if eh.EtcHosts != "localhost" {
return "", errors.New("Invalid option to manage_etc_hosts")
}
@ -47,7 +47,7 @@ func (eh EtcHosts) generateEtcHosts() (out string, err error) {
}
func (eh EtcHosts) File() (*File, error) {
if eh.Config == "" {
if eh.EtcHosts == "" {
return nil, nil
}

View File

@ -48,7 +48,7 @@ Environment="FLEET_PUBLIC_IP=12.34.56.78"
} {
units := Fleet{tt.config}.Units()
if !reflect.DeepEqual(units, tt.units) {
t.Errorf("bad units (%q): want %q, got %q", tt.config, tt.units, units)
t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.units, units)
}
}
}

View File

@ -54,7 +54,7 @@ BUG_REPORT_URL="https://github.com/coreos/coreos-overlay"
} {
file, err := OEM{tt.config}.File()
if err != nil {
t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err)
t.Errorf("bad error (%q): want %v, got %q", tt.config, nil, err)
}
if !reflect.DeepEqual(tt.file, file) {
t.Errorf("bad file (%q): want %#v, got %#v", tt.config, tt.file, file)

View File

@ -187,10 +187,10 @@ func TestMaskUnit(t *testing.T) {
fooPath := path.Join(dir, "etc", "systemd", "system", "foo.service")
fooTgt, err := os.Readlink(fooPath)
if err != nil {
t.Fatalf("Unable to read link", err)
t.Fatal("Unable to read link", err)
}
if fooTgt != "/dev/null" {
t.Fatalf("unit not masked, got unit target", fooTgt)
t.Fatal("unit not masked, got unit target", fooTgt)
}
// Ensure mask works with unit files that already exist
@ -204,10 +204,10 @@ func TestMaskUnit(t *testing.T) {
}
barTgt, err := os.Readlink(barPath)
if err != nil {
t.Fatalf("Unable to read link", err)
t.Fatal("Unable to read link", err)
}
if barTgt != "/dev/null" {
t.Fatalf("unit not masked, got unit target", barTgt)
t.Fatal("unit not masked, got unit target", barTgt)
}
}
@ -254,7 +254,7 @@ func TestUnmaskUnit(t *testing.T) {
t.Errorf("unmask of unit returned unexpected error: %v", err)
}
if _, err := os.Stat(dst); !os.IsNotExist(err) {
t.Errorf("expected %s to not exist after unmask, but got err: %s", err)
t.Errorf("expected %s to not exist after unmask, but got err: %s", dst, err)
}
}

View File

@ -39,8 +39,8 @@ const (
// implementation reading from the filesystem), and provides the system-specific
// File() and Unit().
type Update struct {
Config config.Update
ReadConfig func() (io.Reader, error)
config.Update
}
func DefaultReadConfig() (io.Reader, error) {
@ -58,17 +58,17 @@ func DefaultReadConfig() (io.Reader, error) {
// configuration options are set in cloud-config) by either rewriting the
// existing file on disk, or starting from `/usr/share/coreos/update.conf`
func (uc Update) File() (*File, error) {
if config.IsZero(uc.Config) {
if config.IsZero(uc.Update) {
return nil, nil
}
if err := config.AssertValid(uc.Config); err != nil {
if err := config.AssertValid(uc.Update); err != nil {
return nil, err
}
// Generate the list of possible substitutions to be performed based on the options that are configured
subs := map[string]string{}
uct := reflect.TypeOf(uc.Config)
ucv := reflect.ValueOf(uc.Config)
uct := reflect.TypeOf(uc.Update)
ucv := reflect.ValueOf(uc.Update)
for i := 0; i < uct.NumField(); i++ {
val := ucv.Field(i).String()
if val == "" {
@ -118,7 +118,7 @@ func (uc Update) File() (*File, error) {
// - an update_engine Unit, if "group" or "server" was set in cloud-config
func (uc Update) Units() []Unit {
var units []Unit
if uc.Config.RebootStrategy != "" {
if uc.Update.RebootStrategy != "" {
ls := &Unit{config.Unit{
Name: locksmithUnit,
Command: "restart",
@ -126,14 +126,14 @@ func (uc Update) Units() []Unit {
Runtime: true,
}}
if uc.Config.RebootStrategy == "off" {
if uc.Update.RebootStrategy == "off" {
ls.Command = "stop"
ls.Mask = true
}
units = append(units, *ls)
}
if uc.Config.Group != "" || uc.Config.Server != "" {
if uc.Update.Group != "" || uc.Update.Server != "" {
ue := Unit{config.Unit{
Name: updateEngineUnit,
Command: "restart",

View File

@ -82,7 +82,7 @@ func TestUpdateUnits(t *testing.T) {
}}},
},
} {
units := Update{tt.config, testReadConfig("")}.Units()
units := Update{Update: tt.config, ReadConfig: testReadConfig("")}.Units()
if !reflect.DeepEqual(tt.units, units) {
t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.units, units)
}
@ -153,7 +153,7 @@ func TestUpdateFile(t *testing.T) {
}},
},
} {
file, err := Update{tt.config, testReadConfig(tt.orig)}.File()
file, err := Update{Update: tt.config, ReadConfig: testReadConfig(tt.orig)}.File()
if !reflect.DeepEqual(tt.err, err) {
t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err)
}

7
test
View File

@ -54,4 +54,11 @@ if [ -n "$fmtRes" ]; then
exit 1
fi
echo "Checking govet..."
vetRes=$(go vet $TESTPKGS)
if [ -n "${vetRes}" ]; then
echo -e "govet checking failed:\n${vetRes}"
exit 255
fi
echo "Success"