New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WFLY-1379] Don't replace log4j appenders from the bootstrap logging #4597
Conversation
Triggering build using a merge of 7dbf4a2b99ace92a5d9b01034bac84c17a60ee03 on branch master: |
Build 7344 is now running using a merge of 7dbf4a2b99ace92a5d9b01034bac84c17a60ee03 on branch master: |
Build 7344 outcome was FAILURE using a merge of 7dbf4a2b99ace92a5d9b01034bac84c17a60ee03 on branch master: |
Failure unrelated, retest this please |
Triggering build using a merge of 7dbf4a2b99ace92a5d9b01034bac84c17a60ee03 on branch master: |
Build 7345 is now running using a merge of 7dbf4a2b99ace92a5d9b01034bac84c17a60ee03 on branch master: |
Build 7345 outcome was FAILURE using a merge of 7dbf4a2b99ace92a5d9b01034bac84c17a60ee03 on branch master: |
Triggering build using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Build 7348 is now running using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Build 7348 outcome was FAILURE using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Let's try again. These failures should have nothing to do with the change. retest this please |
Triggering build using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Build 7350 is now running using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Build 7350 outcome was FAILURE using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Trying this again, retest this please |
Triggering build using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Build 7355 is now running using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Build 7355 outcome was FAILURE using a merge of 9b08af2fc902659deefb4cda92b9504d16809699 on branch master: |
Well I'm giving up on retesting this. These failures seem to be unrelated to this PR. I'm not sure why these tests keep failing. |
I don't believe it's safe yet to execute reload ops in the testsuite. That said, a bunch of the failures you've seen are intermittent ones that pop up here and there. |
I was kind of wondering that. Is there a way to do it? A restart is fine too. Really I could remove the test since I doubt I would break that again. |
Since this test is introducing a reload operation, I think this test should be moved to the "manualmode" test module where the containers are started/stopped by individual tests. Having tests like these in "basic" test module where the container is managed and can potentially be started outside of this testcase, can cause problems for others tests. |
Perfect, thanks Jaikran. I will move the test and update he pull request. |
… if the class name and module match.
Triggering build using a merge of 602f3c5 on branch master: |
Build 7368 is now running using a merge of 602f3c5 on branch master: |
Build 7368 outcome was SUCCESS using a merge of 602f3c5 on branch master: |
Merged |
If log4j appenders were defined as custom handlers bootstrap logging configured them correctly, but adding them back via an operation resulted in a bug in the logmanager. See jboss-logging/jboss-logmanager#19 for that fix.
The issue was the class name would never be the same since we wrap the log4j appender in a JUL handler. There is no need to replace the log4j appenders if everything matches.