-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tfprotov5/tf5server: Ensure server options are passed through on startup #153
Conversation
Reference: #152 Currently this functionality cannot implement unit testing without a larger refactoring effort as there is no way to retrieve the generated `ServeConfig` and `plugin.ServeConfig` inside the function. While refactoring that logic into an unexported function such as: ```go func prepareServeConfigs(providerAddress string, providerServerFunc func() tfprotov5.ProviderServer, opts ...ServeOpt) (*ServeConfig, *plugin.ServeConfig, error) ``` Would be fairly straightforward, the `ServeConfig` itself implements all unexported fields which presents some tougher challenges with `go-cmp`: ``` panic: cannot handle unexported field at {*tf5server.ServeConfig}.logger: "github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server".ServeConfig consider using a custom Comparer; if you control the implementation of type, you can also consider using an Exporter, AllowUnexported, or cmpopts.IgnoreUnexported ``` This chooses instead to fix the implementation to match `tf6server`, rather than bundle all this unrelated work into the simple bug fix. Previously: ```console ❯ TF_ACC=1 go test -timeout=1m -v ./internal/sdkv2provider === RUN TestProvider --- PASS: TestProvider (0.00s) === RUN TestProvider_impl --- PASS: TestProvider_impl (0.00s) === RUN TestAccTests === RUN TestAccTests/corner_user {"@caller":"/Users/bflad/go/pkg/mod/github.com/hashicorp/[email protected]/internal/logging/protocol.go:21","@Level":"trace","@message":"Received request","@module":"sdk.proto","@timestamp":"2022-02-04T16:28:52.602080-05:00","EXTRA_VALUE_AT_END":null,"tf_proto_version":"5.2","tf_provider_addr":"","tf_req_id":"a24b8a29-628a-01c2-0516-08b260627109","tf_rpc":"GetProviderSchema"} {"@caller":"/Users/bflad/go/pkg/mod/github.com/hashicorp/[email protected]/internal/logging/protocol.go:21","@Level":"trace","@message":"Calling downstream","@module":"sdk.proto","@timestamp":"2022-02-04T16:28:52.602157-05:00","EXTRA_VALUE_AT_END":null,"tf_proto_version":"5.2","tf_provider_addr":"","tf_req_id":"a24b8a29-628a-01c2-0516-08b260627109","tf_rpc":"GetProviderSchema"} {"@caller":"/Users/bflad/go/pkg/mod/github.com/hashicorp/[email protected]/internal/logging/protocol.go:21","@Level":"trace","@message":"Called downstream","@module":"sdk.proto","@timestamp":"2022-02-04T16:28:52.602257-05:00","EXTRA_VALUE_AT_END":null,"tf_proto_version":"5.2","tf_provider_addr":"","tf_req_id":"a24b8a29-628a-01c2-0516-08b260627109","tf_rpc":"GetProviderSchema"} {"@caller":"/Users/bflad/go/pkg/mod/github.com/hashicorp/[email protected]/internal/logging/protocol.go:21","@Level":"trace","@message":"Served request","@module":"sdk.proto","@timestamp":"2022-02-04T16:28:52.602299-05:00","EXTRA_VALUE_AT_END":null,"tf_proto_version":"5.2","tf_provider_addr":"","tf_req_id":"a24b8a29-628a-01c2-0516-08b260627109","tf_rpc":"GetProviderSchema"} {"@caller":"/Users/bflad/go/pkg/mod/github.com/hashicorp/[email protected]/internal/logging/protocol.go:21","@Level":"trace","@message":"Received request","@module":"sdk.proto","@timestamp":"2022-02-04T16:28:52.666614-05:00","EXTRA_VALUE_AT_END":null,"tf_proto_version":"5.2","tf_provider_addr":"","tf_req_id":"97a74410-24d3-4d4e-c61c-9062200c75e8","tf_rpc":"GetProviderSchema"} ... many more ... ``` Now: ```console ❯ TF_ACC=1 go test -timeout=1m -v ./internal/sdkv2provider === RUN TestProvider --- PASS: TestProvider (0.00s) === RUN TestProvider_impl --- PASS: TestProvider_impl (0.00s) === RUN TestAccTests === RUN TestAccTests/corner_regions_cty === RUN TestAccTests/corner_user === RUN TestAccTests/corner_regions === RUN TestAccTests/corner_bigint_data === RUN TestAccTests/corner_bigint === RUN TestAccTests/corner_user_cty --- PASS: TestAccTests (6.76s) --- PASS: TestAccTests/corner_regions_cty (1.24s) --- PASS: TestAccTests/corner_user (1.02s) --- PASS: TestAccTests/corner_regions (0.92s) --- PASS: TestAccTests/corner_bigint_data (0.92s) --- PASS: TestAccTests/corner_bigint (0.97s) --- PASS: TestAccTests/corner_user_cty (1.69s) PASS ok github.com/hashicorp/terraform-provider-corner/internal/sdkv2provider 7.510s ```
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.
LGTM
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Closes #152
Currently this functionality cannot implement unit testing without a larger refactoring effort as there is no way to retrieve the generated
ServeConfig
andplugin.ServeConfig
inside the function. While refactoring that logic into an unexported function such as:Would be fairly straightforward, the
ServeConfig
itself implements all unexported fields which presents some tougher challenges withgo-cmp
:This chooses instead to fix the implementation to match
tf6server
, rather than bundle all this unrelated work into the simple bug fix.Previously:
Now: