Merge pull request #418 from skriss/refactor-patch-tests

use typed structs for decoding patch JSON in unit tests
pull/482/head
Andy Goldstein 2018-05-09 15:44:21 -04:00 committed by GitHub
commit 43b1f9a19e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 165 additions and 166 deletions

View File

@ -156,6 +156,7 @@ func TestProcessBackup(t *testing.T) {
sharedInformers = informers.NewSharedInformerFactory(client, 0)
logger = arktest.NewLogger()
pluginManager = &MockManager{}
clockTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", "Mon Jan 2 15:04:05 2006")
)
c := NewBackupController(
@ -169,7 +170,8 @@ func TestProcessBackup(t *testing.T) {
pluginManager,
NewBackupTracker(),
).(*backupController)
c.clock = clock.NewFakeClock(time.Now())
c.clock = clock.NewFakeClock(clockTime)
var expiration time.Time
@ -248,40 +250,43 @@ func TestProcessBackup(t *testing.T) {
actions := client.Actions()
require.Equal(t, 2, len(actions))
// validate Patch call 1 (setting version, expiration, and phase)
patchAction, ok := actions[0].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
patch := make(map[string]interface{})
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
// should have status
assert.Equal(t, 1, len(patch), "patch has wrong number of keys")
expectedStatusKeys := 2
if test.backup.Spec.TTL.Duration > 0 {
assert.True(t, collections.HasKeyAndVal(patch, "status.expiration", expiration.UTC().Format(time.RFC3339)), "patch's status.expiration does not match")
expectedStatusKeys = 3
// structs and func for decoding patch content
type StatusPatch struct {
Expiration time.Time `json:"expiration"`
Version int `json:"version"`
Phase v1.BackupPhase `json:"phase"`
}
assert.True(t, collections.HasKeyAndVal(patch, "status.version", float64(1)))
assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(v1.BackupPhaseInProgress)), "patch's status.phase does not match")
type Patch struct {
Status StatusPatch `json:"status"`
}
res, _ := collections.GetMap(patch, "status")
assert.Equal(t, expectedStatusKeys, len(res), "patch's status has the wrong number of keys")
decode := func(decoder *json.Decoder) (interface{}, error) {
actual := new(Patch)
err := decoder.Decode(actual)
return *actual, err
}
// validate Patch call 1 (setting version, expiration, and phase)
expected := Patch{
Status: StatusPatch{
Version: 1,
Phase: v1.BackupPhaseInProgress,
Expiration: expiration,
},
}
arktest.ValidatePatch(t, actions[0], expected, decode)
// validate Patch call 2 (setting phase)
patchAction, ok = actions[1].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
expected = Patch{
Status: StatusPatch{
Phase: v1.BackupPhaseCompleted,
},
}
patch = make(map[string]interface{})
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
assert.Equal(t, 1, len(patch), "patch has wrong number of keys")
res, _ = collections.GetMap(patch, "status")
assert.Equal(t, 1, len(res), "patch's status has the wrong number of keys")
assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(v1.BackupPhaseCompleted)), "patch's status.phase does not match")
arktest.ValidatePatch(t, actions[1], expected, decode)
})
}
}

View File

@ -25,12 +25,11 @@ import (
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
core "k8s.io/client-go/testing"
"k8s.io/apimachinery/pkg/util/clock"
"github.com/heptio/ark/pkg/apis/ark/v1"
"github.com/heptio/ark/pkg/generated/clientset/versioned/fake"
informers "github.com/heptio/ark/pkg/generated/informers/externalversions"
"github.com/heptio/ark/pkg/util/collections"
arktest "github.com/heptio/ark/pkg/util/test"
)
@ -109,6 +108,7 @@ func TestProcessDownloadRequest(t *testing.T) {
restoresInformer = sharedInformers.Ark().V1().Restores()
backupService = &arktest.BackupService{}
logger = arktest.NewLogger()
clockTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", "Mon Jan 2 15:04:05 2006")
)
defer backupService.AssertExpectations(t)
@ -121,6 +121,8 @@ func TestProcessDownloadRequest(t *testing.T) {
logger,
).(*downloadRequestController)
c.clock = clock.NewFakeClock(clockTime)
var downloadRequest *v1.DownloadRequest
if tc.expectedPhase == v1.DownloadRequestPhaseProcessed {
@ -168,26 +170,33 @@ func TestProcessDownloadRequest(t *testing.T) {
// otherwise, we should get exactly 1 patch
require.Equal(t, 1, len(actions))
patchAction, ok := actions[0].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
patch := make(map[string]interface{})
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
type PatchStatus struct {
DownloadURL string `json:"downloadURL"`
Phase v1.DownloadRequestPhase `json:"phase"`
Expiration time.Time `json:"expiration"`
}
// check the URL
assert.True(t, collections.HasKeyAndVal(patch, "status.downloadURL", tc.expectedURL), "patch's status.downloadURL does not match")
type Patch struct {
Status PatchStatus `json:"status"`
}
// check the Phase
assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(tc.expectedPhase)), "patch's status.phase does not match")
decode := func(decoder *json.Decoder) (interface{}, error) {
actual := new(Patch)
err := decoder.Decode(actual)
// check that Expiration exists
// TODO pass a fake clock to the controller and verify
// the expiration value
assert.True(t, collections.Exists(patch, "status.expiration"), "patch's status.expiration does not exist")
return *actual, err
}
// we expect 3 total updates.
res, _ := collections.GetMap(patch, "status")
assert.Equal(t, 3, len(res), "patch's status has the wrong number of keys")
expected := Patch{
Status: PatchStatus{
DownloadURL: tc.expectedURL,
Phase: tc.expectedPhase,
Expiration: clockTime.Add(signedURLTTL),
},
}
arktest.ValidatePatch(t, actions[0], expected, decode)
})
}
}

View File

@ -181,7 +181,7 @@ func TestProcessRestore(t *testing.T) {
{
name: "restore with non-existent backup name fails",
restore: arktest.NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithIncludedNamespace("ns-1").Restore,
restore: NewRestore("foo", "bar", "backup-1", "ns-1", "*", api.RestorePhaseNew).Restore,
expectedErr: false,
expectedPhase: string(api.RestorePhaseFailedValidation),
expectedValidationErrors: []string{"Error retrieving backup: no backup here"},
@ -371,35 +371,35 @@ func TestProcessRestore(t *testing.T) {
return
}
// structs and func for decoding patch content
type StatusPatch struct {
Phase api.RestorePhase `json:"phase"`
ValidationErrors []string `json:"validationErrors"`
Errors int `json:"errors"`
}
type Patch struct {
Status StatusPatch `json:"status"`
}
decode := func(decoder *json.Decoder) (interface{}, error) {
actual := new(Patch)
err := decoder.Decode(actual)
return *actual, err
}
// validate Patch call 1 (setting phase, validation errs)
require.True(t, len(actions) > 0, "len(actions) is too small")
patchAction, ok := actions[0].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
patch := make(map[string]interface{})
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
expectedStatusKeys := 1
assert.True(t, collections.HasKeyAndVal(patch, "status.phase", test.expectedPhase), "patch's status.phase does not match")
if len(test.expectedValidationErrors) > 0 {
errs, err := collections.GetSlice(patch, "status.validationErrors")
require.NoError(t, err, "error getting patch's status.validationErrors")
var errStrings []string
for _, err := range errs {
errStrings = append(errStrings, err.(string))
}
assert.Equal(t, test.expectedValidationErrors, errStrings, "patch's status.validationErrors does not match")
expectedStatusKeys++
expected := Patch{
Status: StatusPatch{
Phase: api.RestorePhase(test.expectedPhase),
ValidationErrors: test.expectedValidationErrors,
},
}
res, _ := collections.GetMap(patch, "status")
assert.Equal(t, expectedStatusKeys, len(res), "patch's status has the wrong number of keys")
arktest.ValidatePatch(t, actions[0], expected, decode)
// if we don't expect a restore, validate it wasn't called and exit the test
if test.expectedRestorerCall == nil {
@ -410,24 +410,15 @@ func TestProcessRestore(t *testing.T) {
assert.Equal(t, 1, len(restorer.Calls))
// validate Patch call 2 (setting phase)
patchAction, ok = actions[1].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
assert.Equal(t, 1, len(patch), "patch has wrong number of keys")
res, _ = collections.GetMap(patch, "status")
expectedStatusKeys = 1
assert.True(t, collections.HasKeyAndVal(patch, "status.phase", string(api.RestorePhaseCompleted)), "patch's status.phase does not match")
if test.expectedRestoreErrors != 0 {
assert.True(t, collections.HasKeyAndVal(patch, "status.errors", float64(test.expectedRestoreErrors)), "patch's status.errors does not match")
expectedStatusKeys++
expected = Patch{
Status: StatusPatch{
Phase: api.RestorePhaseCompleted,
Errors: test.expectedRestoreErrors,
},
}
assert.Equal(t, expectedStatusKeys, len(res), "patch's status has wrong number of keys")
arktest.ValidatePatch(t, actions[1], expected, decode)
// explicitly capturing the argument passed to Restore myself because
// I want to validate the called arg as of the time of calling, but
@ -449,7 +440,7 @@ func NewRestore(ns, name, backup, includeNS, includeResource string, phase api.R
}
for _, n := range nonRestorableResources {
restore.WithExcludedResource(n)
restore = restore.WithExcludedResource(n)
}
return restore

View File

@ -18,7 +18,6 @@ package controller
import (
"encoding/json"
"fmt"
"testing"
"time"
@ -41,15 +40,15 @@ import (
func TestProcessSchedule(t *testing.T) {
tests := []struct {
name string
scheduleKey string
schedule *api.Schedule
fakeClockTime string
expectedErr bool
expectedPhase string
expectedValidationError string
expectedBackupCreate *api.Backup
expectedLastBackup string
name string
scheduleKey string
schedule *api.Schedule
fakeClockTime string
expectedErr bool
expectedPhase string
expectedValidationErrors []string
expectedBackupCreate *api.Backup
expectedLastBackup string
}{
{
name: "invalid key returns error",
@ -67,25 +66,25 @@ func TestProcessSchedule(t *testing.T) {
expectedErr: false,
},
{
name: "schedule with phase New gets validated and failed if invalid",
schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseNew).Schedule,
expectedErr: false,
expectedPhase: string(api.SchedulePhaseFailedValidation),
expectedValidationError: "Schedule must be a non-empty valid Cron expression",
name: "schedule with phase New gets validated and failed if invalid",
schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseNew).Schedule,
expectedErr: false,
expectedPhase: string(api.SchedulePhaseFailedValidation),
expectedValidationErrors: []string{"Schedule must be a non-empty valid Cron expression"},
},
{
name: "schedule with phase <blank> gets validated and failed if invalid",
schedule: arktest.NewTestSchedule("ns", "name").Schedule,
expectedErr: false,
expectedPhase: string(api.SchedulePhaseFailedValidation),
expectedValidationError: "Schedule must be a non-empty valid Cron expression",
name: "schedule with phase <blank> gets validated and failed if invalid",
schedule: arktest.NewTestSchedule("ns", "name").Schedule,
expectedErr: false,
expectedPhase: string(api.SchedulePhaseFailedValidation),
expectedValidationErrors: []string{"Schedule must be a non-empty valid Cron expression"},
},
{
name: "schedule with phase Enabled gets re-validated and failed if invalid",
schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseEnabled).Schedule,
expectedErr: false,
expectedPhase: string(api.SchedulePhaseFailedValidation),
expectedValidationError: "Schedule must be a non-empty valid Cron expression",
name: "schedule with phase Enabled gets re-validated and failed if invalid",
schedule: arktest.NewTestSchedule("ns", "name").WithPhase(api.SchedulePhaseEnabled).Schedule,
expectedErr: false,
expectedPhase: string(api.SchedulePhaseFailedValidation),
expectedValidationErrors: []string{"Schedule must be a non-empty valid Cron expression"},
},
{
name: "schedule with phase New gets validated and triggers a backup",
@ -191,34 +190,34 @@ func TestProcessSchedule(t *testing.T) {
actions := client.Actions()
index := 0
type PatchStatus struct {
ValidationErrors []string `json:"validationErrors"`
Phase api.SchedulePhase `json:"phase"`
LastBackup time.Time `json:"lastBackup"`
}
type Patch struct {
Status PatchStatus `json:"status"`
}
decode := func(decoder *json.Decoder) (interface{}, error) {
actual := new(Patch)
err := decoder.Decode(actual)
return *actual, err
}
if test.expectedPhase != "" {
require.True(t, len(actions) > index, "len(actions) is too small")
patchAction, ok := actions[index].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
patch := make(map[string]interface{})
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
assert.Equal(t, 1, len(patch), "patch has wrong number of keys")
expectedStatusKeys := 1
assert.True(t, collections.HasKeyAndVal(patch, "status.phase", test.expectedPhase), "patch's status.phase does not match")
if test.expectedValidationError != "" {
errs, err := collections.GetSlice(patch, "status.validationErrors")
require.NoError(t, err, "error getting patch's status.validationErrors")
require.Equal(t, 1, len(errs))
assert.Equal(t, test.expectedValidationError, errs[0].(string), "patch's status.validationErrors does not match")
expectedStatusKeys++
expected := Patch{
Status: PatchStatus{
ValidationErrors: test.expectedValidationErrors,
Phase: api.SchedulePhase(test.expectedPhase),
},
}
res, _ := collections.GetMap(patch, "status")
assert.Equal(t, expectedStatusKeys, len(res), "patch's status has the wrong number of keys")
arktest.ValidatePatch(t, actions[index], expected, decode)
index++
}
@ -239,27 +238,13 @@ func TestProcessSchedule(t *testing.T) {
if test.expectedLastBackup != "" {
require.True(t, len(actions) > index, "len(actions) is too small")
patchAction, ok := actions[index].(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
expected := Patch{
Status: PatchStatus{
LastBackup: parseTime(test.expectedLastBackup),
},
}
patch := make(map[string]interface{})
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch), "cannot unmarshal patch")
assert.Equal(t, 1, len(patch), "patch has wrong number of keys")
lastBackup, _ := collections.GetValue(patch, "status.lastBackup")
fmt.Println(lastBackup)
assert.True(
t,
collections.HasKeyAndVal(patch, "status.lastBackup", parseTime(test.expectedLastBackup).UTC().Format(time.RFC3339)),
"patch's status.lastBackup does not match",
)
res, _ := collections.GetMap(patch, "status")
assert.Equal(t, 1, len(res), "patch's status has the wrong number of keys")
index++
arktest.ValidatePatch(t, actions[index], expected, decode)
}
})
}

View File

@ -122,14 +122,3 @@ func Exists(root map[string]interface{}, path string) bool {
_, err := GetValue(root, path)
return err == nil
}
// HasKeyAndVal returns true if root[path] exists and the value
// contained is equal to val, or false otherwise.
func HasKeyAndVal(root map[string]interface{}, path string, val interface{}) bool {
valObj, err := GetValue(root, path)
if err != nil {
return false
}
return valObj == val
}

View File

@ -17,10 +17,13 @@ limitations under the License.
package test
import (
"bytes"
"encoding/json"
"reflect"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
core "k8s.io/client-go/testing"
)
@ -59,3 +62,20 @@ func CompareActions(t *testing.T, expected, actual []core.Action) {
}
}
}
// ValidatePatch tests the validity of an action. It checks
// that the action is a PatchAction, that the patch decodes from JSON
// with the provided decode func and has no extraneous fields, and that
// the decoded patch matches the expected.
func ValidatePatch(t *testing.T, action core.Action, expected interface{}, decodeFunc func(*json.Decoder) (interface{}, error)) {
patchAction, ok := action.(core.PatchAction)
require.True(t, ok, "action is not a PatchAction")
decoder := json.NewDecoder(bytes.NewReader(patchAction.GetPatch()))
decoder.DisallowUnknownFields()
actual, err := decodeFunc(decoder)
require.NoError(t, err)
assert.Equal(t, expected, actual)
}