refactor(core-tests): Convert DescriptionAuthorizerService tests to java #6140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
January 24, 2024 16:18
We want to refactor from groovy to java tests as much as possible. This refactors the groovy tests in DescriptionAuthorizerServiceSpec to a DescriptionAuthorizerServiceTest class in java.
Adds some test coverage on the DescriptionAuthorizerServiceTest, particularly with the metrics that it is testing, as well as adding a test for the happy path. Also refactors some of the mocking in the DescriptionAuthorizerServiceTest. It has a AccountDefinitionSecretManager as a dependency, and rather than mocking that class, the test was mocking the dependencies of that class. This led to an unnecessary level of testing/mocking for a unit test, because the only way to setup the right mocking values was to mock the dependency of the dependency. Rather than do this, it's better to just setup the secretManager as a mock itself. Part of this refactor is to add some missing tests for the DefaultAccountSecurityPolicy class. These tests never existed in the first place, and because DefaultAccountSecurityPolicy is down the dependency chain from AccountDefinitionSecretManager (which wasn't properly mocked in DescriptionAuthorizerServiceTest), the code in this class was being tested in the DescriptionAuthorizerServiceTest class. To simplify the mocking and testing code, it's better to have test this code in a self contained testing class.
dbyron-sf
approved these changes
Jan 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to refactor from groovy to java tests as much as possible. This refactors the groovy tests in DescriptionAuthorizerServiceSpec to a DescriptionAuthorizerServiceTest class in java.
Adds some test coverage on the DescriptionAuthorizerServiceTest, particularly with the metrics that it is testing, as well as adding a test for the happy path.
Also refactors some of the mocking in the DescriptionAuthorizerServiceTest. It has a AccountDefinitionSecretManager as a dependency, and rather than mocking that class, the test was mocking the dependencies of that class. This led to an unnecessary level of testing/mocking for a unit test, because the only way to setup the right mocking values was to mock the dependency of the dependency. Rather than do this, it's better to just setup the secretManager as a mock itself.
Part of this refactor is to add some missing tests for the DefaultAccountSecurityPolicy class. These tests never existed in the first place, and because DefaultAccountSecurityPolicy is down the dependency chain from AccountDefinitionSecretManager (which wasn't properly mocked in DescriptionAuthorizerServiceTest), the code in this class was being tested in the DescriptionAuthorizerServiceTest class. To simplify the mocking and testing code, it's better to have test this code in a self contained testing class.