diff --git a/Makefile b/Makefile index f03b010f5..d0d67dab8 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,6 @@ TEST?=./... +VETARGS?=-asmdecl -atomic -bool -buildtags -copylocks -methods \ + -nilfunc -printf -rangeloops -shift -structtags -unsafeptr default: test @@ -10,6 +12,7 @@ dev: test: go test $(TEST) $(TESTARGS) -timeout=10s + @$(MAKE) vet testrace: go test -race $(TEST) $(TESTARGS) @@ -17,4 +20,14 @@ testrace: updatedeps: go get -d -v -p 2 ./... -.PHONY: bin default test updatedeps +vet: + @go tool vet 2>/dev/null ; if [ $$? -eq 3 ]; then \ + go get golang.org/x/tools/cmd/vet; \ + fi + @go tool vet $(VETARGS) . ; if [ $$? -eq 1 ]; then \ + echo ""; \ + echo "Vet found suspicious constructs. Please check the reported constructs"; \ + echo "and fix them if necessary before submitting the code for reviewal."; \ + fi + +.PHONY: bin default test updatedeps vet diff --git a/builder/amazon/common/step_ami_region_copy.go b/builder/amazon/common/step_ami_region_copy.go index 6e61b40d1..898eacc08 100644 --- a/builder/amazon/common/step_ami_region_copy.go +++ b/builder/amazon/common/step_ami_region_copy.go @@ -79,7 +79,7 @@ func amiRegionCopy(state multistep.StateBag, auth aws.Auth, imageId string, if err != nil { return "", fmt.Errorf("Error Copying AMI (%s) to region (%s): %s", - imageId, target, err) + imageId, target.Name, err) } stateChange := StateChangeConf{ @@ -91,7 +91,7 @@ func amiRegionCopy(state multistep.StateBag, auth aws.Auth, imageId string, if _, err := WaitForState(&stateChange); err != nil { return "", fmt.Errorf("Error waiting for AMI (%s) in region (%s): %s", - resp.ImageId, target, err) + resp.ImageId, target.Name, err) } return resp.ImageId, nil diff --git a/builder/amazon/ebs/step_create_ami.go b/builder/amazon/ebs/step_create_ami.go index 967ad2d39..f380ea0b1 100644 --- a/builder/amazon/ebs/step_create_ami.go +++ b/builder/amazon/ebs/step_create_ami.go @@ -87,7 +87,7 @@ func (s *stepCreateAMI) Cleanup(state multistep.StateBag) { ui.Error(fmt.Sprintf("Error deregistering AMI, may still be around: %s", err)) return } else if resp.Return == false { - ui.Error(fmt.Sprintf("Error deregistering AMI, may still be around: %s", resp.Return)) + ui.Error(fmt.Sprintf("Error deregistering AMI, may still be around: %t", resp.Return)) return } } diff --git a/builder/digitalocean/builder_test.go b/builder/digitalocean/builder_test.go index d6857bbe6..bd3bb1d21 100644 --- a/builder/digitalocean/builder_test.go +++ b/builder/digitalocean/builder_test.go @@ -264,7 +264,7 @@ func TestBuilderPrepare_SSHUsername(t *testing.T) { } if b.config.SSHUsername != "root" { - t.Errorf("invalid: %d", b.config.SSHUsername) + t.Errorf("invalid: %s", b.config.SSHUsername) } // Test set @@ -297,7 +297,7 @@ func TestBuilderPrepare_SSHTimeout(t *testing.T) { } if b.config.RawSSHTimeout != "1m" { - t.Errorf("invalid: %d", b.config.RawSSHTimeout) + t.Errorf("invalid: %s", b.config.RawSSHTimeout) } // Test set @@ -338,7 +338,7 @@ func TestBuilderPrepare_StateTimeout(t *testing.T) { } if b.config.RawStateTimeout != "6m" { - t.Errorf("invalid: %d", b.config.RawStateTimeout) + t.Errorf("invalid: %s", b.config.RawStateTimeout) } // Test set @@ -379,7 +379,7 @@ func TestBuilderPrepare_PrivateNetworking(t *testing.T) { } if b.config.PrivateNetworking != false { - t.Errorf("invalid: %s", b.config.PrivateNetworking) + t.Errorf("invalid: %t", b.config.PrivateNetworking) } // Test set @@ -394,7 +394,7 @@ func TestBuilderPrepare_PrivateNetworking(t *testing.T) { } if b.config.PrivateNetworking != true { - t.Errorf("invalid: %s", b.config.PrivateNetworking) + t.Errorf("invalid: %t", b.config.PrivateNetworking) } } diff --git a/builder/googlecompute/step_create_instance_test.go b/builder/googlecompute/step_create_instance_test.go index c7272bea8..baae10849 100644 --- a/builder/googlecompute/step_create_instance_test.go +++ b/builder/googlecompute/step_create_instance_test.go @@ -39,7 +39,7 @@ func TestStepCreateInstance(t *testing.T) { t.Fatal("should've deleted instance") } if driver.DeleteInstanceZone != config.Zone { - t.Fatal("bad zone: %#v", driver.DeleteInstanceZone) + t.Fatalf("bad zone: %#v", driver.DeleteInstanceZone) } } diff --git a/builder/googlecompute/step_teardown_instance_test.go b/builder/googlecompute/step_teardown_instance_test.go index f007d5179..bd32ea26d 100644 --- a/builder/googlecompute/step_teardown_instance_test.go +++ b/builder/googlecompute/step_teardown_instance_test.go @@ -26,7 +26,7 @@ func TestStepTeardownInstance(t *testing.T) { t.Fatal("should've deleted instance") } if driver.DeleteInstanceZone != config.Zone { - t.Fatal("bad zone: %#v", driver.DeleteInstanceZone) + t.Fatalf("bad zone: %#v", driver.DeleteInstanceZone) } // cleanup @@ -36,6 +36,6 @@ func TestStepTeardownInstance(t *testing.T) { t.Fatal("should've deleted disk") } if driver.DeleteDiskZone != config.Zone { - t.Fatal("bad zone: %#v", driver.DeleteDiskZone) + t.Fatalf("bad zone: %#v", driver.DeleteDiskZone) } } diff --git a/builder/openstack/artifact.go b/builder/openstack/artifact.go index f55ff8727..6e75fad3e 100644 --- a/builder/openstack/artifact.go +++ b/builder/openstack/artifact.go @@ -41,6 +41,6 @@ func (a *Artifact) State(name string) interface{} { } func (a *Artifact) Destroy() error { - log.Printf("Destroying image: %d", a.ImageId) + log.Printf("Destroying image: %s", a.ImageId) return a.Conn.DeleteImageById(a.ImageId) } diff --git a/builder/openstack/step_wait_for_rackconnect.go b/builder/openstack/step_wait_for_rackconnect.go index 94affbe37..ee6ee6138 100644 --- a/builder/openstack/step_wait_for_rackconnect.go +++ b/builder/openstack/step_wait_for_rackconnect.go @@ -21,7 +21,6 @@ func (s *StepWaitForRackConnect) Run(state multistep.StateBag) multistep.StepAct csp := state.Get("csp").(gophercloud.CloudServersProvider) server := state.Get("server").(*gophercloud.Server) ui := state.Get("ui").(packer.Ui) - fmt.Printf("%s", server) ui.Say(fmt.Sprintf("Waiting for server (%s) to become RackConnect ready...", server.Id)) diff --git a/builder/parallels/iso/builder_test.go b/builder/parallels/iso/builder_test.go index 9c7f56f6b..b7d4cac50 100644 --- a/builder/parallels/iso/builder_test.go +++ b/builder/parallels/iso/builder_test.go @@ -75,7 +75,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { } if b.config.DiskSize != 60000 { - t.Fatalf("bad size: %s", b.config.DiskSize) + t.Fatalf("bad size: %d", b.config.DiskSize) } } diff --git a/builder/qemu/builder_test.go b/builder/qemu/builder_test.go index 86c561263..e3415d514 100644 --- a/builder/qemu/builder_test.go +++ b/builder/qemu/builder_test.go @@ -160,7 +160,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { } if b.config.DiskSize != 60000 { - t.Fatalf("bad size: %s", b.config.DiskSize) + t.Fatalf("bad size: %d", b.config.DiskSize) } } diff --git a/builder/virtualbox/iso/builder_test.go b/builder/virtualbox/iso/builder_test.go index 3b8e59569..714587f24 100644 --- a/builder/virtualbox/iso/builder_test.go +++ b/builder/virtualbox/iso/builder_test.go @@ -83,7 +83,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { } if b.config.DiskSize != 60000 { - t.Fatalf("bad size: %s", b.config.DiskSize) + t.Fatalf("bad size: %d", b.config.DiskSize) } } diff --git a/builder/vmware/common/step_shutdown_test.go b/builder/vmware/common/step_shutdown_test.go index b3c13a9f6..cdbda852d 100644 --- a/builder/vmware/common/step_shutdown_test.go +++ b/builder/vmware/common/step_shutdown_test.go @@ -127,7 +127,7 @@ func TestStepShutdown_locks(t *testing.T) { lockPath := filepath.Join(dir.dir, "nope.lck") err := ioutil.WriteFile(lockPath, []byte("foo"), 0644) if err != nil { - t.Fatalf("err: %s") + t.Fatalf("err: %s", err) } // Remove the lock file after a certain time diff --git a/builder/vmware/iso/builder_test.go b/builder/vmware/iso/builder_test.go index b6c0cfd81..1749b396e 100644 --- a/builder/vmware/iso/builder_test.go +++ b/builder/vmware/iso/builder_test.go @@ -175,7 +175,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) { } if b.config.DiskSize != 60000 { - t.Fatalf("bad size: %s", b.config.DiskSize) + t.Fatalf("bad size: %d", b.config.DiskSize) } } diff --git a/common/step_create_floppy_test.go b/common/step_create_floppy_test.go index b6e591a41..d1a3000a7 100644 --- a/common/step_create_floppy_test.go +++ b/common/step_create_floppy_test.go @@ -76,7 +76,7 @@ func TestStepCreateFloppy(t *testing.T) { floppy_path := state.Get("floppy_path").(string) if _, err := os.Stat(floppy_path); err != nil { - t.Fatal("file not found: %s for %v", floppy_path, step.Files) + t.Fatalf("file not found: %s for %v", floppy_path, step.Files) } if len(step.FilesAdded) != expected { @@ -86,7 +86,7 @@ func TestStepCreateFloppy(t *testing.T) { step.Cleanup(state) if _, err := os.Stat(floppy_path); err == nil { - t.Fatal("file found: %s for %v", floppy_path, step.Files) + t.Fatalf("file found: %s for %v", floppy_path, step.Files) } } } @@ -177,7 +177,7 @@ func xxxTestStepCreateFloppy_notfound(t *testing.T) { floppy_path := state.Get("floppy_path").(string) if _, err := os.Stat(floppy_path); err != nil { - t.Fatal("file not found: %s for %v", floppy_path, step.Files) + t.Fatalf("file not found: %s for %v", floppy_path, step.Files) } if len(step.FilesAdded) != expected { @@ -187,7 +187,7 @@ func xxxTestStepCreateFloppy_notfound(t *testing.T) { step.Cleanup(state) if _, err := os.Stat(floppy_path); err == nil { - t.Fatal("file found: %s for %v", floppy_path, step.Files) + t.Fatalf("file found: %s for %v", floppy_path, step.Files) } } } diff --git a/communicator/ssh/communicator_test.go b/communicator/ssh/communicator_test.go index c5b8832dd..aa241cca8 100644 --- a/communicator/ssh/communicator_test.go +++ b/communicator/ssh/communicator_test.go @@ -83,10 +83,10 @@ func newMockLineServer(t *testing.T) string { } t.Log("Accepted channel") - go func() { + go func(channelType string) { defer channel.Close() - conn.OpenChannel(newChannel.ChannelType(), nil) - }() + conn.OpenChannel(channelType, nil) + }(newChannel.ChannelType()) } conn.Close() }() diff --git a/packer/template_test.go b/packer/template_test.go index bf11fc7d0..b676672b1 100644 --- a/packer/template_test.go +++ b/packer/template_test.go @@ -497,7 +497,7 @@ func TestParseTemplate_Provisioners(t *testing.T) { result, err := ParseTemplate([]byte(data), nil) if err != nil { - t.Fatal("err: %s", err) + t.Fatalf("err: %s", err) } if result == nil { t.Fatal("should have result") @@ -529,7 +529,7 @@ func TestParseTemplate_ProvisionerPauseBefore(t *testing.T) { result, err := ParseTemplate([]byte(data), nil) if err != nil { - t.Fatal("err: %s", err) + t.Fatalf("err: %s", err) } if result == nil { t.Fatal("should have result") diff --git a/post-processor/vagrant-cloud/client.go b/post-processor/vagrant-cloud/client.go index 104f13832..0c475075d 100644 --- a/post-processor/vagrant-cloud/client.go +++ b/post-processor/vagrant-cloud/client.go @@ -78,7 +78,7 @@ func (v VagrantCloudClient) Get(path string) (*http.Response, error) { req.Header.Add("Content-Type", "application/json") resp, err := v.client.Do(req) - log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp) + log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp) return resp, err } @@ -96,7 +96,7 @@ func (v VagrantCloudClient) Delete(path string) (*http.Response, error) { req.Header.Add("Content-Type", "application/json") resp, err := v.client.Do(req) - log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp) + log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp) return resp, err } @@ -128,7 +128,7 @@ func (v VagrantCloudClient) Upload(path string, url string) (*http.Response, err resp, err := v.client.Do(request) - log.Printf("Post-Processor Vagrant Cloud Upload Response: \n\n%s", resp) + log.Printf("Post-Processor Vagrant Cloud Upload Response: \n\n%+v", resp) return resp, err } @@ -153,7 +153,7 @@ func (v VagrantCloudClient) Post(path string, body interface{}) (*http.Response, resp, err := v.client.Do(req) - log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp) + log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp) return resp, err } @@ -172,7 +172,7 @@ func (v VagrantCloudClient) Put(path string) (*http.Response, error) { resp, err := v.client.Do(req) - log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp) + log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp) return resp, err } diff --git a/post-processor/vagrant/post-processor_test.go b/post-processor/vagrant/post-processor_test.go index 5d2d9ce2f..1bcb50be3 100644 --- a/post-processor/vagrant/post-processor_test.go +++ b/post-processor/vagrant/post-processor_test.go @@ -132,7 +132,7 @@ func TestPostProcessorPrepare_vagrantfileTemplateExists(t *testing.T) { c["vagrantfile_template"] = name if err := f.Close(); err != nil { - t.Fatal("err: %s", err) + t.Fatalf("err: %s", err) } if err := os.Remove(name); err != nil { diff --git a/provisioner/puppet-masterless/provisioner.go b/provisioner/puppet-masterless/provisioner.go index 307ecce38..efb5e53e2 100644 --- a/provisioner/puppet-masterless/provisioner.go +++ b/provisioner/puppet-masterless/provisioner.go @@ -190,7 +190,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { fmt.Errorf("module_path[%d] is invalid: %s", i, err)) } else if !info.IsDir() { errs = packer.MultiErrorAppend(errs, - fmt.Errorf("module_path[%d] must point to a directory")) + fmt.Errorf("module_path[%d] must point to a directory", i)) } } diff --git a/provisioner/salt-masterless/provisioner.go b/provisioner/salt-masterless/provisioner.go index 07710cdd5..1e2877fe3 100644 --- a/provisioner/salt-masterless/provisioner.go +++ b/provisioner/salt-masterless/provisioner.go @@ -118,14 +118,14 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { } ui.Message(fmt.Sprintf("Downloading saltstack bootstrap to /tmp/install_salt.sh")) if err = cmd.StartWithUi(comm, ui); err != nil { - return fmt.Errorf("Unable to download Salt: %d", err) + return fmt.Errorf("Unable to download Salt: %s", err) } cmd = &packer.RemoteCmd{ Command: fmt.Sprintf("sudo sh /tmp/install_salt.sh %s", p.config.BootstrapArgs), } - ui.Message(fmt.Sprintf("Installing Salt with command %s", cmd)) + ui.Message(fmt.Sprintf("Installing Salt with command %s", cmd.Command)) if err = cmd.StartWithUi(comm, ui); err != nil { - return fmt.Errorf("Unable to install Salt: %d", err) + return fmt.Errorf("Unable to install Salt: %s", err) } } @@ -146,7 +146,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { src = filepath.ToSlash(filepath.Join(p.config.TempConfigDir, "minion")) dst = "/etc/salt/minion" if err = p.moveFile(ui, comm, dst, src); err != nil { - return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %d", p.config.TempConfigDir, err) + return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %s", p.config.TempConfigDir, err) } } @@ -161,7 +161,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { src = filepath.ToSlash(filepath.Join(p.config.TempConfigDir, "states")) dst = "/srv/salt" if err = p.moveFile(ui, comm, dst, src); err != nil { - return fmt.Errorf("Unable to move %s/states to /srv/salt: %d", p.config.TempConfigDir, err) + return fmt.Errorf("Unable to move %s/states to /srv/salt: %s", p.config.TempConfigDir, err) } if p.config.LocalPillarRoots != "" { @@ -176,7 +176,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error { src = filepath.ToSlash(filepath.Join(p.config.TempConfigDir, "pillar")) dst = "/srv/pillar" if err = p.moveFile(ui, comm, dst, src); err != nil { - return fmt.Errorf("Unable to move %s/pillar to /srv/pillar: %d", p.config.TempConfigDir, err) + return fmt.Errorf("Unable to move %s/pillar to /srv/pillar: %s", p.config.TempConfigDir, err) } } @@ -220,7 +220,7 @@ func (p *Provisioner) moveFile(ui packer.Ui, comm packer.Communicator, dst, src err = fmt.Errorf("Bad exit status: %d", cmd.ExitStatus) } - return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %d", p.config.TempConfigDir, err) + return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %s", p.config.TempConfigDir, err) } return nil }