Skip to content

Run RollingAppenderCountTest on 2.x again - #4413

Merged
ramanathan1504 merged 7 commits into
apache:2.xfrom
sharanggupta:run-rolling-appender-count-test
Oct 22, 2026
Merged

ramanathan1504 merged 7 commits into
apache:2.xfrom
sharanggupta:run-rolling-appender-count-test

Conversation

@sharanggupta

@sharanggupta sharanggupta commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

RollingAppenderCountTest has been @Ignored on 2.x since it was found to be flaky in LOG4J2-3522. The fix for that issue, 4387a2f, only went to main. It replaced the Thread.sleep(250) between the log events with a RolloverListener and a CountDownLatch, and wrote more data per iteration.

This ports that approach to the 2.x test, keeping its JUnit 4 rule. The test registers itself as a RolloverListener on the RollingFile manager, logs 60 iterations without sleeping, and waits for at least 16 rollovers. The check for the 17 files stays in @AfterClass, which runs after the rule has stopped the logger context. The test no longer sleeps for 15 seconds.

./mvnw -pl :log4j-core-test -am verify -Dtest=RollingAppenderCountTest passes locally on JDK 17, in every Surefire execution of log4j-core-test.

Signed-off-by: Sharang Gupta <sharang@sharanggupta.dev>
The test was ignored for LOG4J2-3522 because it was flaky. Port the
approach of the fix on main (4387a2f): wait for the rollovers with a
RolloverListener and a CountDownLatch instead of sleeping between log
events, and check the number of files in the test method.
Keep the check of the 17 files in @afterclass, which runs after the rule
has stopped the logger context, so it does not depend on the rollovers
having completed on the logging thread.
@ramanathan1504 ramanathan1504 added tests Pull requests or issues related to tests port-done No further port needed: all required ports are merged, or none was needed labels Oct 20, 2026
@ramanathan1504

Copy link
Copy Markdown
Contributor

Fixed randomAscii(512) payloads take the random size out of the rollover count, and the test no longer sleeps for 15 seconds.
The 17-file check in afterClass still runs after the context stops, and the latch fails the test if the listener in setUp is not registered. Looks good to me.

@ramanathan1504
ramanathan1504 enabled auto-merge (squash) October 21, 2026 12:51
@ramanathan1504
ramanathan1504 merged commit 645e401 into apache:2.x Oct 22, 2026
13 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Merged in Log4j pull request tracker Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

port-done No further port needed: all required ports are merged, or none was needed tests Pull requests or issues related to tests

Projects

Development

Successfully merging this pull request may close these issues.

2 participants