Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
jvz merged PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
jvz commented on PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#issuecomment-2515589997 I'll review this, especially since converting tests from v4 to v5 was one of my "hobbies" in this project not that long ago. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
strangelookingnerd commented on PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#issuecomment-2493968110 > @strangelookingnerd, > > Do you have a deadline for the "Migrate to JUnit 5" task? We are swamped with work these days, so the review might take more time than anticipated. Not at all. I fully understand that the changeset is quite large and will take its time to review. I still hope that it will be merged eventually and I will try my best to keept it up-to-date and free of merge confilict 👍🏼 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
ppkarwasz commented on PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#issuecomment-2493859066 @strangelookingnerd, Do you have a deadline for the "Migrate to JUnit 5" task? We are swamped with work these days, so the review might take more time than anticipated. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
strangelookingnerd commented on code in PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#discussion_r1850069574 ## log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java: ## Review Comment: Added a check in [94884a7](https://github.com/apache/logging-log4j2/pull/3221/commits/94884a70323a634f42ccc92502bfb2396bd21656) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
ppkarwasz commented on code in PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#discussion_r1850002853 ## log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java: ## Review Comment: The actual test results seems OK! Probably the test was broken and you fixed it! :100: You just need to adapt the expected value of the test to `org.apache.logging.log4j.core.async.TimetoutBlockingWaitStrategy` if `Disruptorutil.DISRUPTOR_MAJOR_VERSION` returns 3 and `com.lmax.disruptor.TimetoutBlockingWaitStrategy` if the constant is 4. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
ppkarwasz commented on code in PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#discussion_r1849991388 ## log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java: ## Review Comment: This is probably a problem with dependencies: `com.lmax:disruptor` version 4.x has a `com.lmax.disruptor.TimeoutBlockingWaitStrategy` class, but `com.lmax:disruptor` version 3.x doesn't. We have two Surefire runs to test asynchronous logger support against both Disruptor 3.x (which supports our target Java 8) and Disruptor 4.x (which requires Java 11). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
strangelookingnerd commented on code in PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#discussion_r1849965952 ## log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java: ## Review Comment: ``` [ERROR] org.apache.logging.log4j.core.async.AsyncWaitStrategyFactoryConfigTest.testIncorrectWaitStrategyFallsBackToDefault(ListAppender, LoggerContext) -- Time elapsed: 5.378 s <<< FAILURE! org.opentest4j.AssertionFailedError: expected: ; but was: ``` No idea how the changes may be causing this. Could this be some weird class loading issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
strangelookingnerd commented on PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#issuecomment-2488048270 The failing test seems odd as the changes made should not cause it to fail. Any pointers what may be causing it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
github-actions[bot] commented on PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#issuecomment-2487847913 Job Requested goals Build Tool Version Build Outcome Build Scan® build-macos-latest clean install 3.9.8 :x: https://ge.apache.org/s/mo35ntzi34ecq"; rel="nofollow">https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle"; alt="Build Scan PUBLISHED" /> build-ubuntu-latest clean install 3.9.8 :x: https://ge.apache.org/s/2xby3r6tjtyn6"; rel="nofollow">https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle"; alt="Build Scan PUBLISHED" /> build-windows-latest clean install 3.9.8 :x: https://ge.apache.org/s/4ryvy7r4afroy"; rel="nofollow">https://img.shields.io/badge/Build%20Scan%C2%AE-PUBLISHED-06A0CE?logo=Gradle"; alt="Build Scan PUBLISHED" /> ## Generated by gradle/develocity-actions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Migrate tests to JUnit5 (logging-log4j2)
ppkarwasz commented on PR #3221: URL: https://github.com/apache/logging-log4j2/pull/3221#issuecomment-2487818020 @strangelookingnerd, It will take me some time to scroll through all the changes in this PR, but I'll try to review this and your other PRs by the end of next week. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
