search

elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.2k stars 24.84k forks source link

LoggersTests incompatible with tests.iters #82107

Closed nik9000 closed 2 years ago

nik9000 commented 2 years ago

Repro line:

./gradlew ':server:test' --tests "org.elasticsearch.common.logging.LoggersTests" -Dtests.iters=2

Reproduces locally?: yes

Applicable branches: master, more?

elasticmachine commented 2 years ago

Pinging @elastic/es-core-infra (Team:Core/Infra)

pgomulka commented 2 years ago 
 REPRODUCE WITH: ./gradlew ':server:test' --tests "org.elasticsearch.common.logging.LoggersTests.testParameterizedMessageLambda {seed=[63C538FEBB285B59:C4E38DE6C5C14243]}" -Dtests.seed=63C538FEBB285B59 -Dtests.locale=ar-LY -Dtests.timezone=America/Catamarca -Druntime.java=17
  2> java.lang.NullPointerException: Cannot invoke "org.apache.logging.log4j.core.LogEvent.getLevel()" because "appender.lastEvent" is null
        at __randomizedtesting.SeedInfo.seed([63C538FEBB285B59:C4E38DE6C5C14243]:0)
        at org.elasticsearch.common.logging.LoggersTests.testParameterizedMessageLambda(LoggersTests.java:36)

This only happens when the test is executed twice (or more)

I think this is because there is only one Logger instance (cached by log4j). On the first run it adds a new instance of the appender and asserts about this. On the second instance it adds another instance of an appender under the same name. Which won't be added to a logger - an appender with that name was already set (AbstractConfiguration#addAppender). the test in the second run will try to assert about the second appender instance (which is not set on a logger).

we can easily fix this by a different unique name of the appender per run. But is this really a problem? What make you to run this test twice @nik9000 ? :)