Skip to content
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

Configure test logging with Log4j 2 #20284

Merged
merged 2 commits into from
Sep 1, 2016
Merged

Configure test logging with Log4j 2 #20284

merged 2 commits into from
Sep 1, 2016

Conversation

jasontedor
Copy link
Member

This commit configures test logging for Log4j 2. The default logger
configuration uses the console appender but at the error level, so most
tests are missing logging. Instead, this commit provides a configuration
for tests which is picked up from the classpath by Log4j 2 when it
initializes. However, this now means that we can no longer initialize
Log4j with a bare-bones configuration when tests run as doing so will
prevent Log4j 2 from attempting to configure logging via the
classpath. Consequently, we move this needed initialization (as
commented, to avoid a message about a status logger not being configured
when we are preparing to configure Log4j from properties files in the
config directory) to only run when we are explicitly configuring Log4j
from properties files.

Relates #20235

This commit configures test logging for Log4j 2. The default logger
configuration uses the console appender but at the error level, so most
tests are missing logging. Instead, this commit provides a configuration
for tests which is picked up from the classpath by Log4j 2 when it
initializes. However, this now means that we can no longer initialize
Log4j with a bare-bones configuration when tests run as doing so will
prevent Log4j 2 from attempting to configure logging via the
classpath. Consequently, we move this needed initialization (as
commented, to avoid a message about a status logger not being configured
when we are preparing to configure Log4j from properties files in the
config directory) to only run when we are explicitly configuring Log4j
from properties files.
@jasontedor jasontedor added >test Issues or PRs that are addressing/adding tests review :Core/Infra/Logging Log management and logging utilities v5.0.0-beta1 labels Sep 1, 2016
@nik9000
Copy link
Member

nik9000 commented Sep 1, 2016

LGTM

@colings86
Copy link
Contributor

Tested in Eclipse and it works. Not sure I completely follow the change so feel free to get another reviewer if you want but LGTM what I can see

* master:
  Elasticsearch 2.4.0 uses Lucene 5.5.2
  settings: Make `action.auto_create_index` setting a dynamic cluster setting.
  Added version 2.4.0 with bwc indices
  Fix NPE when running a range query on a `scaled_float` with no upper bound. #20253
  Optimize indexing for the autogenerated ID append-only case (#20211)
  Ensure ESTestCase is initialized before we run tests
@jasontedor jasontedor merged commit 1e80adb into elastic:master Sep 1, 2016
@jasontedor jasontedor deleted the configure-test-logging branch September 1, 2016 18:00
@jasontedor
Copy link
Member Author

Thanks for looking @nik9000 and @colings86.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Logging Log management and logging utilities >test Issues or PRs that are addressing/adding tests v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants