Skip to content

fix AwsDomainStoreTest test exception, aws.disableEc2Metadata true#3287

Merged
abvaidya merged 2 commits intoAthenZ:masterfrom
Bhuff1:feature/awsdomainstoretest_fails
Apr 8, 2026
Merged

fix AwsDomainStoreTest test exception, aws.disableEc2Metadata true#3287
abvaidya merged 2 commits intoAthenZ:masterfrom
Bhuff1:feature/awsdomainstoretest_fails

Conversation

@Bhuff1
Copy link
Copy Markdown
Contributor

@Bhuff1 Bhuff1 commented Apr 8, 2026

Description

  • In some CI/CD environments a different exception is thrown for a unit test in AwsDomainStoreTest.
  • The unit test should have a finally block to ensure cleanup is done regardless of failure.
  • For the server_aws_common module, we should not let the unit tests access aws.disableEc2Metadata as it will cause tests to fail in unexpected and difficult to debug ways. Addresses issue raised here.

Contribution Checklist:

  • The pull request does not introduce any breaking changes
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Signed-off-by: Benjamin Huff <benjaminehuff@gmail.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Maven configuration to disable EC2 IMDS during tests to prevent real AWS calls and improves the error handling in AwsDomainStoreTest by using a finally block for property cleanup and broadening the expected exception types. I have provided feedback regarding the need for an explicit failure case when an exception is not thrown, the requirement to properly configure dummy properties for test coverage, and a correction for a log message inconsistency.

Signed-off-by: Benjamin Huff <benjaminehuff@gmail.com>
@abvaidya abvaidya merged commit ced7be1 into AthenZ:master Apr 8, 2026
3 checks passed
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