Skip to content

Commit

Permalink
convert: Test the exact error message strings
Browse files Browse the repository at this point in the history 
Producing intuitive error messages is part of the contract of this package,
so we should test exactly the error messages we're returning rather than
just whether conversion succeeded or not.

This exposed some cases where we _aren't_ producing good error messages
today, which are for the moment marked as FIXME because the purpose of
this change is just to improve the coverage of the test and not to change
any observable behavior.
  • Loading branch information
@apparentlymart
apparentlymart committed Oct 19, 2022
1 parent afd4db1 commit 82eaa67
Showing 1 changed file with 63 additions and 30 deletions.
93 changes: 63 additions & 30 deletions cty/convert/public_test.go
View file
Expand Up @@ -3,6 +3,7 @@ package convert
import (
"fmt"
"math/big"
"strings"
"testing"

"github.com/zclconf/go-cty/cty"
Expand All @@ -13,7 +14,7 @@ func TestConvert(t *testing.T) {
Value cty.Value
Type cty.Type
Want cty.Value
WantError bool
WantError string
}{
{
Value: cty.StringVal("hello"),
Expand All @@ -33,7 +34,7 @@ func TestConvert(t *testing.T) {
{
Value: cty.StringVal("hello"),
Type: cty.Number,
WantError: true,
WantError: `a number is required`,
},
{
Value: cty.StringVal("true"),
Expand All @@ -58,7 +59,7 @@ func TestConvert(t *testing.T) {
{
Value: cty.StringVal("hello"),
Type: cty.Bool,
WantError: true,
WantError: `a bool is required`,
},
{
Value: cty.NumberIntVal(4),
Expand Down Expand Up @@ -142,7 +143,7 @@ func TestConvert(t *testing.T) {
}),
}),
Type: cty.List(cty.DynamicPseudoType),
WantError: true, // there is no type that both tuple elements can unify to for conversion to list
WantError: `all list elements must have the same type`,
},
{
Value: cty.SetVal([]cty.Value{
Expand Down Expand Up @@ -361,7 +362,7 @@ func TestConvert(t *testing.T) {
"bool": cty.True,
}),
Type: cty.Map(cty.DynamicPseudoType),
WantError: true, // no common base type to unify to
WantError: `all map elements must have the same type`,
},
{
Value: cty.MapVal(map[string]cty.Value{
Expand Down Expand Up @@ -397,7 +398,7 @@ func TestConvert(t *testing.T) {
"greeting": cty.List(cty.String),
"name": cty.String,
}),
WantError: true, // "greeting" cannot be converted
WantError: `object required`, // FIXME: should be something like "attribute greeting: must be a list"
},
{
Value: cty.MapVal(map[string]cty.Value{
Expand All @@ -419,7 +420,7 @@ func TestConvert(t *testing.T) {
"name": cty.String,
"greeting": cty.String,
}),
WantError: true, // map has no element for required attribute "greeting"
WantError: `map has no element for required attribute "greeting"`,
},
{
Value: cty.MapVal(map[string]cty.Value{
Expand Down Expand Up @@ -511,7 +512,7 @@ func TestConvert(t *testing.T) {
Type: cty.Object(map[string]cty.Type{
"foo": cty.String,
}),
WantError: true, // given value must have superset object type
WantError: `attribute "foo" is required`,
},
{
Value: cty.ObjectVal(map[string]cty.Value{
Expand All @@ -521,7 +522,7 @@ func TestConvert(t *testing.T) {
"foo": cty.String,
"baz": cty.String,
}),
WantError: true, // given value must have superset object type
WantError: `attributes "baz" and "foo" are required`,
},
{
Value: cty.EmptyObjectVal,
Expand All @@ -530,7 +531,7 @@ func TestConvert(t *testing.T) {
"bar": cty.String,
"baz": cty.String,
}),
WantError: true, // given value must have superset object type
WantError: `attributes "bar", "baz", and "foo" are required`,
},
{
Value: cty.ObjectVal(map[string]cty.Value{
Expand Down Expand Up @@ -574,7 +575,7 @@ func TestConvert(t *testing.T) {
},
[]string{"foo"},
),
WantError: true, // Attribute "bar" is required
WantError: `attribute "bar" is required`,
},
{
Value: cty.NullVal(cty.DynamicPseudoType),
Expand Down Expand Up @@ -622,7 +623,7 @@ func TestConvert(t *testing.T) {
Type: cty.Object(map[string]cty.Type{
"foo": cty.Number,
}),
WantError: true, // recursive conversion from bool to number is impossible
WantError: `attribute "foo": number required`,
},
{
Value: cty.ObjectVal(map[string]cty.Value{
Expand All @@ -631,7 +632,7 @@ func TestConvert(t *testing.T) {
Type: cty.Object(map[string]cty.Type{
"foo": cty.Number,
}),
WantError: true, // recursive conversion from bool to number is impossible
WantError: `attribute "foo": number required`,
},
{
Value: cty.NullVal(cty.String),
Expand Down Expand Up @@ -670,14 +671,14 @@ func TestConvert(t *testing.T) {
cty.True,
}),
Type: cty.EmptyTuple,
WantError: true,
WantError: `tuple required`, // FIXME: this error is not descriptive enough
},
{
Value: cty.EmptyTupleVal,
Type: cty.Tuple([]cty.Type{
cty.String,
}),
WantError: true,
WantError: `tuple required`, // FIXME: this error is not descriptive enough
},
{
Value: cty.EmptyTupleVal,
Expand Down Expand Up @@ -769,8 +770,14 @@ func TestConvert(t *testing.T) {
}),
}),
}),
Type: cty.Map(cty.Map(cty.Object(map[string]cty.Type{"x": cty.Map(cty.DynamicPseudoType)}))),
WantError: true,
Type: cty.Map(
cty.Map(
cty.Object(map[string]cty.Type{
"x": cty.Map(cty.DynamicPseudoType),
}),
),
),
WantError: `element "b": element "x": attribute "x" is required`,
},
// reduction of https://github.com/hashicorp/terraform/issues/23431
{
Expand Down Expand Up @@ -900,7 +907,6 @@ func TestConvert(t *testing.T) {
"b": cty.StringVal("2"),
}),
}),
WantError: false,
},
// https://github.com/hashicorp/terraform/issues/24377:
{
Expand All @@ -910,7 +916,7 @@ func TestConvert(t *testing.T) {
cty.NullVal(cty.DynamicPseudoType),
}),
Type: cty.Set(cty.DynamicPseudoType),
WantError: true,
WantError: `all set elements must have the same type`,
},
{
Value: cty.TupleVal([]cty.Value{
Expand All @@ -919,23 +925,23 @@ func TestConvert(t *testing.T) {
cty.NullVal(cty.DynamicPseudoType),
}),
Type: cty.List(cty.DynamicPseudoType),
WantError: true,
WantError: `all list elements must have the same type`,
},
{
Value: cty.TupleVal([]cty.Value{
cty.ListVal([]cty.Value{cty.StringVal("a")}),
cty.StringVal("b"),
}),
Type: cty.Set(cty.DynamicPseudoType),
WantError: true,
WantError: `all set elements must have the same type`,
},
{
Value: cty.TupleVal([]cty.Value{
cty.ListVal([]cty.Value{cty.StringVal("a")}),
cty.StringVal("b"),
}),
Type: cty.List(cty.DynamicPseudoType),
WantError: true,
WantError: `all list elements must have the same type`,
},
{
Value: cty.TupleVal([]cty.Value{
Expand All @@ -949,7 +955,6 @@ func TestConvert(t *testing.T) {
cty.StringVal("9"),
cty.NullVal(cty.DynamicPseudoType),
}),
WantError: false,
},
{
Value: cty.TupleVal([]cty.Value{
Expand All @@ -963,7 +968,6 @@ func TestConvert(t *testing.T) {
cty.StringVal("9"),
cty.NullVal(cty.DynamicPseudoType),
}),
WantError: false,
},
{
Value: cty.TupleVal([]cty.Value{
Expand All @@ -975,7 +979,6 @@ func TestConvert(t *testing.T) {
Want: cty.SetVal([]cty.Value{
cty.NullVal(cty.DynamicPseudoType),
}),
WantError: false,
},
{
Value: cty.TupleVal([]cty.Value{
Expand All @@ -989,7 +992,6 @@ func TestConvert(t *testing.T) {
cty.NullVal(cty.DynamicPseudoType),
cty.NullVal(cty.DynamicPseudoType),
}),
WantError: false,
},
{
Value: cty.ListVal([]cty.Value{
Expand Down Expand Up @@ -1025,7 +1027,6 @@ func TestConvert(t *testing.T) {
})),
})},
),
WantError: false,
},
{
Value: cty.SetVal([]cty.Value{
Expand Down Expand Up @@ -1061,7 +1062,6 @@ func TestConvert(t *testing.T) {
})),
})},
),
WantError: false,
},
{
Value: cty.MapVal(map[string]cty.Value{
Expand Down Expand Up @@ -1097,7 +1097,6 @@ func TestConvert(t *testing.T) {
})),
})},
),
WantError: false,
},
}

Expand All @@ -1106,9 +1105,13 @@ func TestConvert(t *testing.T) {
got, err := Convert(test.Value, test.Type)

switch {
case test.WantError:
case test.WantError != "":
if err == nil {
t.Errorf("conversion succeeded with %#v; want error", got)
} else {
if got, want := errorStrForTesting(err), test.WantError; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}
}
default:
if err != nil {
Expand All @@ -1126,3 +1129,33 @@ func TestConvert(t *testing.T) {
})
}
}

func errorStrForTesting(err error) string {
switch err := err.(type) {
case cty.PathError:
if pathStr := pathStrForTesting(err.Path); pathStr != "" {
return pathStr + ": " + err.Error()
}
return err.Error()
default:
return err.Error()
}
}

func pathStrForTesting(path cty.Path) string {
if len(path) == 0 {
return ""
}
var buf strings.Builder
for _, step := range path {
switch step := step.(type) {
case cty.GetAttrStep:
fmt.Fprintf(&buf, ".%s", step.Name)
case cty.IndexStep:
fmt.Fprintf(&buf, "[%#v]", step.Key)
default:
fmt.Fprintf(&buf, "<INVALID: %#v>", step)
}
}
return buf.String()
}

0 comments on commit 82eaa67

Please sign in to comment.