Re: [PR] Migrate `log4j-jul` to JUnit 5 (logging-log4j2)
AlbaHerrerias commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2541767918 > There might be a lot of differences in `log4j-core-test` between `2.x` and `main`, so please make a separate PR for `main` if you can. `log4j-iostreams` shouldn't event be in `main` and I can port `log4j-osgi-test` myself. > Thank you. I've incorporated the applicable changes from `log4j-core-test` into `main` in [this PR](https://github.com/apache/logging-log4j2/pull/3289). > Thanks for the remark, we didn't notice that, sorry. We'll double-check that PR to see if it contains independent work or is just a squash of all your PRs. It contains independent work, I don't think it include our changes -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
ppkarwasz commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2541173189 > Am I correct in understanding that three modules require porting? (`log4j-osgi-test`, `log4j-core-test`, `log4j-iostreams`). Would you prefer to handle the porting directly, or would you like us to open the PRs against `main`? There might be a lot of differences in `log4j-core-test` between `2.x` and `main`, so please make a separate PR for `main` if you can. `log4j-iostreams` shouldn't event be in `main` and I can port `log4j-osgi-test` myself. > Please note this PR #3221 was related to the migration but not done by us. If I'm not mistaken this is not in `main` either. Thanks for the remark, we didn't notice that, sorry. We'll double-check that PR to see if it contains independent work or is just a squash of all your PRs. -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
AlbaHerrerias commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2538600593 @ppkarwasz They were made by @vy 🙇 I've compiled a small list with our changes: | PR against `2.x` | Ported to `main` | | - | - | | [`log4j-jdbc-dbcp2`](https://github.com/apache/logging-log4j2/pull/3007) | [Yes, done by vy](https://github.com/apache/logging-log4j2/commit/1a8df6bbf6dc39109ad9320a0af66e5b308c5caa) | | [`log4j-jpl`](https://github.com/apache/logging-log4j2/pull/3029) | [Yes, done by vy](https://github.com/apache/logging-log4j2/commit/b6101b34b3d51f8bd4e9f4278a6287f06149e3f9) | | [`log4j-to-slf4j`](https://github.com/apache/logging-log4j2/pull/3040) | [No need](https://github.com/apache/logging-log4j2/pull/2924) | | [`log4j-slf4j-impl`](https://github.com/apache/logging-log4j2/pull/3138) | [No need](https://github.com/apache/logging-log4j2/pull/2924) | | [`log4j-slf4j2-impl`](https://github.com/apache/logging-log4j2/pull/3080) | [No need](https://github.com/apache/logging-log4j2/pull/3080#pullrequestreview-2387694227) | | [`log4j-jul`](https://github.com/apache/logging-log4j2/pull/3225)| [No need](https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2535938492) | | [`log4j-jpa`](https://github.com/apache/logging-log4j2/pull/3060) | Does not look it is in `main` | | [`log4j-jakarta-smtp`](https://github.com/apache/logging-log4j2/pull/3052) | Does not look it is in `main` | | [`log4j-taglib`](https://github.com/apache/logging-log4j2/pull/3227) | Does not look it is in `main` | | [`log4j-1.2-api`](https://github.com/apache/logging-log4j2/pull/3067) | Does not look it is in `main` | | [`log4j-api-test`](https://github.com/apache/logging-log4j2/pull/3218) (not finished, we need assistance) | Does not look it is in `main` | | [`log4j-osgi-test`](https://github.com/apache/logging-log4j2/pull/3219) | | | [`log4j-core-test`](https://github.com/apache/logging-log4j2/pull/3061) (will be finished soon) | | | [`log4j-iostreams`](https://github.com/apache/logging-log4j2/pull/3248) (not merged) | | Am I correct in understanding that three modules require porting? (`log4j-osgi-test`, `log4j-core-test`, `log4j-iostreams`). Would you prefer to handle the porting directly, or would you like us to open the PRs against `main`? Please note this PR https://github.com/apache/logging-log4j2/pull/3221 was related to the migration but not done by us. If I'm not mistaken this is not in `main` either. Thank you -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
ppkarwasz commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2535938492 > Previously, your team has been handling the porting, and I was wondering if you plan to continue doing so for this and the other modules we've refactored that are not yet in `main`. Thank you! 🙏 I have the impression that you did all the ports to `main` (of course for modules that still exist in main). The version of `log4j-jul` in `main` is strongly amputated, so no need to migrate this one. -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
AlbaHerrerias commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2535930026 Hello @ppkarwasz. I wanted to ask if this refactor needs to be ported to `main`. Previously, your team has been handling the porting, and I was wondering if you plan to continue doing so for this and the other modules we've refactored that are not yet in `main`. Thank you! 🙏 -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
ppkarwasz merged PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225 -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
github-actions[bot] commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2532290732 Job Requested goals Build Tool Version Build Outcome Build Scan® build-macos-latest clean install 3.9.8 :white_check_mark: https://ge.apache.org/s/frdbunlzuh3bi"; 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 :white_check_mark: https://ge.apache.org/s/cnqn5eg6yf622"; 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 :white_check_mark: https://ge.apache.org/s/wbj4zqatbrfky"; 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 `log4j-jul` to JUnit 5 (logging-log4j2)
AlbaHerrerias commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2532175704 Hello @ppkarwasz , thank you for your help, it unblocked me. -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
ppkarwasz commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2489033877 @AlbaHerrerias, The problem with `surefire-junit-platform` was that either Surefire or JUnit 5 (I don't remember which one) initializes JUL (and therefore Log4j) way before control is transferred to the test class. Therefore this code executes too late: https://github.com/apache/logging-log4j2/blob/031d4dab8b32e1d3802f73495a7bb36b8b2685f5/log4j-jul/src/test/java/org/apache/logging/log4j/jul/test/CoreLoggerTest.java#L59-L62 To workaround that, all the tests must be split into at least 4 Surefire runs, depending on the system properties that they need. Most test classes require the `java.util.logging.manager` property to be set (except `Log4jBridgeHandlerTest`), but some need additional properties. For example `AsyncLoggerThreadsTest` additionally needs the `log4j2.contextSelector` property to be set: ```xml async-logger-test test test **/AsyncLoggerThreadsTest.class org.apache.logging.jul.tolog4j.LogManager org.apache.logging.log4j.core.async.AsyncLoggerContextSelector ``` `CoreLoggerTest` needs an additional `log4j2.julLoggerAdapter` system property. -- 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 `log4j-jul` to JUnit 5 (logging-log4j2)
AlbaHerrerias commented on PR #3225: URL: https://github.com/apache/logging-log4j2/pull/3225#issuecomment-2488814095 Hi, we are currently blocked and would like your advice. We've found out that the `surefire-junit47` provider specified [here](https://github.com/apache/logging-log4j2/blob/031d4dab8b32e1d3802f73495a7bb36b8b2685f5/log4j-jul/pom.xml#L100) only executes JUnit 4 tests. We've attempted to change the version and the provider with no luck. Is there another version of this dependency that would set up java.util.logging as required, but will also run JUnit 5 tests? Do you have any pointers for us? Thank you -- 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
