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

Update generated IAM acceptance tests to include conditions, or omit IAM conditions from missing test detector #18215

Open
Assignees
Labels
magician Issues and features related to our bespoke CI size/m technical-debt
Milestone

Comments

@SarahFrench
Copy link
Member

What kind of contribution is this issue about?

MMv1-based resource

Details

When new resources are added to the provider and associated IAM resources are added at the same time there are missing test detector messages like this: GoogleCloudPlatform/magic-modules#10757 (comment)

The PR author doesn't have power to update the IAM resource tests because they are generated, and the PR reviewer needs to know context about IAM resource generation.

I imagine that generated IAM resource tests don't include conditions because they aren't available for all IAM resources, which is fair enough! If that is the case it would be good to come up with a way to either stop the missing test detector flagging condition arguments in IAM resources, or enable conditions to be conditionally included in generated IAM acc tests.

References

No response

@SarahFrench
Copy link
Member Author

See also this list: GoogleCloudPlatform/magic-modules#10757 (comment)

@rileykarson rileykarson added magician Issues and features related to our bespoke CI size/m labels May 28, 2024
@rileykarson rileykarson added this to the Goals milestone May 28, 2024
@melinath melinath assigned melinath and shuyama1 and unassigned shuyama1 and melinath May 28, 2024
@shuyama1 shuyama1 assigned trodge and unassigned shuyama1 Jun 10, 2024
@melinath
Copy link
Collaborator

Weirdly we still add the conditions field to the IAM resources regardless - we just don't document them.

@melinath
Copy link
Collaborator

Unfortunately the missing test detector does not run at generation time, and I don't think we have a great way to introspect whether conditions are supported. So the best option really might be to just ignore the conditions field in the missing test detector, since we know they're generated if supported.

Alternatively, the "real" fix would be to not support the fields unless conditions are supported - but that would be a breaking change and a much larger one as well.

Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 Jul 19, 2024
@melinath melinath reopened this Nov 14, 2024
@melinath
Copy link
Collaborator

@trodge it looks like the omission isn't working - GoogleCloudPlatform/magic-modules#12181 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.