[BAEL-8422] Exception handling through spring AOP and AspectJ #17512
Conversation
spring-aop-2/src/test/java/com/baeldung/exceptionhandling/ExceptionHandlingAspectTest.java
Outdated
Show resolved
Hide resolved
spring-aop-2/src/test/java/com/baeldung/exceptionhandling/ExceptionHandlingAspectTest.java
Outdated
Show resolved
Hide resolved
spring-aop-2/src/main/java/com/baeldung/exceptionhandling/ExceptionHandlingAspect.java
Outdated
Show resolved
Hide resolved
|
|
||
| // Then | ||
| List<ILoggingEvent> logsList = listAppender.list; | ||
| assertThat(logsList).isNotEmpty(); |
There was a problem hiding this comment.
This is not necessary, the following assertion already implies the list is not empty (same in the other test)
| @Test | ||
| public void givenInvalidPaymentDetails_whenValidationFails_thenAspectLogsException() { | ||
| // When | ||
| assertThatThrownBy(() -> paymentService.validatePaymentDetails("")).isInstanceOf(IllegalArgumentException.class) |
There was a problem hiding this comment.
Please use the Baeldung formatter, we should be putting chained method calls line by line https://github.com/Baeldung/dev-settings/tree/main/javaCodeStyle/intellij
There was a problem hiding this comment.
I have already configured my IntelliJ with that code style, but still on formatting the code, it doesn't split all the chained calls to new lines.
| // Then | ||
| List<ILoggingEvent> logsList = listAppender.list; | ||
| assertThat(logsList).isNotEmpty(); | ||
| assertThat(logsList).extracting(Object::toString) |
There was a problem hiding this comment.
We are still not showing that the Exception is logged:
logger.error("Exception occurred: {}", ex.getMessage(), ex);
The last parameter ex is untested at the moment
There was a problem hiding this comment.
The last parameter "ex" is not actually needed. I have removed it in the latest commit.
assertThat(logsList).extracting(Object::toString) .containsExactly("[ERROR] Exception occurred: Payment details cannot be null or empty.");
This assertion captures the logs and checks whether the exception that actually occurred matches the expected log.
There was a problem hiding this comment.
Sorry but I have to disagree here. It's essential when logging exceptions to preserve the stack trace - can we find a way to test this please?
There was a problem hiding this comment.
I agree that preserving the stacktrace is important. But I can't think of a way to create an assertion for that. One way can be to actually assert each line of the stacktrace printed but that will be too clumsy.
On a side note, isn't it implicit that if the code has reached the error logger and thrown an error message that we have already asserted, then the stack trace is already tested? Let me know if you think otherwise and has a solution to test.
There was a problem hiding this comment.
Ok I'll show you how to do this later when I get some time
There was a problem hiding this comment.
And no it's not implicit. The test would still be green if I replaced the ex parameter with a dummy new Exception()
There was a problem hiding this comment.
You can do something like:
assertThat(logsList)
.extracting(event -> event.getThrowableProxy().getMessage())
.containsExactly("Payment processing failed due to invalid details.");
or alternatively, wire a mock/spy Logger into the ExceptionHandlingAspect and use Mockito to verify
No description provided.