Add TODOs, clean up, clarify comments, and add one more test.

pkg/util/fuzz.go

@@ -28,12 +28,21 @@ type Fuzzer struct {
 }
 
 // NewFuzzer returns a new Fuzzer with the given custom fuzzing functions.
+//
 // Each entry in fuzzFuncs must be a function with one parameter, which will
 // be the variable we wish that function to fill with random data. For this
-// to be effective, the variable type should be either a pointer, map, slice,
-// etc. These functions are called sensibly, e.g., if you wanted custom string
+// to be effective, the variable type should be either a pointer or a map.
+//
+// These functions are called sensibly, e.g., if you wanted custom string
 // fuzzing, the function `func(s *string)` would get called and passed the
-// address of strings.
+// address of strings. Maps and pointers will be made/new'd for you. For
+// slices, it doesn't make much sense to pre-create them--Fuzzer doesn't
+// know how long you want your slice--so take a pointer to a slice, and
+// make it yourself. (If you don't want your map/pointer type pre-made,
+// take a pointer to it, and make it yourself.)
+//
+// TODO: Take a source of randomness for deterministic, repeatable fuzzing.
+// TODO: Make probability of getting a nil customizable.
 func NewFuzzer(fuzzFuncs ...interface{}) *Fuzzer {
 	f := &Fuzzer{
 		map[reflect.Type]func(reflect.Value){},
@@ -48,7 +57,11 @@ func NewFuzzer(fuzzFuncs ...interface{}) *Fuzzer {
 			panic("Need 1 in and 0 out params!")
 		}
 		argT := t.In(0)
-		// fmt.Printf("Making entry for thing with '%v' type\n", argT)
+		switch argT.Kind() {
+		case reflect.Ptr, reflect.Map:
+		default:
+			panic("fuzzFunc must take pointer or map type")
+		}
 		f.customFuzz[argT] = func(toFuzz reflect.Value) {
 			if toFuzz.Type().AssignableTo(argT) {
 				v.Call([]reflect.Value{toFuzz})
@@ -96,9 +109,6 @@ func (f *Fuzzer) doFuzz(v reflect.Value) {
 	case reflect.Map:
 		if rand.Intn(5) > 0 {
 			v.Set(reflect.MakeMap(v.Type()))
-			if f.tryCustom(v) {
-				return
-			}
 			n := 1 + rand.Intn(10)
 			for i := 0; i < n; i++ {
 				key := reflect.New(v.Type().Key()).Elem()
@@ -121,9 +131,6 @@ func (f *Fuzzer) doFuzz(v reflect.Value) {
 		if rand.Intn(5) > 0 {
 			n := 1 + rand.Intn(10)
 			v.Set(reflect.MakeSlice(v.Type(), n, n))
-			if f.tryCustom(v) {
-				return
-			}
 			for i := 0; i < n; i++ {
 				f.doFuzz(v.Index(i))
 			}
@@ -147,15 +154,15 @@ func (f *Fuzzer) doFuzz(v reflect.Value) {
 	}
 }
 
-// tryCustom searches for custom handlers and Randomizer implementations, and
-// returns true if it finds a match and successfully randomizes v.
+// tryCustom searches for custom handlers, and returns true iff it finds a match
+// and successfully randomizes v.
 func (f Fuzzer) tryCustom(v reflect.Value) bool {
-	// fmt.Printf("Trying thing with '%v' type\n", v.Type())
+	doCustom, ok := f.customFuzz[v.Type()]
+	if !ok {
+		return false
+	}
+
 	switch v.Kind() {
-	case reflect.Chan, reflect.Func, reflect.Interface, reflect.Slice:
-		if v.IsNil() {
-			return false
-		}
 	case reflect.Ptr:
 		if v.IsNil() {
 			if !v.CanSet() {
@@ -170,12 +177,12 @@ func (f Fuzzer) tryCustom(v reflect.Value) bool {
 			}
 			v.Set(reflect.MakeMap(v.Type()))
 		}
+	default:
+		return false
 	}
-	if f, ok := f.customFuzz[v.Type()]; ok {
-		f(v)
-		return true
-	}
-	return false
+
+	doCustom(v)
+	return true
 }
 
 func fuzzInt(v reflect.Value) {

pkg/util/fuzz_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package util
 
 import (
+	"reflect"
 	"testing"
 )
 
@@ -113,11 +114,12 @@ func TestFuzz_structptr(t *testing.T) {
 	checkFailed(t, failed)
 }
 
-// Try fuzzing up to 20 times. Fail if check() never passes.
-func tryFuzz(t *testing.T, obj interface{}, check func() (stage int, passed bool)) {
+// tryFuzz tries fuzzing up to 20 times. Fail if check() never passes, report the highest
+// stage it ever got to.
+func tryFuzz(t *testing.T, f *Fuzzer, obj interface{}, check func() (stage int, passed bool)) {
 	maxStage := 0
 	for i := 0; i < 20; i++ {
-		NewFuzzer().Fuzz(obj)
+		f.Fuzz(obj)
 		stage, passed := check()
 		if stage > maxStage {
 			maxStage = stage
@@ -139,7 +141,7 @@ func TestFuzz_structmap(t *testing.T) {
 		B map[string]string
 	}{}
 
-	tryFuzz(t, obj, func() (int, bool) {
+	tryFuzz(t, NewFuzzer(), obj, func() (int, bool) {
 		if obj.A == nil {
 			return 1, false
 		}
@@ -181,7 +183,7 @@ func TestFuzz_structslice(t *testing.T) {
 		B []string
 	}{}
 
-	tryFuzz(t, obj, func() (int, bool) {
+	tryFuzz(t, NewFuzzer(), obj, func() (int, bool) {
 		if obj.A == nil {
 			return 1, false
 		}
@@ -213,17 +215,40 @@ func TestFuzz_custom(t *testing.T) {
 	obj := &struct {
 		A string
 		B *string
+		C map[string]string
+		D *map[string]string
 	}{}
 
 	testPhrase := "gotcalled"
-	NewFuzzer(func(s *string) { *s = testPhrase }).Fuzz(obj)
-	if obj.A != testPhrase {
-		t.Errorf("A not set")
-	}
-	if obj.B == nil {
-		t.Fatalf("B is nil")
-	}
-	if *obj.B != testPhrase {
-		t.Errorf("B not set")
-	}
+	testMap := map[string]string{"C": "D"}
+	f := NewFuzzer(
+		func(s *string) {
+			*s = testPhrase
+		},
+		func(m map[string]string) {
+			m["C"] = "D"
+		},
+	)
+
+	tryFuzz(t, f, obj, func() (int, bool) {
+		if obj.A != testPhrase {
+			return 1, false
+		}
+		if obj.B == nil {
+			return 2, false
+		}
+		if *obj.B != testPhrase {
+			return 3, false
+		}
+		if e, a := testMap, obj.C; !reflect.DeepEqual(e, a) {
+			return 4, false
+		}
+		if obj.D == nil {
+			return 5, false
+		}
+		if e, a := testMap, *obj.D; !reflect.DeepEqual(e, a) {
+			return 6, false
+		}
+		return 7, true
+	})
 }