Add unit testing for IsAWSErr() and IsAWSErrExtended(), prevent missing OrigErr() panic #16
Conversation
…ng OrigErr() panic Reference: hashicorp/terraform-provider-aws#10670 This was originally to just add unit testing to match testing added to the Terraform AWS Provider codebase, but discovered and fixed a panic condition in `IsAWSErrExtended()`. In the future these functions will be updated to support Go 1.13 error wrapping via `errors.As()` so we can easily add error context without losing the error type checking, however this is waiting for github.com/hashicorp/terraform to begin releases via Go 1.13.
| Err error | ||
| Code string | ||
| Message string | ||
| Expected bool |
There was a problem hiding this comment.
I don't see Expected set on most of the test cases, are you relying on a default behavior?
There was a problem hiding this comment.
In Go, each type has an associated zero value and for bool it is false. I'm not sure I necessarily have good reference of linters that check for this specifically or documentation for whether or not its idiomatic to rely on zero values in composite literals other than maybe this section of Effective Go. If we'd prefer explicitly writing out Expected: false, in places though, can certainly update this.
There was a problem hiding this comment.
It's consistent so I think this is ok. I have a general preference for explicit defaults -- but I'm still getting used to the Go testing framework, so I'm not going to be pushy about what that should look like.
Reference: hashicorp/terraform-provider-aws#10670
This was originally to just add unit testing to match testing added to the Terraform AWS Provider codebase, but discovered and fixed a panic condition in
IsAWSErrExtended().In the future these functions will be updated to support Go 1.13 error wrapping via
errors.As()so we can easily add error context without losing the error type checking, however this is waiting for github.com/hashicorp/terraform to begin releases via Go 1.13.