-
Couldn't load subscription status.
- Fork 11
[CONTP-962] Add tests for IAM role names with paths. #42
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
[CONTP-962] Add tests for IAM role names with paths. #42
Conversation
|
|
||
| taskRoleArn := task["task_role_arn"] | ||
| s.NotEmpty(taskRoleArn, "Task role ARN should not be empty") | ||
| s.Contains(taskRoleArn, "/terraform-test/", "Task role ARN should contain the path '/test-path/'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: expected role path
| s.Contains(taskRoleArn, "/terraform-test/", "Task role ARN should contain the path '/test-path/'") | |
| s.Contains(taskRoleArn, "/terraform-test/", "Task role ARN should contain the path '/terraform-test/'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with small comment. Thanks for adding these tests!
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
mergequeue build completed successfully, but the github api returned an error while merging the pr DetailsError: PUT https://api.github.com/repos/DataDog/terraform-aws-ecs-datadog/pulls/42/merge: 405 Merge commits are not allowed on this repository. [] (Request ID: 8C34:89BE4:7FD034:1CE968D:68C0877D) FullStacktrace: |
What does this PR do?
PR #40 fixed role arn name parsing to account for a role's
path. This PR adds unit tests to validate that change/Motivation
Improve test coverage for edge cases like the one identified in the merged PR.
Describe how you validated your changes
Additional Notes