From c985a0a0f655b35ead79bbde655955d57a76d478 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Mon, 15 Apr 2024 11:50:18 -0400 Subject: [PATCH] Group CBOR decode tests by the kind of their inputs. No test cases are added, removed, or modified. Grouping them this way is intended to make it easier to reason about the coverage for possible inputs versus one long list of test cases. --- .../cbor/internal/modes/decode_test.go | 165 +++++++++++------- 1 file changed, 103 insertions(+), 62 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go index 4a7982d13dc..86db2e4f8ab 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go @@ -37,14 +37,99 @@ func TestDecode(t *testing.T) { return b } - for _, tc := range []struct { + type test struct { name string - modes []cbor.DecMode + modes []cbor.DecMode // most tests should run for all modes in []byte into interface{} // prototype for concrete destination type. if nil, decode into empty interface value. want interface{} assertOnError func(t *testing.T, e error) - }{ + } + + // Test cases are grouped by the kind of the CBOR data item being decoded, as enumerated in + // https://www.rfc-editor.org/rfc/rfc8949.html#section-2. + group := func(t *testing.T, name string, tests []test) { + t.Run(name, func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + decModes := test.modes + if len(decModes) == 0 { + decModes = allDecModes + } + + for _, decMode := range decModes { + modeName, ok := decModeNames[decMode] + if !ok { + t.Fatal("test case configured to run against unrecognized mode") + } + + t.Run(fmt.Sprintf("%s/mode=%s", test.name, modeName), func(t *testing.T) { + var dst reflect.Value + if test.into == nil { + var i interface{} + dst = reflect.ValueOf(&i) + } else { + dst = reflect.New(reflect.TypeOf(test.into)) + } + err := decMode.Unmarshal(test.in, dst.Interface()) + test.assertOnError(t, err) + if test.want != nil { + if diff := cmp.Diff(test.want, dst.Elem().Interface()); diff != "" { + t.Errorf("unexpected output:\n%s", diff) + } + } + }) + } + }) + } + }) + } + + group(t, "unsigned integer", []test{ + { + name: "unsigned integer decodes to interface{} as int64", + in: hex("0a"), // 10 + want: int64(10), + assertOnError: assertNilError, + }, + }) + + group(t, "negative integer", []test{}) + + group(t, "byte string", []test{}) + + group(t, "text string", []test{ + { + name: "reject text string containing invalid utf-8 sequence", + in: hex("6180"), // text string beginning with continuation byte 0x80 + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.SemanticError) { + const expected = "cbor: invalid UTF-8 string" + if msg := e.Error(); msg != expected { + t.Errorf("expected %v, got %v", expected, msg) + } + }), + }, + { + name: "indefinite-length text string", + in: hex("7f616161626163ff"), // (_ "a", "b", "c") + want: "abc", + assertOnError: assertNilError, + }, + }) + + group(t, "array", []test{ + { + name: "nested indefinite-length array", + in: hex("9f9f8080ff9f8080ffff"), // [_ [_ [] []] [_ [][]]] + want: []interface{}{ + []interface{}{[]interface{}{}, []interface{}{}}, + []interface{}{[]interface{}{}, []interface{}{}}, + }, + assertOnError: assertNilError, + }, + }) + + group(t, "map", []test{ { name: "reject duplicate negative int keys into struct", modes: []cbor.DecMode{modes.DecodeLax}, @@ -216,22 +301,6 @@ func TestDecode(t *testing.T) { }, assertOnError: assertNilError, }, - { - name: "reject text string containing invalid utf-8 sequence", - in: hex("6180"), // text string beginning with continuation byte 0x80 - assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.SemanticError) { - const expected = "cbor: invalid UTF-8 string" - if msg := e.Error(); msg != expected { - t.Errorf("expected %v, got %v", expected, msg) - } - }), - }, - { - name: "unsigned integer decodes to interface{} as int64", - in: hex("0a"), // 10 - want: int64(10), - assertOnError: assertNilError, - }, { name: "unknown field error", modes: []cbor.DecMode{modes.Decode}, @@ -251,21 +320,6 @@ func TestDecode(t *testing.T) { want: struct{}{}, assertOnError: assertNilError, }, - { - name: "indefinite-length text string", - in: hex("7f616161626163ff"), // (_ "a", "b", "c") - want: "abc", - assertOnError: assertNilError, - }, - { - name: "nested indefinite-length array", - in: hex("9f9f8080ff9f8080ffff"), // [_ [_ [] []] [_ [][]]] - want: []interface{}{ - []interface{}{[]interface{}{}, []interface{}{}}, - []interface{}{[]interface{}{}, []interface{}{}}, - }, - assertOnError: assertNilError, - }, { name: "nested indefinite-length map", in: hex("bf6141bf616101616202ff6142bf616901616a02ffff"), // {_ "A": {_ "a": 1, "b": 2}, "B": {_ "i": 1, "j": 2}} @@ -275,34 +329,21 @@ func TestDecode(t *testing.T) { }, assertOnError: assertNilError, }, - } { - decModes := tc.modes - if len(decModes) == 0 { - decModes = allDecModes - } + }) - for _, decMode := range decModes { - modeName, ok := decModeNames[decMode] - if !ok { - t.Fatal("test case configured to run against unrecognized mode") - } + group(t, "floating-point number", []test{}) - t.Run(fmt.Sprintf("mode=%s/%s", modeName, tc.name), func(t *testing.T) { - var dst reflect.Value - if tc.into == nil { - var i interface{} - dst = reflect.ValueOf(&i) - } else { - dst = reflect.New(reflect.TypeOf(tc.into)) - } - err := decMode.Unmarshal(tc.in, dst.Interface()) - tc.assertOnError(t, err) - if tc.want != nil { - if diff := cmp.Diff(tc.want, dst.Elem().Interface()); diff != "" { - t.Errorf("unexpected output:\n%s", diff) - } - } - }) - } - } + group(t, "simple value", []test{}) + + t.Run("tag", func(t *testing.T) { + group(t, "rfc3339 time", []test{}) + + group(t, "epoch time", []test{}) + + group(t, "unsigned bignum", []test{}) + + group(t, "negative bignum", []test{}) + + group(t, "unrecognized", []test{}) + }) }