Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fix error formatting based on best practices from Code Review Comments #1734

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/deserialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func assertErrorResponse(rr *httptest.ResponseRecorder, code int) error {
ctypes := rr.HeaderMap["Content-Type"]
expect := []string{"application/json"}
if !reflect.DeepEqual(expect, ctypes) {
return fmt.Errorf("Expected Content-Type %v, got %v", expect, ctypes)
return fmt.Errorf("expected Content-Type %v, got %v", expect, ctypes)
}

var eresp errorResponse
Expand Down
18 changes: 9 additions & 9 deletions fleetctl/fleetctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ func lazyCreateUnits(cCmd *cobra.Command, args []string) error {
}

if haserr {
return fmt.Errorf("One or more errors creating units")
return fmt.Errorf("one or more errors creating units")
}

return nil
Expand Down Expand Up @@ -1030,10 +1030,10 @@ func assertUnitState(name string, js job.JobState, out io.Writer) bool {

u, err := cAPI.Unit(name)
if err != nil {
return fmt.Errorf("Error retrieving Unit(%s) from Registry: %v", name, err)
return fmt.Errorf("error retrieving Unit(%s) from Registry: %v", name, err)
}
if u == nil {
return fmt.Errorf("Unit %s not found", name)
return fmt.Errorf("unit %s not found", name)
}

// If this is a global unit, CurrentState will never be set. Instead, wait for DesiredState.
Expand All @@ -1044,7 +1044,7 @@ func assertUnitState(name string, js job.JobState, out io.Writer) bool {
}

if job.JobState(state) != js {
return fmt.Errorf("Waiting for Unit(%s) state(%s) to be %s", name, job.JobState(state), js)
return fmt.Errorf("waiting for Unit(%s) state(%s) to be %s", name, job.JobState(state), js)
}

msg := fmt.Sprintf("Unit %s %s", name, u.CurrentState)
Expand Down Expand Up @@ -1139,19 +1139,19 @@ func assertSystemdActiveState(unitName string) error {
fetchSystemdActiveState := func() error {
us, err := cAPI.UnitState(unitName)
if err != nil {
return fmt.Errorf("Error getting unit state of %s: %v", unitName, err)
return fmt.Errorf("error getting unit state of %s: %v", unitName, err)
}

// Get systemd state and check the state is active & loaded.
if us.SystemdActiveState != "active" || us.SystemdLoadState != "loaded" {
return fmt.Errorf("Failed to find an active unit %s", unitName)
return fmt.Errorf("failed to find an active unit %s", unitName)
}
return nil
}

timeout, err := waitForState(fetchSystemdActiveState)
if err != nil {
return fmt.Errorf("Failed to find an active unit %s within %v, err: %v",
return fmt.Errorf("failed to find an active unit %s within %v, err: %v",
unitName, timeout, err)
}

Expand Down Expand Up @@ -1236,7 +1236,7 @@ func waitForState(stateCheckFunc func() error) (time.Duration, error) {
for {
select {
case <-alarm:
return timeout, fmt.Errorf("Failed to fetch states within %v", timeout)
return timeout, fmt.Errorf("failed to fetch states within %v", timeout)
case <-ticker:
err := stateCheckFunc()
if err == nil {
Expand Down Expand Up @@ -1270,7 +1270,7 @@ func cmdGlobalMachineState(cCmd *cobra.Command, globalUnits []schema.Unit) (err
for id, name := range resultIDs {
// run a correspondent systemctl command
if exitVal := runCommand(cCmd, id, "systemctl", cmd, name); exitVal != 0 {
err = fmt.Errorf("Error running systemctl %s. machine id=%v, unit name=%s",
err = fmt.Errorf("error running systemctl %s. machine id=%v, unit name=%s",
cmd, id, name)
break
}
Expand Down
4 changes: 2 additions & 2 deletions fleetctl/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import (
func checkLoadUnitState(unit schema.Unit, loadRet int, errchan chan error) {
if loadRet == 0 {
if job.JobState(unit.DesiredState) != job.JobStateLoaded {
errchan <- fmt.Errorf("Error: unit %s was not loaded as requested", unit.Name)
errchan <- fmt.Errorf("error: unit %s was not loaded as requested", unit.Name)
}
} else if unit.DesiredState != "" {
// if the whole load operation failed, then no unit
// should have a DesiredState set
errchan <- fmt.Errorf("Error: Unit(%s) DesiredState was set to (%s)", unit.Name, unit.DesiredState)
errchan <- fmt.Errorf("error: Unit(%s) DesiredState was set to (%s)", unit.Name, unit.DesiredState)
}
}

Expand Down
2 changes: 1 addition & 1 deletion fleetctl/restart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func checkRestartUnitState(unit schema.Unit, restartRet int, errchan chan error)
if restartRet != 0 && unit.DesiredState != "" {
// if the whole restart operation failed, then no unit
// should have a DesiredState set
errchan <- fmt.Errorf("Error: Unit(%s) DesiredState was set to (%s)", unit.Name, unit.DesiredState)
errchan <- fmt.Errorf("error: Unit(%s) DesiredState was set to (%s)", unit.Name, unit.DesiredState)
}
}

Expand Down
4 changes: 2 additions & 2 deletions fleetctl/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import (
func checkStartUnitState(unit schema.Unit, startRet int, errchan chan error) {
if startRet == 0 {
if job.JobState(unit.DesiredState) != job.JobStateLaunched {
errchan <- fmt.Errorf("Error: unit %s was not started as requested", unit.Name)
errchan <- fmt.Errorf("error: unit %s was not started as requested", unit.Name)
}
} else if unit.DesiredState != "" {
// if the whole start operation failed, then no unit
// should have a DesiredState set
errchan <- fmt.Errorf("Error: Unit(%s) DesiredState was set to (%s)", unit.Name, unit.DesiredState)
errchan <- fmt.Errorf("error: Unit(%s) DesiredState was set to (%s)", unit.Name, unit.DesiredState)
}
}

Expand Down
2 changes: 1 addition & 1 deletion fleetctl/stop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func doStopUnits(t *testing.T, r commandTestResults, errchan chan error) {
// We assume that we reached the desired state
for _, v := range real_units {
if job.JobState(v.DesiredState) != job.JobStateLoaded {
errchan <- fmt.Errorf("Error: unit %s was not stopped as requested", v.Name)
errchan <- fmt.Errorf("error: unit %s was not stopped as requested", v.Name)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion fleetctl/unload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func doUnloadUnits(t *testing.T, r commandTestResults, errchan chan error) {
// We assume that we reached the desired state
for _, v := range real_units {
if job.JobState(v.DesiredState) != job.JobStateInactive {
errchan <- fmt.Errorf("Error: unit %s was not unloaded as requested", v.Name)
errchan <- fmt.Errorf("error: unit %s was not unloaded as requested", v.Name)
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions functional/machines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,16 +453,16 @@ func TestMachinesPatchBad(t *testing.T) {
func getHTTPResponse(method string, urlPath string, val string) (*http.Response, error) {
req, err := http.NewRequest(method, urlPath, strings.NewReader(val))
if err != nil {
return nil, fmt.Errorf("Failed creating http.Request: %v", err)
return nil, fmt.Errorf("failed creating http.Request: %v", err)
}

cl := http.Client{}
resp, err := cl.Do(req)
if err != nil {
return nil, fmt.Errorf("Failed to run client.Do: %v", err)
return nil, fmt.Errorf("failed to run client.Do: %v", err)
}
if resp.Body == nil {
return nil, fmt.Errorf("Got HTTP response nil body")
return nil, fmt.Errorf("got HTTP response nil body")
}

return resp, nil
Expand All @@ -471,9 +471,9 @@ func getHTTPResponse(method string, urlPath string, val string) (*http.Response,
func checkContentType(resp *http.Response) error {
ct := resp.Header.Get("Content-Type")
if len(ct) == 0 {
return fmt.Errorf("Response has wrong number of Content-Type values: %v", ct)
return fmt.Errorf("response has wrong number of Content-Type values: %v", ct)
} else if !strings.Contains(ct, "application/json") {
return fmt.Errorf("Expected application/json, got %v", ct)
return fmt.Errorf("expected application/json, got %v", ct)
}
return nil
}
6 changes: 3 additions & 3 deletions functional/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,16 @@ func TestDetectMachineId(t *testing.T) {
restartFleetService := func(m platform.Member) error {
stdout, stderr, err := cluster.MemberCommand(m, "sudo", "systemctl", "restart", "fleet.service")
if err != nil {
return fmt.Errorf("Failed to restart fleet service\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
return fmt.Errorf("failed to restart fleet service\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

stdout, stderr, err = cluster.MemberCommand(m, "systemctl", "show", "--property=ActiveState", "fleet")
if strings.TrimSpace(stdout) != "ActiveState=active" {
return fmt.Errorf("Fleet unit not reported as active:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
return fmt.Errorf("fleet unit not reported as active:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout, stderr, err = cluster.MemberCommand(m, "systemctl", "show", "--property=Result", "fleet")
if strings.TrimSpace(stdout) != "Result=success" {
return fmt.Errorf("Result for fleet unit not reported as success:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
return fmt.Errorf("result for fleet unit not reported as success:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion functional/platform/nspawn.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ func (nc *nspawnCluster) createMember(id string) (m Member, err error) {
for {
select {
case <-alarm:
err = fmt.Errorf("Timed out waiting for machine to start")
err = fmt.Errorf("timed out waiting for machine to start")
log.Printf("Starting %s%s failed: %v", nc.name, nm.ID(), err)
return
default:
Expand Down
8 changes: 4 additions & 4 deletions functional/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,14 @@ func waitForReloadConfig(cluster platform.Cluster, m0 platform.Member) (err erro
stdout, _, _ := cluster.MemberCommand(m0, "sudo", "journalctl --priority=info _PID=$(pidof fleetd)")
journalfleet := strings.TrimSpace(stdout)
if !strings.Contains(journalfleet, "Reloading configuration") {
fmt.Errorf("Fleetd is not fully reconfigured, retrying... entire fleet journal:\n%v", journalfleet)
fmt.Errorf("fleetd is not fully reconfigured, retrying... entire fleet journal:\n%v", journalfleet)
return false
}
return true
},
)
if err != nil {
return fmt.Errorf("Reloading configuration log not found: %v", err)
return fmt.Errorf("reloading configuration log not found: %v", err)
}

return nil
Expand All @@ -146,14 +146,14 @@ func waitForFleetdSocket(cluster platform.Cluster, m0 platform.Member) (err erro
func() bool {
stdout, _, _ := cluster.MemberCommand(m0, "test -S /var/run/fleet.sock && echo 1")
if strings.TrimSpace(stdout) == "" {
fmt.Errorf("Fleetd is not fully started, retrying...")
fmt.Errorf("fleetd is not fully started, retrying")
return false
}
return true
},
)
if err != nil {
return fmt.Errorf("Fleetd socket not found: %v", err)
return fmt.Errorf("fleetd socket not found: %v", err)
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion functional/systemd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func waitForUnitState(mgr unit.UnitManager, name string, want unit.UnitState) er
for {
select {
case <-timeout:
return fmt.Errorf("Timed out waiting for state of %s to match %#v", name, want)
return fmt.Errorf("timed out waiting for state of %s to match %#v", name, want)
default:
}

Expand Down
38 changes: 19 additions & 19 deletions functional/unit_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,21 +485,21 @@ func replaceUnitCommon(t *testing.T, cmd string, numRUnits int) error {
tmpHelloFixture := fmt.Sprintf("/tmp/fixtures/hello@%d.service", i)
err = util.CopyFile(tmpHelloFixture, fxtHelloService)
if err != nil {
return nil, fmt.Errorf("Failed to copy a temp fleet service: %v", err)
return nil, fmt.Errorf("failed to copy a temp fleet service: %v", err)
}

// retrieve content of hello.service, and append to bodiesOrig[]
bodyCur, stderr, err := cluster.Fleetctl(m, "cat", helloFilename)
if err != nil {
return nil, fmt.Errorf("Failed to run cat %s: %v\nstderr: %s", helloFilename, err, stderr)
return nil, fmt.Errorf("failed to run cat %s: %v\nstderr: %s", helloFilename, err, stderr)
}
bodiesOrig = append(bodiesOrig, bodyCur)

// generate a new service derived by fixtures, and store it under /tmp
curHelloService := path.Join("/tmp", helloFilename)
err = util.GenNewFleetService(curHelloService, fxtHelloService, "sleep 2", "sleep 1")
if err != nil {
return nil, fmt.Errorf("Failed to generate a temp fleet service: %v", err)
return nil, fmt.Errorf("failed to generate a temp fleet service: %v", err)
}
}
return bodiesOrig, nil
Expand All @@ -511,21 +511,21 @@ func replaceUnitCommon(t *testing.T, cmd string, numRUnits int) error {

// replace the unit and assert it shows up
if stdout, stderr, err := cluster.Fleetctl(m, cmd, "--replace", curHelloService); err != nil {
return fmt.Errorf("Unable to replace fleet unit: %v\nstdout: %s\nstderr: %s", err, stdout, stderr)
return fmt.Errorf("unable to replace fleet unit: %v\nstdout: %s\nstderr: %s", err, stdout, stderr)
}
if err := waitForNUnitsCmd(cluster, m, cmd, numUnits); err != nil {
return fmt.Errorf("Did not find %d units in cluster", numUnits)
return fmt.Errorf("did not find %d units in cluster", numUnits)
}

// retrieve content of hello.service, and compare it with the
// correspondent entry in bodiesOrig[]
bodyCur, stderr, err := cluster.Fleetctl(m, "cat", helloFilename)
if err != nil {
return fmt.Errorf("Failed to run cat %s: %v\nstderr: %s", helloFilename, err, stderr)
return fmt.Errorf("failed to run cat %s: %v\nstderr: %s", helloFilename, err, stderr)
}

if bodiesOrig[i] == bodyCur {
return fmt.Errorf("Error. the unit %s has not been replaced.", helloFilename)
return fmt.Errorf("error. the unit %s has not been replaced", helloFilename)
}
}

Expand All @@ -539,7 +539,7 @@ func replaceUnitCommon(t *testing.T, cmd string, numRUnits int) error {
return err
}
if err := waitForNUnitsCmd(cluster, m, cmd, numRUnits); err != nil {
return fmt.Errorf("Did not find %d units in cluster", numRUnits)
return fmt.Errorf("did not find %d units in cluster", numRUnits)
}

// Before starting comparison, prepare a slice of unit bodies of each
Expand All @@ -566,7 +566,7 @@ func replaceUnitCommon(t *testing.T, cmd string, numRUnits int) error {
}

if err := waitForNUnitsCmd(cluster, m, cmd, 0); err != nil {
return fmt.Errorf("Failed to get every unit to be cleaned up: %v", err)
return fmt.Errorf("failed to get every unit to be cleaned up: %v", err)
}

os.Remove(tmpFixtures)
Expand All @@ -584,11 +584,11 @@ func launchUnitsCmd(cluster platform.Cluster, m platform.Member, cmd string, num

if stdout, stderr, err := cluster.Fleetctl(m, args...); err != nil {
return nil,
fmt.Errorf("Unable to %s batch of units: \nstdout: %s\nstderr: %s\nerr: %v",
fmt.Errorf("unable to %s batch of units: \nstdout: %s\nstderr: %s\nerr: %v",
cmd, stdout, stderr, err)
} else if strings.Contains(stderr, "Error") {
return nil,
fmt.Errorf("Failed to correctly %s batch of units: \nstdout: %s\nstderr: %s\nerr: %v",
fmt.Errorf("failed to correctly %s batch of units: \nstdout: %s\nstderr: %s\nerr: %v",
cmd, stdout, stderr, err)
}

Expand All @@ -598,7 +598,7 @@ func launchUnitsCmd(cluster platform.Cluster, m platform.Member, cmd string, num
func cleanUnits(cl platform.Cluster, m platform.Member, cmd string, ufs []string, nu int) (err error) {
for i := 0; i < nu; i++ {
if stdout, stderr, err := cl.Fleetctl(m, cmd, ufs[i]); err != nil {
return fmt.Errorf("Failed to %s unit: %v\nstdout: %s\nstderr: %s", cmd, err, stdout, stderr)
return fmt.Errorf("failed to %s unit: %v\nstdout: %s\nstderr: %s", cmd, err, stdout, stderr)
}
}
return nil
Expand All @@ -624,11 +624,11 @@ func checkListUnits(cl platform.Cluster, m platform.Member, cmd string, ufs []st
lenLists = len(lus)
break
default:
return fmt.Errorf("Failed to run an invalid cmd %s", cmd)
return fmt.Errorf("failed to run an invalid cmd %s", cmd)
}

if nu == 0 && lenLists != 0 {
return fmt.Errorf("Failed to get an empty unit list")
return fmt.Errorf("failed to get an empty unit list")
}

// given unit name must be there in list-unit-files
Expand All @@ -640,7 +640,7 @@ func checkListUnits(cl platform.Cluster, m platform.Member, cmd string, ufs []st
_, found = lus[ufs[i]]
}
if lenLists != nu || !found {
return fmt.Errorf("Expected %s to be unit file", ufs[i])
return fmt.Errorf("expected %s to be unit file", ufs[i])
}
}
return err
Expand All @@ -650,23 +650,23 @@ func waitForNUnitsSubmit(cl platform.Cluster, m platform.Member, nu int) (map[st
// wait until the unit gets processed up to 15 seconds
listUnitStates, err := cl.WaitForNUnitFiles(m, nu)
if err != nil {
return nil, fmt.Errorf("Failed to run list-unit-files: %v", err)
return nil, fmt.Errorf("failed to run list-unit-files: %v", err)
}
return listUnitStates, nil
}

func waitForNUnitsLoad(cl platform.Cluster, m platform.Member, nu int) (map[string][]util.UnitState, error) {
listUnitStates, err := cl.WaitForNUnits(m, nu)
if err != nil {
return nil, fmt.Errorf("Failed to run list-units: %v", err)
return nil, fmt.Errorf("failed to run list-units: %v", err)
}
return listUnitStates, nil
}

func waitForNUnitsStart(cl platform.Cluster, m platform.Member, nu int) (map[string][]util.UnitState, error) {
listUnitStates, err := cl.WaitForNActiveUnits(m, nu)
if err != nil {
return nil, fmt.Errorf("Failed to run list-units: %v", err)
return nil, fmt.Errorf("failed to run list-units: %v", err)
}
return listUnitStates, nil
}
Expand All @@ -685,7 +685,7 @@ func waitForNUnitsCmd(cl platform.Cluster, m platform.Member, cmd string, nu int
_, err = waitForNUnitsStart(cl, m, nu)
break
default:
return fmt.Errorf("Failed to run an invalid cmd %s", cmd)
return fmt.Errorf("failed to run an invalid cmd %s", cmd)
}
return err
}
Expand Down
Loading