Closed preichel-cg closed 6 years ago
Can one of the admins verify this patch?
@hohwille Hi Jörg, I have addressed all of your mentioned points and made 2 commits, which now show up in this PR.
Hi Jörg, the new main changes are:
//given
- //when
- //then
open points:
assertThat
statements are much shorter) and reduce the chance of making inconsistent changes. And especially with the //given
- //when
- //then
logic they make clearer, that their correct initialization is a particular feature which shall be tested. But I will change it if you insist ;-).Cheers, Patrick
@preichel-cg 👍 thanks for your changes. Now everything looks great. I do not have any further concerns except maybe:
// Check the filter chain decision for this event
// (does not work) assertThat(this.mockAppender.getFilterChainDecision(loggingEvent)).isEqualTo(FilterReply.DENY);
However, I am fine with merging this and we can still address such fine details separately. I will wait for the next architects meeting to announce the merge and will then hopyfully merge.
Still no concerns. Merging now.
569