Skip to content

TFECO-9157: Add tests for identity_data.go and switch to multiple field reader #1453

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

Merged
merged 4 commits into from
Mar 26, 2025

Conversation

ansgarm
Copy link
Member

@ansgarm ansgarm commented Mar 21, 2025

This PR adds tests for identity_data.go inspired by existing tests for resource_data.go.

This uncovered a flaw in the current implementation, when it comes to non-string identity attributes which currently don't work.

To quote the relevant commit message on this:

Switch to using multiple field readers in IdentityData

This resembles the behaviour in ResourceData more closely. Specifically,
this makes it possible to have number values in the source identity sent by
Terraform that are encoded as strings. Before this commit this failed as the
field writer does not allow to write strings for fields that are typed as
integers in the schema, which is what we did to initialize the result.

@ansgarm ansgarm force-pushed the test-identity-data-go branch from 55bb270 to 2c11511 Compare March 25, 2025 10:02
@ansgarm ansgarm changed the title tests for identity_data.go Add tests for identity_data.go Mar 25, 2025
@ansgarm ansgarm force-pushed the test-identity-data-go branch from 06cf479 to 5eeaa23 Compare March 25, 2025 10:21
@ansgarm ansgarm marked this pull request as ready for review March 25, 2025 10:21
@ansgarm ansgarm requested a review from a team as a code owner March 25, 2025 10:21
@ansgarm ansgarm changed the title Add tests for identity_data.go Add tests for identity_data.go and switch to multiple field reader Mar 25, 2025
@ansgarm ansgarm changed the title Add tests for identity_data.go and switch to multiple field reader TFECO-9157: Add tests for identity_data.go and switch to multiple field reader Mar 25, 2025
austinvalle
austinvalle previously approved these changes Mar 25, 2025
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

ansgarm added 4 commits March 26, 2025 17:19
This resembles the behaviour in `ResourceData` more closely. Specifically,
this makes it possible to have number values in the source identity sent by
Terraform that are encoded as strings. Before this commit this failed as the
field writer does not allow to write strings for fields that are typed as
integers in the schema, which is what we did to initialize the result.
@ansgarm ansgarm force-pushed the test-identity-data-go branch from 8ef122c to ddb027f Compare March 26, 2025 16:19
@ansgarm ansgarm merged commit 8032732 into main Mar 26, 2025
4 of 5 checks passed
@ansgarm ansgarm deleted the test-identity-data-go branch March 26, 2025 16:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants