From f857608557d4b7999ec365eba8744129cd870b8e Mon Sep 17 00:00:00 2001 From: Mickael Stanislas Date: Tue, 25 Apr 2023 14:50:01 +0200 Subject: [PATCH] fix: RequireIfAttributeIsOneOf error message and tests --- internal/require_if_attribute_is_one_of.go | 8 +- .../require_if_attribute_is_one_of_test.go | 146 ++++++++++++++---- 2 files changed, 120 insertions(+), 34 deletions(-) diff --git a/internal/require_if_attribute_is_one_of.go b/internal/require_if_attribute_is_one_of.go index ab1b7a5..02c5ff9 100644 --- a/internal/require_if_attribute_is_one_of.go +++ b/internal/require_if_attribute_is_one_of.go @@ -46,12 +46,12 @@ func (av RequireIfAttributeIsOneOf) Description(_ context.Context) string { var expectedValueDescritpion string for i, expectedValue := range av.ExceptedValues { if i == len(av.ExceptedValues)-1 { - expectedValueDescritpion += fmt.Sprintf("%q", expectedValue) + expectedValueDescritpion += expectedValue.String() break } - expectedValueDescritpion += fmt.Sprintf("%q, ", expectedValue) + expectedValueDescritpion += expectedValue.String() } - return fmt.Sprintf("If %q attribute is set and the value is one of %s, this attribute is required", av.PathExpression, expectedValueDescritpion) + return fmt.Sprintf("If %s attribute is set and the value is one of %s, this attribute is required", av.PathExpression, expectedValueDescritpion) } func (av RequireIfAttributeIsOneOf) MarkdownDescription(_ context.Context) string { @@ -63,7 +63,7 @@ func (av RequireIfAttributeIsOneOf) MarkdownDescription(_ context.Context) strin } expectedValueDescritpion += fmt.Sprintf("`%s`, ", expectedValue) } - return fmt.Sprintf("If %q attribute is set and the value is one of %s, this attribute is required", av.PathExpression, expectedValueDescritpion) + return fmt.Sprintf("If %s attribute is set and the value is one of %s, this attribute is required", av.PathExpression, expectedValueDescritpion) } func (av RequireIfAttributeIsOneOf) Validate(ctx context.Context, req RequireIfAttributeIsOneOfRequest, res *RequireIfAttributeIsOneOfResponse) { diff --git a/internal/require_if_attribute_is_one_of_test.go b/internal/require_if_attribute_is_one_of_test.go index ea997c9..f0d229a 100644 --- a/internal/require_if_attribute_is_one_of_test.go +++ b/internal/require_if_attribute_is_one_of_test.go @@ -2,9 +2,11 @@ package internal_test import ( "context" + "fmt" "testing" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/tfsdk" @@ -18,16 +20,18 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { t.Parallel() type testCase struct { - req internal.RequireIfAttributeIsOneOfRequest - in path.Expression - exceptedValues []attr.Value - expError bool + req internal.RequireIfAttributeIsOneOfRequest + in path.Expression + inPath path.Path + exceptedValues []attr.Value + expError bool + expErrorMessage string } testCases := map[string]testCase{ "baseString": { req: internal.RequireIfAttributeIsOneOfRequest{ - ConfigValue: types.StringValue("bar value"), + ConfigValue: types.StringNull(), Path: path.Root("bar"), PathExpression: path.MatchRoot("bar"), Config: tfsdk.Config{ @@ -44,19 +48,87 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { }, }, map[string]tftypes.Value{ "foo": tftypes.NewValue(tftypes.String, "excepted value"), - "bar": tftypes.NewValue(tftypes.String, "bar value"), + "bar": tftypes.NewValue(tftypes.String, attr.NullValueString), }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.StringValue("excepted value"), }, - expError: false, + expError: true, + expErrorMessage: "If foo attribute is set and the value is one of \"excepted value\", this attribute is required", + }, + "extendedString": { + req: internal.RequireIfAttributeIsOneOfRequest{ + ConfigValue: types.StringNull(), + Path: path.Root("foobar").AtListIndex(0).AtName("bar2"), + PathExpression: path.MatchRoot("foobar").AtListIndex(0).AtName("bar2"), + Config: tfsdk.Config{ + Schema: schema.Schema{ + Attributes: map[string]schema.Attribute{ + "foo": schema.StringAttribute{}, + "bar": schema.StringAttribute{}, + "foobar": schema.ListNestedAttribute{ + NestedObject: schema.NestedAttributeObject{ + Attributes: map[string]schema.Attribute{ + "bar1": schema.StringAttribute{}, + "bar2": schema.StringAttribute{}, + }, + }, + }, + }, + }, + Raw: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "foo": tftypes.String, + "bar": tftypes.String, + "foobar": tftypes.List{ + ElementType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bar1": tftypes.String, + "bar2": tftypes.String, + }, + }, + }, + }, + }, map[string]tftypes.Value{ + "foo": tftypes.NewValue(tftypes.String, "excepted value"), + "bar": tftypes.NewValue(tftypes.String, "bar value"), + "foobar": tftypes.NewValue(tftypes.List{ + ElementType: tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bar1": tftypes.String, + "bar2": tftypes.String, + }, + }, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "bar1": tftypes.String, + "bar2": tftypes.String, + }, + }, map[string]tftypes.Value{ + "bar1": tftypes.NewValue(tftypes.String, "bar1 excepted value"), + "bar2": tftypes.NewValue(tftypes.String, attr.NullValueString), + }), + }, + ), + }), + }, + }, + in: path.MatchRoot("foobar").AtListIndex(0).AtName("bar1"), + inPath: path.Root("foobar").AtListIndex(0).AtName("bar1"), + exceptedValues: []attr.Value{ + types.StringValue("bar1 excepted value"), + }, + expError: true, + expErrorMessage: "If foobar[0].bar1 attribute is set and the value is one of \"bar1 excepted value\", this attribute is required", }, "baseInt64": { req: internal.RequireIfAttributeIsOneOfRequest{ - ConfigValue: types.StringValue("bar value"), + ConfigValue: types.StringNull(), Path: path.Root("bar"), PathExpression: path.MatchRoot("bar"), Config: tfsdk.Config{ @@ -72,26 +144,28 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { "bar": tftypes.String, }, }, map[string]tftypes.Value{ - "foo": tftypes.NewValue(tftypes.Number, 10), - "bar": tftypes.NewValue(tftypes.String, "bar value"), + "foo": tftypes.NewValue(tftypes.Number, int64(10)), + "bar": tftypes.NewValue(tftypes.String, attr.NullValueString), }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.Int64Value(10), }, - expError: false, + expError: true, + expErrorMessage: "If foo attribute is set and the value is one of 10, this attribute is required", }, "baseBool": { req: internal.RequireIfAttributeIsOneOfRequest{ - ConfigValue: types.StringValue("bar value"), + ConfigValue: types.StringNull(), Path: path.Root("bar"), PathExpression: path.MatchRoot("bar"), Config: tfsdk.Config{ Schema: schema.Schema{ Attributes: map[string]schema.Attribute{ - "foo": schema.Int64Attribute{}, + "foo": schema.BoolAttribute{}, "bar": schema.StringAttribute{}, }, }, @@ -102,19 +176,21 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { }, }, map[string]tftypes.Value{ "foo": tftypes.NewValue(tftypes.Bool, true), - "bar": tftypes.NewValue(tftypes.String, "bar value"), + "bar": tftypes.NewValue(tftypes.String, attr.NullValueString), }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.BoolValue(true), }, - expError: false, + expError: true, + expErrorMessage: "If foo attribute is set and the value is one of true, this attribute is required", }, "path-attribute-is-null": { req: internal.RequireIfAttributeIsOneOfRequest{ - ConfigValue: types.StringValue("bar value"), + ConfigValue: types.StringNull(), Path: path.Root("bar"), PathExpression: path.MatchRoot("bar"), Config: tfsdk.Config{ @@ -131,19 +207,20 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { }, }, map[string]tftypes.Value{ "foo": tftypes.NewValue(tftypes.String, attr.NullValueString), - "bar": tftypes.NewValue(tftypes.String, "bar value"), + "bar": tftypes.NewValue(tftypes.String, attr.NullValueString), }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.StringValue("excepted value"), }, expError: false, }, - "config-attribute-is-null": { + "config-attribute-is-set": { req: internal.RequireIfAttributeIsOneOfRequest{ - ConfigValue: types.StringNull(), + ConfigValue: types.StringValue("excepted value"), Path: path.Root("bar"), PathExpression: path.MatchRoot("bar"), Config: tfsdk.Config{ @@ -160,15 +237,16 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { }, }, map[string]tftypes.Value{ "foo": tftypes.NewValue(tftypes.String, "excepted value"), - "bar": tftypes.NewValue(tftypes.String, attr.NullValueString), + "bar": tftypes.NewValue(tftypes.String, "bar value"), }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.StringValue("excepted value"), }, - expError: true, + expError: false, }, "config-attribute-is-null-and-path-attribute-not-match": { req: internal.RequireIfAttributeIsOneOfRequest{ @@ -193,7 +271,8 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.StringValue("test value"), }, @@ -222,7 +301,8 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { }), }, }, - in: path.MatchRoot("foo"), + in: path.MatchRoot("foo"), + inPath: path.Root("foo"), exceptedValues: []attr.Value{ types.StringValue("test value"), }, @@ -241,8 +321,14 @@ func TestRequireIfAttributeIsOneOfValidator(t *testing.T) { ExceptedValues: test.exceptedValues, }.Validate(context.TODO(), test.req, res) - if test.expError && !res.Diagnostics.HasError() { - t.Fatal("expected error(s), got none") + if test.expError && res.Diagnostics.HasError() { + if !res.Diagnostics.Contains(diag.NewAttributeErrorDiagnostic( + test.inPath, + fmt.Sprintf("Invalid configuration for attribute %s", test.req.Path), + test.expErrorMessage, + )) { + t.Fatal("expected error(s), got none. Error message is : ", res.Diagnostics.Errors()) + } } if !test.expError && res.Diagnostics.HasError() {