(WIP) Add tests for DynamicPseudoType behavior #208
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ require ( | |
| github.com/hashicorp/terraform-plugin-go v0.21.0 | ||
| github.com/hashicorp/terraform-plugin-mux v0.14.0 | ||
| github.com/hashicorp/terraform-plugin-sdk/v2 v2.32.0 | ||
| github.com/hashicorp/terraform-plugin-testing v1.6.1-0.20240130155516-51777dda582e | ||
|
There was a problem hiding this comment. I'll tag a release once we've finalised and merged hashicorp/terraform-plugin-testing#276 |
||
| ) | ||
|
|
||
| require ( | ||
|
|
@@ -24,7 +24,7 @@ require ( | |
| github.com/hashicorp/go-checkpoint v0.5.0 // indirect | ||
| github.com/hashicorp/go-cleanhttp v0.5.2 // indirect | ||
| github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 // indirect | ||
| github.com/hashicorp/go-hclog v1.6.2 // indirect | ||
| github.com/hashicorp/go-immutable-radix v1.3.0 // indirect | ||
| github.com/hashicorp/go-multierror v1.1.1 // indirect | ||
| github.com/hashicorp/go-plugin v1.6.0 // indirect | ||
|
|
@@ -53,7 +53,6 @@ require ( | |
| github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect | ||
| github.com/zclconf/go-cty v1.14.2 // indirect | ||
| golang.org/x/crypto v0.18.0 // indirect | ||
| golang.org/x/mod v0.14.0 // indirect | ||
| golang.org/x/net v0.18.0 // indirect | ||
| golang.org/x/sys v0.16.0 // indirect | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| // dynamic6provider contains tests that verify the behavior of DynamicPseudoType in terraform-plugin-go | ||
| package dynamic6provider |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,253 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| package dynamic6provider_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/hashicorp/terraform-plugin-go/tfprotov6" | ||
| "github.com/hashicorp/terraform-plugin-go/tftypes" | ||
| r "github.com/hashicorp/terraform-plugin-testing/helper/resource" | ||
| "github.com/hashicorp/terraform-plugin-testing/knownvalue" | ||
| "github.com/hashicorp/terraform-plugin-testing/statecheck" | ||
| "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" | ||
| "github.com/hashicorp/terraform-provider-corner/internal/testing/testprovider" | ||
| "github.com/hashicorp/terraform-provider-corner/internal/testing/testsdk/providerserver" | ||
| "github.com/hashicorp/terraform-provider-corner/internal/testing/testsdk/resource" | ||
| ) | ||
|
|
||
| func Test_Dynamic_Attribute_ListType(t *testing.T) { | ||
| r.UnitTest(t, r.TestCase{ | ||
| Steps: []r.TestStep{ | ||
| { | ||
| // This may eventually be considered an invalid schema. It currently behaves as expected but may not be exposed to provider developers | ||
| // to avoid confusion. While the element type is dynamic, all elements must still have the exact same type, which Terraform will achieve | ||
| // by either performing a conversion (like below, converting 12345 to "12345"), or throw an error if conversion is impossible, for example: | ||
| // | ||
| // Error: Incorrect attribute value type | ||
| // | ||
| // on terraform_plugin_test.tf line 12, in resource "corner_dynamic_thing" "foo": | ||
| // 12: attribute_with_dpt = ["hey", { number = 12345 }] | ||
| // | ||
| // Inappropriate value for attribute "attribute_with_dpt": all list elements must have the same type. | ||
| // | ||
| // Related issue: https://github.com/hashicorp/terraform/issues/34574 | ||
| Config: `resource "corner_dynamic_thing" "foo" { | ||
| attribute_with_dpt = ["hey", 12345] | ||
| }`, | ||
| ConfigStateChecks: []statecheck.StateCheck{ | ||
| statecheck.ExpectKnownValue( | ||
| "corner_dynamic_thing.foo", | ||
| tfjsonpath.New("attribute_with_dpt"), | ||
| knownvalue.ListExact([]knownvalue.Check{ | ||
| knownvalue.StringExact("hey"), | ||
|
There was a problem hiding this comment. Might not be appropriate for this PR, or this repo even, but if it's not documented somewhere, is it worth providing some information about the rules that Terraform applies when there is ambiguity in the types specified in configuration, in this instance a string and an integer, and how that is handled in the context of a attributes that use There was a problem hiding this comment. I'm not sure it's documented in the context you're describing, but it is an interesting thought I also share. Just observing the behavior of Terraform/cty it looks potentially arbitrary, but digging more into
If we switch the config around to: resource "corner_dynamic_thing" "foo" {
attribute_with_dpt = [12345, "hey"]
}We receive the same element type as {
"attribute_with_dpt": [
[
"\"string\"",
"12345"
],
[
"\"string\"",
"hey"
]
]
}It would be nice to get some confirmation on this behavior, so I will note this down as something to ask the core team so we can understand a little deeper 👍🏻 |
||
| knownvalue.StringExact("12345"), | ||
| }), | ||
| ), | ||
| }, | ||
| }, | ||
| }, | ||
| ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ | ||
| "corner": providerserver.NewProviderServer(testprovider.Provider{ | ||
| Resources: map[string]testprovider.Resource{ | ||
| "corner_dynamic_thing": { | ||
| SchemaResponse: &resource.SchemaResponse{ | ||
| Schema: &tfprotov6.Schema{ | ||
| Block: &tfprotov6.SchemaBlock{ | ||
|
There was a problem hiding this comment. Because I spend far more time looking at a schema at the "Framework-level", scanning |
||
| Attributes: []*tfprotov6.SchemaAttribute{ | ||
| { | ||
| Name: "attribute_with_dpt", | ||
| Required: true, | ||
| Type: tftypes.List{ | ||
| ElementType: tftypes.DynamicPseudoType, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| func Test_Dynamic_Attribute_MapType(t *testing.T) { | ||
| r.UnitTest(t, r.TestCase{ | ||
| Steps: []r.TestStep{ | ||
| { | ||
| // This may eventually be considered an invalid schema. It currently behaves as expected but may not be exposed to provider developers | ||
| // to avoid confusion. While the element type is dynamic, all elements must still have the exact same type, which Terraform will achieve | ||
| // by either performing a conversion (like below, converting 12345 to "12345"), or throw an error if conversion is impossible, for example: | ||
| // | ||
| // Error: Incorrect attribute value type | ||
| // | ||
| // on terraform_plugin_test.tf line 12, in resource "corner_dynamic_thing" "foo": | ||
| // attribute_with_dpt = { | ||
| // "key1" = "hey" | ||
| // "key2" = { | ||
| // number = 12345 | ||
| // } | ||
| // } | ||
| // | ||
| // Inappropriate value for attribute "attribute_with_dpt": all map elements must have the same type. | ||
| // | ||
| // Related issue: https://github.com/hashicorp/terraform/issues/34574 | ||
|
Comment on lines
+81
to
+97
There was a problem hiding this comment. ❤️ the extensive notes/documentation. |
||
| Config: `resource "corner_dynamic_thing" "foo" { | ||
| attribute_with_dpt = { | ||
| "key1" = "hey" | ||
| "key2" = 12345 | ||
| } | ||
| }`, | ||
| ConfigStateChecks: []statecheck.StateCheck{ | ||
| statecheck.ExpectKnownValue( | ||
| "corner_dynamic_thing.foo", | ||
| tfjsonpath.New("attribute_with_dpt"), | ||
| knownvalue.MapExact(map[string]knownvalue.Check{ | ||
| "key1": knownvalue.StringExact("hey"), | ||
| "key2": knownvalue.StringExact("12345"), | ||
| }), | ||
| ), | ||
| }, | ||
| }, | ||
| }, | ||
| ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ | ||
| "corner": providerserver.NewProviderServer(testprovider.Provider{ | ||
| Resources: map[string]testprovider.Resource{ | ||
| "corner_dynamic_thing": { | ||
| SchemaResponse: &resource.SchemaResponse{ | ||
| Schema: &tfprotov6.Schema{ | ||
| Block: &tfprotov6.SchemaBlock{ | ||
| Attributes: []*tfprotov6.SchemaAttribute{ | ||
| { | ||
| Name: "attribute_with_dpt", | ||
| Required: true, | ||
| Type: tftypes.Map{ | ||
| ElementType: tftypes.DynamicPseudoType, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| func Test_Dynamic_Attribute_SetType(t *testing.T) { | ||
| r.UnitTest(t, r.TestCase{ | ||
| Steps: []r.TestStep{ | ||
| { | ||
| // This may eventually be considered an invalid schema. It currently behaves as expected but may not be exposed to provider developers | ||
| // to avoid confusion. While the element type is dynamic, all elements must still have the exact same type, which Terraform will achieve | ||
| // by either performing a conversion (like below, converting 12345 to "12345"), or throw an error if conversion is impossible, for example: | ||
| // | ||
| // Error: Incorrect attribute value type | ||
| // | ||
| // on terraform_plugin_test.tf line 12, in resource "corner_dynamic_thing" "foo": | ||
| // attribute_with_dpt = ["hey", { number = 12345 }] | ||
| // | ||
| // Inappropriate value for attribute "attribute_with_dpt": all set elements must have the same type. | ||
| // | ||
| // Related issue: https://github.com/hashicorp/terraform/issues/34574 | ||
| Config: `resource "corner_dynamic_thing" "foo" { | ||
| attribute_with_dpt = ["hey", 12345] | ||
| }`, | ||
| ConfigStateChecks: []statecheck.StateCheck{ | ||
| statecheck.ExpectKnownValue( | ||
| "corner_dynamic_thing.foo", | ||
| tfjsonpath.New("attribute_with_dpt"), | ||
| knownvalue.SetExact([]knownvalue.Check{ | ||
| knownvalue.StringExact("hey"), | ||
| knownvalue.StringExact("12345"), | ||
| }), | ||
| ), | ||
| }, | ||
| }, | ||
| }, | ||
| ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ | ||
| "corner": providerserver.NewProviderServer(testprovider.Provider{ | ||
| Resources: map[string]testprovider.Resource{ | ||
| "corner_dynamic_thing": { | ||
| SchemaResponse: &resource.SchemaResponse{ | ||
| Schema: &tfprotov6.Schema{ | ||
| Block: &tfprotov6.SchemaBlock{ | ||
| Attributes: []*tfprotov6.SchemaAttribute{ | ||
| { | ||
| Name: "attribute_with_dpt", | ||
| Required: true, | ||
| Type: tftypes.Set{ | ||
| ElementType: tftypes.DynamicPseudoType, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| func Test_Dynamic_Attribute_TupleType(t *testing.T) { | ||
| r.UnitTest(t, r.TestCase{ | ||
| Steps: []r.TestStep{ | ||
| { | ||
| Config: `resource "corner_dynamic_thing" "foo" { | ||
| attribute_with_dpt = ["hey", { number = 12345 }, ["there", "tuple"]] | ||
| }`, | ||
| ConfigStateChecks: []statecheck.StateCheck{ | ||
| statecheck.ExpectKnownValue( | ||
| "corner_dynamic_thing.foo", | ||
| tfjsonpath.New("attribute_with_dpt"), | ||
| // The type is stored as a tuple, but we can still use `ListExact` to verify state | ||
| knownvalue.ListExact([]knownvalue.Check{ | ||
| knownvalue.StringExact("hey"), | ||
| knownvalue.ObjectExact(map[string]knownvalue.Check{ | ||
| "number": knownvalue.Int64Exact(12345), | ||
| }), | ||
| knownvalue.ListExact([]knownvalue.Check{ | ||
| knownvalue.StringExact("there"), | ||
| knownvalue.StringExact("tuple"), | ||
| }), | ||
| }), | ||
|
Comment on lines
+210
to
+220
There was a problem hiding this comment. @bflad @bendbennett I hesitate to suggest we add a There was a problem hiding this comment. Interesting. If there are legitimate use-cases for |
||
| ), | ||
| }, | ||
| }, | ||
| }, | ||
| ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ | ||
| "corner": providerserver.NewProviderServer(testprovider.Provider{ | ||
| Resources: map[string]testprovider.Resource{ | ||
| "corner_dynamic_thing": { | ||
| SchemaResponse: &resource.SchemaResponse{ | ||
| Schema: &tfprotov6.Schema{ | ||
| Block: &tfprotov6.SchemaBlock{ | ||
| Attributes: []*tfprotov6.SchemaAttribute{ | ||
| { | ||
| Name: "attribute_with_dpt", | ||
| Required: true, | ||
| Type: tftypes.Tuple{ | ||
| ElementTypes: []tftypes.Type{ | ||
| tftypes.String, | ||
| tftypes.DynamicPseudoType, | ||
| tftypes.DynamicPseudoType, | ||
|
Comment on lines
+239
to
+240
There was a problem hiding this comment. Not directly related to this PR, but given that within There was a problem hiding this comment. Right, I think the documentation and wording will be important for framework developers to understand. As described by
|
||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| }, | ||
| }) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice to not expand our packages here and use protocol(v6)provider because it means updating the terraform-plugin-go CI and I'm not sure it offers much benefit over having all the terraform-plugin-go tests in one package per protocol version. Maybe we can introduce the testsdk and update the existing provider in a separate PR, then the dynamic tests are additive to the existing provider?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good shout and will make even more sense once framework implements dynamic types and we add tests for that.
I think I can wait for @bendbennett's work on #210 to merge, then will add all my tests to the
protocolv6provider+protocolproviderand switch them all to using thetestsdk.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#210 is almost complete with the exception of dealing with hashicorp/terraform-plugin-framework#914. I'll be looking into that imminently, but don't let me hold you up in terms of merging this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't plan on opening this PR for review until the dynamic type work in
terraform-plugin-frameworkis nearing completion (which I just started), so I think we still have some time, whoever gets to it first 😆After that work is done, I want to make sure we also have a discussion with the core team to ensure we aren't exposing dynamics in any way that could be invalid/confusing.