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

Skip blocks while marking computed nils unknown #555

merged 1 commit into from Nov 28, 2022

Skip blocks while marking computed nils unknown #555

merged 1 commit into from Nov 28, 2022


Copy link

@jar-b jar-b commented Nov 28, 2022

Relates #483
Relates hashicorp/terraform-provider-corner#89
Relates hashicorp/terraform-provider-aws#27857 (acceptance tests pass after this fix)

MarkComputedNilsAsUnknown now passes block values through unmodified, rather than throwing an error.

I extended the single large test object to add block attributes, but I can also split into individual test cases if that'd be helpful. Original issue is reproduced with the updated test case and the fix commented out:

$ go test -count=1 -run "^TestMarkComputedNilsAsUnknown$" ./internal/fwserver/...
--- FAIL: TestMarkComputedNilsAsUnknown (0.00s)
    server_planresourcechange_test.go:283: Unexpected error: AttributeName("block-nil-optional-computed"): couldn't find attribute in resource schema: path leads to block, not an attribute
FAIL       0.353s

Edit: #552 included the change to ignore block type errors, so this PR has been updated to just extend test coverage.

Copy link

hashicorp-cla commented Nov 28, 2022

CLA assistant check
All committers have signed the CLA.

@jar-b jar-b marked this pull request as ready for review November 28, 2022 20:24
@jar-b jar-b requested a review from a team as a code owner November 28, 2022 20:24
Copy link

bflad commented Nov 28, 2022

Hi @jar-b 👋 Thank you for submitting this change. This appears to overlap with #552 which was recently merged, however this change does include additional TestMarkComputedNilsAsUnknown testing. If you're willing or able, I'd suggest rebasing this against main and dropping the (now duplicate) changelog entry.

additional test coverage for the MarkComputedNilsAsUnknown changes
added in #552
@bflad bflad added this to the v0.17.0 milestone Nov 28, 2022
@bflad bflad self-assigned this Nov 28, 2022
Copy link

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks so much @jar-b 🚀

@bflad bflad merged commit 1dfcd30 into hashicorp:main Nov 28, 2022
16 checks passed
@jar-b jar-b deleted the b-computed-nil-blocks branch November 28, 2022 21:26
Copy link

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 Dec 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants