From 1fc9672971bd3ab0eebef403c41f495ea8e5a8c1 Mon Sep 17 00:00:00 2001 From: Ian Fijolek Date: Wed, 19 Aug 2020 10:19:43 -0700 Subject: [PATCH] Update tests to 75%+ coverage --- Makefile | 2 +- main.go | 5 +- main_test.go | 165 +++++++++++++++++++++++++++++---------------------- 3 files changed, 98 insertions(+), 74 deletions(-) diff --git a/Makefile b/Makefile index e69f179..f3d99de 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ run: test: go test -coverprofile=coverage.out go tool cover -func=coverage.out - # @go tool cover -func=coverage.out | awk -v target=80.0% \ + @go tool cover -func=coverage.out | awk -v target=75.0% \ '/^total:/ { print "Total coverage: " $$3 " Minimum coverage: " target; if ($$3+0.0 >= target+0.0) print "ok"; else { print "fail"; exit 1; } }' # Installs pre-commit hooks diff --git a/main.go b/main.go index c77d163..0f16be0 100644 --- a/main.go +++ b/main.go @@ -168,8 +168,11 @@ func (job ContainerExecJob) Run() { job.context, execID.ID, ) + slog.Debug("Exec info: %+v", execInfo) if err != nil { - panic(err) + // Nothing we can do if we got an error here, so let's go + slog.WarnOnErr(err, "Could not get status for exec job %s", job.name) + return } time.Sleep(1 * time.Second) } diff --git a/main_test.go b/main_test.go index c658a80..95be674 100644 --- a/main_test.go +++ b/main_test.go @@ -43,62 +43,19 @@ type FakeDockerClient struct { FakeCalls map[string][]FakeCall } +// AssertFakeCalls checks expected against actual calls to fake methods func (fakeClient FakeDockerClient) AssertFakeCalls(t *testing.T, expectedCalls map[string][]FakeCall, message string) { - if !reflect.DeepEqual(expectedCalls, fakeClient.FakeCalls) { + if !reflect.DeepEqual(fakeClient.FakeCalls, expectedCalls) { t.Errorf( - "%s: Calls are not as expected. Expected %+v Actual %+v", + "%s: Expected and actual calls do not match. Expected %+v Actual %+v", message, expectedCalls, fakeClient.FakeCalls, ) } - /* - * for method, calls := range fakeClient.FakeCalls { - * exCalls, ok := expectedCalls[method] - * if !ok { - * // We did not expect these calls - * t.Errorf( - * "%s: Calls made to %s but none expected. Actual %+v", - * message, - * method, - * calls, - * ) - * } - * if len(calls) != len(exCalls) { - * t.Errorf( - * "%s: Number of calls to %s are not as expected. Expected %+v Actual %+v", - * message, - * method, - * exCalls, - * calls, - * ) - * } - * - * #<{(| - * * for i, call := range calls { - * * exCall := exCalls[i] - * * if call != exCall { - * * - * * } - * * } - * |)}># - * - * if !reflect.DeepEqual(calls, exCalls) { - * // The calls to the method aren't what we expected - * t.Errorf( - * "%s: Calls to %s are not as expected. Expected %+v Actual %+v", - * message, - * method, - * exCalls, - * calls, - * ) - * } - * // Remove method from expected calls so we can see if any weren't called later - * delete(expectedCalls, method) - * } - */ } +// called is a helper method to get return values and log the method call func (fakeClient *FakeDockerClient) called(method string, v ...interface{}) FakeResult { if fakeClient.FakeCalls == nil { fakeClient.FakeCalls = map[string][]FakeCall{} @@ -800,6 +757,86 @@ func TestRunExecJobs(t *testing.T) { }, expectPanic: true, }, + { + name: "Successfully start an exec job fail on status", + client: &FakeDockerClient{ + FakeResults: map[string][]FakeResult{ + "ContainerInspect": []FakeResult{ + FakeResult{runningContainerInfo, nil}, + }, + "ContainerExecCreate": []FakeResult{ + FakeResult{dockerTypes.IDResponse{ID: "id"}, nil}, + }, + "ContainerExecStart": []FakeResult{ + FakeResult{nil}, + }, + "ContainerExecInspect": []FakeResult{ + FakeResult{nil, fmt.Errorf("fail")}, + }, + }, + }, + expectedCalls: map[string][]FakeCall{ + "ContainerInspect": []FakeCall{ + FakeCall{jobContext, jobContainerID}, + }, + "ContainerExecCreate": []FakeCall{ + FakeCall{ + jobContext, + jobContainerID, + dockerTypes.ExecConfig{ + Cmd: []string{"sh", "-c", jobCommand}, + }, + }, + }, + "ContainerExecStart": []FakeCall{ + FakeCall{jobContext, "id", dockerTypes.ExecStartCheck{}}, + }, + "ContainerExecInspect": []FakeCall{ + FakeCall{jobContext, "id"}, + }, + }, + }, + { + name: "Successfully start an exec job and run to completion", + client: &FakeDockerClient{ + FakeResults: map[string][]FakeResult{ + "ContainerInspect": []FakeResult{ + FakeResult{runningContainerInfo, nil}, + }, + "ContainerExecCreate": []FakeResult{ + FakeResult{dockerTypes.IDResponse{ID: "id"}, nil}, + }, + "ContainerExecStart": []FakeResult{ + FakeResult{nil}, + }, + "ContainerExecInspect": []FakeResult{ + FakeResult{dockerTypes.ContainerExecInspect{Running: true}, nil}, + FakeResult{dockerTypes.ContainerExecInspect{Running: false}, nil}, + }, + }, + }, + expectedCalls: map[string][]FakeCall{ + "ContainerInspect": []FakeCall{ + FakeCall{jobContext, jobContainerID}, + }, + "ContainerExecCreate": []FakeCall{ + FakeCall{ + jobContext, + jobContainerID, + dockerTypes.ExecConfig{ + Cmd: []string{"sh", "-c", jobCommand}, + }, + }, + }, + "ContainerExecStart": []FakeCall{ + FakeCall{jobContext, "id", dockerTypes.ExecStartCheck{}}, + }, + "ContainerExecInspect": []FakeCall{ + FakeCall{jobContext, "id"}, + FakeCall{jobContext, "id"}, + }, + }, + }, } for _, c := range cases { @@ -819,7 +856,10 @@ func TestRunExecJobs(t *testing.T) { defer func() { // Recover from panics, if there were any - recover() + if err := recover(); err != nil { + t.Log("Recovered from panic") + t.Log(err) + } c.client.AssertFakeCalls(t, c.expectedCalls, "Failed") }() job.Run() @@ -866,10 +906,7 @@ func TestRunStartJobs(t *testing.T) { client: &FakeDockerClient{ FakeResults: map[string][]FakeResult{ "ContainerInspect": []FakeResult{ - FakeResult{ - runningContainerInfo, - nil, - }, + FakeResult{runningContainerInfo, nil}, }, }, }, @@ -884,12 +921,8 @@ func TestRunStartJobs(t *testing.T) { client: &FakeDockerClient{ FakeResults: map[string][]FakeResult{ "ContainerInspect": []FakeResult{ - FakeResult{ - stoppedContainerInfo, - nil, - }, + FakeResult{stoppedContainerInfo, nil}, }, - // "ContainerStart": []FakeResult{FakeResult{nil}}, "ContainerStart": []FakeResult{FakeResult{fmt.Errorf("fail")}}, }, }, @@ -907,18 +940,9 @@ func TestRunStartJobs(t *testing.T) { client: &FakeDockerClient{ FakeResults: map[string][]FakeResult{ "ContainerInspect": []FakeResult{ - FakeResult{ - stoppedContainerInfo, - nil, - }, - FakeResult{ - runningContainerInfo, - nil, - }, - FakeResult{ - stoppedContainerInfo, - nil, - }, + FakeResult{stoppedContainerInfo, nil}, + FakeResult{runningContainerInfo, nil}, + FakeResult{stoppedContainerInfo, nil}, }, "ContainerStart": []FakeResult{FakeResult{nil}}, }, @@ -952,11 +976,8 @@ func TestRunStartJobs(t *testing.T) { // Recover from panics, if there were any recover() c.client.AssertFakeCalls(t, c.expectedCalls, "Failed") - // ErrorUnequal(t, c.expectedCalls, c.client.FakeCalls, "Unexpected calls") }() - job.Run() - if c.expectPanic { t.Errorf("Expected panic but got none") }