Skip to content

SOLR-18107: Test fix testLogLevelHandlerOutput#4146

Open
dsmiley wants to merge 2 commits intoapache:mainfrom
dsmiley:SOLR-18107-LoggingHandlerTest-fix
Open

SOLR-18107: Test fix testLogLevelHandlerOutput#4146
dsmiley wants to merge 2 commits intoapache:mainfrom
dsmiley:SOLR-18107-LoggingHandlerTest-fix

Conversation

@dsmiley
Copy link
Contributor

@dsmiley dsmiley commented Feb 19, 2026

LoggingHandlerTest.testLogLevelHandlerOutput

Use hard references for consistent logger lookup success

https://issues.apache.org/jira/browse/SOLR-18107

LoggingHandlerTest.testLogLevelHandlerOutput

Use hard references for consistent logger lookup success
@github-actions github-actions bot added the tests label Feb 19, 2026
@dsmiley
Copy link
Contributor Author

dsmiley commented Feb 19, 2026

I'll commit in 48 hours if no review

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes flakiness in LoggingHandlerTest.testLogLevelHandlerOutput by ensuring logger instances are strongly referenced so Log4j2’s weakly-referenced logger cache doesn’t allow loggers to be GC’d and recreated mid-test.

Changes:

  • Add strong Logger references for the relevant logger names used by the test.
  • Update “effective level” assertions to use the held logger instances instead of repeated lookups.
  • Minor comment text adjustment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

assertEquals(Level.DEBUG, LogManager.getLogger(A_LOGGER_NAME).getLevel());
assertEquals(Level.TRACE, LogManager.getLogger(B_LOGGER_NAME).getLevel());
assertEquals(Level.TRACE, LogManager.getLogger(BX_LOGGER_NAME).getLevel());
// ...and with its effective values
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor grammar issue in the comment: "...and with its effective values" reads awkwardly and seems to be missing a verb. Consider rephrasing to "...and what its effective values are" (or similar) for clarity.

Suggested change
// ...and with its effective values
// ...and what its effective values are

Copilot uses AI. Check for mistakes.
Comment on lines +147 to 148
// check directly with log4j what its (updated) config has...
assertEquals(Level.DEBUG, config.getLoggerConfig(PARENT_LOGGER_NAME).getExplicitLevel());
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block corrects "it's" → "its" here, but a similar comment later in the UNSET section still uses "what it's (updated) config has...". Please update that occurrence too to keep wording consistent.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments