Implemented ECS formatter for JBoss Logmanager #75
Conversation
|
💚 CLA has been signed |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
|
@spinscale you worked on something similar in https://github.com/spinscale/quarkus-logging-ecs. The main difference seems to be that your repo contains the Quarkus build steps. Also the configuration model seems to be different. Could the @tobiasstadler Looks really good! Could you also add a |
|
nice work, I will just use this and simplify the extension with the next release |
|
Could anyone start a CI run, please? |
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class EcsFormatterTest { |
There was a problem hiding this comment.
Isn't most of this already tested by JBossLogManagerTest? The exact matching on the JSON structure in this test is a bit brittle. If, for example, a new field is added all tests will fail.
There was a problem hiding this comment.
I don't know what is JBossLogManagerTest is testing. But you are most probably right that there is an overlap.
Basicly I did the same as for the JUL ECS Formatter. There is also a EcsFormatterTest and JBossLogManagerTest. Do yo think I should remove EcsFormatterTest?
There was a problem hiding this comment.
If you feel there’s something important that’s not covered in JBossLogManagerTest it might make sense to add it to the abstract test. If you want to test stuff that’s really specific to JBoss log manager just leave those tests and remove the ones that duplicate. When in doubt, lean towards lowering complexity.
There was a problem hiding this comment.
I removed a couple of tests from EcsFormatterTest which I think are already covered by JBossLogManagerTest.
JBoss Logmanager is used in e.g WildFly and Quarkus.