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

Create VpcAssociationPolicy e2e test #417

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

zijun726911
Copy link
Contributor

@zijun726911 zijun726911 commented Sep 29, 2023

What type of PR is this? e2e test cases for the security group api feature #199

What does this PR do / Why do we need it: e2e test cases for the security group api feature #199

If an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue:

Testing done on this change:

Automation added to e2e: Add VpcAssociationPolicy e2e test cases, whole test suite pass

Ran 34 of 34 Specs in 2230.206 seconds
SUCCESS! -- 34 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestIntegration (2230.52s)
PASS

Will this PR introduce any new dependencies?: No

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -362,23 +368,23 @@ func (env *Framework) VerifyTargetGroupNotFound(tg *vpclattice.TargetGroupSummar
}).Should(Succeed())
}

func (env *Framework) IsVpcAssociatedWithServiceNetwork(ctx context.Context, vpcId string, serviceNetwork *vpclattice.ServiceNetworkSummary) (bool, error) {
func (env *Framework) IsVpcAssociatedWithServiceNetwork(ctx context.Context, vpcId string, serviceNetwork *vpclattice.ServiceNetworkSummary) (bool, string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

It's typically considered an antipattern to return multiple values like this. Shouldn't we return a struct type containing named properties, or error?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you return association id you can omit bool value. Use empty id as false value.

Copy link
Member

@xWink xWink left a comment

Choose a reason for hiding this comment

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

Test logic looks good. Just left a nitpick comment, approving!

@zijun726911 zijun726911 merged commit fa26532 into aws:main Oct 5, 2023
5 checks passed
@zijun726911 zijun726911 deleted the vap-e2e-test branch November 2, 2023 22:33
@zijun726911 zijun726911 restored the vap-e2e-test branch May 18, 2024 02:08
@zijun726911 zijun726911 deleted the vap-e2e-test branch May 22, 2024 23:55
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.

3 participants