DATAGO-64300: Test negative scenarios #135
Conversation
…ot errors and logs. Error logs are determined by the log level.
…directory. Also, ignore the tests when running unit tests
| @@ -305,6 +305,9 @@ | |||
| <configuration> | |||
| <!--suppress UnresolvedMavenProperty --> | |||
| <argLine>${jacocoSurefire}</argLine> | |||
There was a problem hiding this comment.
We don't want the "real" tests to run as part of unit tests
| Map<String, String> envVars; | ||
| try { | ||
| envVars = setBrokerSpecificEnvVars(request.getServiceId()); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Added error handling when we are unable to find the messaging service from the service id
| switch (command.getCommandType()) { | ||
| case terraform: | ||
| terraformManager.execute(commandMapper.map(request), command, envVars); | ||
| break; |
There was a problem hiding this comment.
Send back an error when the command type is unknown
| .build()); | ||
| break; | ||
| } | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Send back an error if any other type of exception occurs.
| @Autowired | ||
| private EventPortalProperties eventPortalProperties; | ||
|
|
||
| @Test |
There was a problem hiding this comment.
For now, just make sure the application starts up in standalone mode.
| @@ -0,0 +1,130 @@ | |||
| springdoc: | |||
There was a problem hiding this comment.
An application yml file for negative config push tests.
| <cleanHistoryOnStart>false</cleanHistoryOnStart> | ||
| </rollingPolicy> | ||
| </appender> | ||
|
|
There was a problem hiding this comment.
New test output appender. This is written in markdown which makes it easy to drag/drop into confluence.
| @@ -27,6 +28,10 @@ public class SolacePublisher { | |||
| private final OutboundMessageBuilder outboundMessageBuilder; | |||
| private final DirectMessagePublisher directMessagePublisher; | |||
|
|
|||
There was a problem hiding this comment.
The date wasn't serializing in the command response, needed to add this module to the mapper.
| @@ -89,8 +88,10 @@ private static String executeTerraformCommand(Command command, Map<String, Strin | |||
| switch (commandVerb) { | |||
| case "apply" -> { | |||
| writeHclToFile(command, configPath); | |||
| terraformClient.plan(envVars).get(); | |||
| terraformClient.apply(envVars).get(); | |||
| Boolean planSuccessful = terraformClient.plan(envVars).get(); | |||
There was a problem hiding this comment.
No point in running apply if the plan failed. This short circuit saves us 255 seconds on a config push request.
There was a problem hiding this comment.
Nice!
If the apply is not successful, would we return a response back to ep-core?
There was a problem hiding this comment.
Yes, we always send a response back to ep-core. If the plan step fails, we expect to get terraform error logs that would cause the EMA response to be an error. (with the error logs in the response).
| byte[] decodedBytes; | ||
| try { | ||
| decodedBytes = Base64.getDecoder().decode(command.getBody()); | ||
| } catch (IllegalArgumentException e) { |
There was a problem hiding this comment.
Catch and rethrow since the original exception has a cryptic message.
|
|
||
| @Test | ||
| public void manyRequestsForTheSameContextAtTheSameTime() { | ||
| String context = "abc123"; |
There was a problem hiding this comment.
Looks like context is same value in all tests, can we move it to the @BeforeEach section?
|
SonarQube Quality Gate
|
What is the purpose of this change?
Test many negative scenarios.
Added changes to improve error handling
How was this change implemented?
Add a new set of "real" tests that test against actual brokers. This makes regression much easier as well.
How was this change tested?
Manual testing from intellij
Is there anything the reviewers should focus on/be aware of?
No