Skip to content
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

Adding resource.TestCheckResourceAttrWith test utility #950

Merged
merged 6 commits into from Apr 28, 2022

Conversation

detro
Copy link
Contributor

@detro detro commented Apr 28, 2022

Closes #948

@detro detro requested a review from as a code owner Apr 28, 2022
helper/resource/testing_test.go Show resolved Hide resolved
Copy link
Member

@bflad bflad left a comment

Overall I think this looks pretty good, just a few minor things to consider.

.changelog/950.txt Outdated Show resolved Hide resolved
// TestCheckResourceAttrWith - as per TestCheckResourceAttr but the attribute value
// checking logic can be customised.
//
// If a value is found for the given name and key, it is passed to the CheckResourceAttrWithFunc function.
// The CheckResourceAttrWithFunc function can then apply any checking logic:
// should return an error for the check to fail, or `nil` to succeed.
Copy link
Member

@bflad bflad Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this description here, I'm just wondering if we should also give provider developers all the usage information when they reference it in Go documentation or via the Go language server (e.g. hovering in an editor), similar to the other TestCheckFunc

// TestCheckResourceAttrWith ensures a specific value is stored in state for
// the given name and key combination, by checking it against a custom
// validation function. State value checking is only recommended for testing
// Computed attributes and attribute defaults.
//
// For managed resources, the name parameter is combination of the resource
// type, a period (.), and the name label. The name for the below example
// configuration would be "myprovider_thing.example".
//
//     resource "myprovider_thing" "example" { ... }
//
// For data sources, the name parameter is a combination of the keyword "data",
// a period (.), the data source type, a period (.), and the name label. The
// name for the below example configuration would be
// "data.myprovider_thing.example".
//
//     data "myprovider_thing" "example" { ... }
//
// The key parameter is an attribute path in Terraform CLI 0.11 and earlier
// "flatmap" syntax. Keys start with the attribute name of a top-level
// attribute. Use the following special key syntax to inspect list, map, and
// set attributes:
//
//     - .{NUMBER}: List value at index, e.g. .0 to inspect the first element.
//        Use the TestCheckTypeSet* and TestMatchTypeSet* functions instead
//        for sets.
//     - .{KEY}: Map value at key, e.g. .example to inspect the example key
//        value.
//
// The f parameter is a function containing the custom value validation logic.
// Any returned errors from that function will cause the check and test to fail.

Copy link
Contributor Author

@detro detro Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I'll update.

Copy link
Contributor Author

@detro detro Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have provided a much more comprehensive godoc for this function.

Let me know your thoughts

helper/resource/testing.go Outdated Show resolved Hide resolved
helper/resource/testing_example_test.go Outdated Show resolved Hide resolved
helper/resource/testing_example_test.go Outdated Show resolved Hide resolved
@detro detro requested a review from bflad Apr 28, 2022
@bflad bflad added enhancement subsystem/tests labels Apr 28, 2022
@bflad bflad added this to the v2.15.0 milestone Apr 28, 2022
bflad
bflad approved these changes Apr 28, 2022
Copy link
Member

@bflad bflad left a comment

Looks good to me 🚀

@detro detro merged commit 5adf5f1 into main Apr 28, 2022
5 checks passed
@detro detro deleted the detro/Adding_TestCheckWith branch Apr 28, 2022
// this configuration and test is shown for illustrative purposes.

// Verify the attribute value is an integer, and it's between 5 (included) and 20 (excluded)
resource.TestCheckResourceAttrWith("example_thing.test", "example_string_attribute", func(value string) error {
Copy link

@inkel inkel May 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the second parameter be example_int_attribute in this case?

@github-actions
Copy link

@github-actions github-actions bot commented Jun 10, 2022

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement subsystem/tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants