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

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

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 added 3 commits March 25, 2025 11:21
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 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 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants