Skip to content

Stabilize RollingAppenderDirectCronTest and XmlCompleteFileAppenderTest #3946

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 22, 2025
Merged

Stabilize RollingAppenderDirectCronTest and XmlCompleteFileAppenderTest #3946

merged 5 commits into from
Oct 22, 2025
+13 −14

Conversation

rjuare8
Copy link
Contributor

@rjuare8 rjuare8 commented Oct 3, 2025 •
edited by vy
Loading

Stabilize RollingAppenderDirectCronTest.

Fixes #3046.

@vy
Copy link
Member

vy commented Oct 10, 2025

@rjuare8, thanks for the contribution. Would you mind performing the following tasks, please?

  1. Sign your commits
  2. Run ./mvnw spotless:apply -pl :log4j-core-test
  3. Ensure ./mvnw verify -pl :log4j-core-test passes
  4. Read the Log4j Development Guide

@vy
Copy link
Member

vy commented Oct 17, 2025

@rjuare8, all your commits must be signed, and they are not. See the Commits tab and Unverified labels there.

@vy vy changed the title fixed undeterministic RollingAppenderDirectCronTest Stabilize RollingAppenderDirectCronTest Oct 17, 2025
@vy vy added waiting-for-user More information is needed from the user tests Pull requests or issues related to tests labels Oct 17, 2025
@vy vy self-assigned this Oct 17, 2025
@rjuare8
Copy link
Contributor Author

rjuare8 commented Oct 21, 2025

Signed all my commits! Let me know if there is additional actions I need to take.

Related to my fix for testFlushAtEndOfBatch(). This change hardens XmlCompleteFileAppenderTest#testFlushAtEndOfBatch against formatting/timing variance where a blank line after the <Events> root element is not guaranteed. The prior test unconditionally consumed one line after <Events>, assuming it was always blank; when the blank line was absent, the <Event …> line was skipped and later assertions occasionally saw line3 == null.

@vy
Copy link
Member

vy commented Oct 22, 2025

Signed all my commits! Let me know if there is additional actions I need to take.

Yes, there are were: #3946 (comment) Since it was 2 weeks ago, I have decided to address it myself in 6fbd257. While I really appreciate your contribution, I'd very glad if you can read and address all review remarks next time.

@vy vy enabled auto-merge (squash) October 22, 2025 09:39
@github-actions
Copy link

github-actions bot commented Oct 22, 2025 •
edited
Loading

Job Requested goals Build Tool Version Build Outcome Build Scan®
build-macos-latest clean install 3.9.8 ✅ Build Scan PUBLISHED
build-ubuntu-latest clean install 3.9.8 ✅ Build Scan PUBLISHED
build-windows-latest clean install 3.9.8 ✅ Build Scan PUBLISHED
Generated by gradle/develocity-actions

@vy vy changed the title Stabilize RollingAppenderDirectCronTest Stabilize RollingAppenderDirectCronTest and XmlCompleteFileAppenderTest Oct 22, 2025
@vy vy disabled auto-merge October 22, 2025 10:28
@vy vy enabled auto-merge (squash) October 22, 2025 10:29
@vy vy merged commit 60c62ee into apache:2.x Oct 22, 2025
7 checks passed
@github-project-automation github-project-automation bot moved this from To triage to Done in Log4j bug tracker Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vy vy vy approved these changes

Assignees

@vy vy

Labels

tests Pull requests or issues related to tests waiting-for-user More information is needed from the user

Projects

Log4j bug tracker
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RollingAppenderDirectCronTest.testAppender(LoggerContext, RollingFileAppender) is unreliable

2 participants

@rjuare8 @vy