You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/06/05 13:22:44 UTC

[GitHub] [logging-log4j2] ppkarwasz opened a new pull request, #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

ppkarwasz opened a new pull request, #916:
URL: https://github.com/apache/logging-log4j2/pull/916

   Unit tests in `release-2.x` are executed sequentially in per-test JVMs. While parallel execution in a single JVM requires a lot of work, this PR makes a step in this direction: the tests based on JUnit5 in `log4j-core` (about 50% of the tests) are executed in per-test JVMs, but in parallel.
   
   This requires only a private/temporary directory for each test to store its logs (@PrivateDir extension), so that tests using the same file names don't clash.
   
   As a preliminary step to phase 2 (running multiple JVMs in parallel and reusing them for multiple tests), this PR also introduces a thread-bound property source/lookup that can be retrieved with the @TestProperties annotation (or @SetTestProperty).


-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] jvz commented on a diff in pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
jvz commented on code in PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#discussion_r889741333


##########
log4j-core/pom.xml:
##########
@@ -444,6 +444,30 @@
             <org.apache.activemq.SERIALIZABLE_PACKAGES>*</org.apache.activemq.SERIALIZABLE_PACKAGES>
           </systemPropertyVariables>
         </configuration>
+        <executions>
+            <execution>
+                <id>default-test</id>
+                <phase>test</phase>
+                <goals>
+                  <goal>test</goal>
+                </goals>
+                <configuration>
+                   <includeJUnit5Engines>junit-jupiter</includeJUnit5Engines>
+                   <forkCount>1C</forkCount>
+                   <runOrder>random</runOrder>

Review Comment:
   Ah, that's another goal of mine whenever I set the ordering to random. I'm trying to see if there are any test interferences that would otherwise have gone unnoticed locally.



-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on a diff in pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on code in PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#discussion_r889740893


##########
log4j-core/pom.xml:
##########
@@ -444,6 +444,30 @@
             <org.apache.activemq.SERIALIZABLE_PACKAGES>*</org.apache.activemq.SERIALIZABLE_PACKAGES>
           </systemPropertyVariables>
         </configuration>
+        <executions>
+            <execution>
+                <id>default-test</id>
+                <phase>test</phase>
+                <goals>
+                  <goal>test</goal>
+                </goals>
+                <configuration>
+                   <includeJUnit5Engines>junit-jupiter</includeJUnit5Engines>
+                   <forkCount>1C</forkCount>
+                   <runOrder>random</runOrder>

Review Comment:
   I set the run order to random, to empirically check if two test classes do not use the same log files. Hopefully if they do, once in a while we will have some meaningful test failures.



-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] jvz commented on pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
jvz commented on PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#issuecomment-1146857349

   This looks like a great approach! I like that this can enable more parallelism than the equivalent system property approach since those are global. I also like that this is such a small change compared to modifying every existing test. While I've managed to get the API and plugins modules to work in parallel+concurrency (single process multi threaded option rather than forking), what I've been doing there won't scale to core tests without additional changes such as in this PR. As such, this PR most likely applies to master with minimal changes, too!


-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] jvz commented on a diff in pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
jvz commented on code in PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#discussion_r889732979


##########
log4j-core/pom.xml:
##########
@@ -444,6 +444,30 @@
             <org.apache.activemq.SERIALIZABLE_PACKAGES>*</org.apache.activemq.SERIALIZABLE_PACKAGES>
           </systemPropertyVariables>
         </configuration>
+        <executions>
+            <execution>
+                <id>default-test</id>
+                <phase>test</phase>
+                <goals>
+                  <goal>test</goal>
+                </goals>
+                <configuration>
+                   <includeJUnit5Engines>junit-jupiter</includeJUnit5Engines>
+                   <forkCount>1C</forkCount>
+                   <runOrder>random</runOrder>

Review Comment:
   I've taken to using `<runOrder>balanced</runOrder>` when parallelizing tests. I did set them to random recently in master because I was sick of seeing the same slow tests at the beginning and wanted to change things up. :)



-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#issuecomment-1147395799

   @rgoers,
   Can I merge this before the 2.18.0 release or should I wait to be safe?


-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz closed pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
ppkarwasz closed pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests
URL: https://github.com/apache/logging-log4j2/pull/916


-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#issuecomment-1167843430

   I'll fix the problems with parallel testing in the `master` branch and then backport it to `release-2.x`.


-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] PRINSIS commented on pull request #916: [LOG4J2-2921] Parallel execution for JUnit5 tests

Posted by GitBox <gi...@apache.org>.
PRINSIS commented on PR #916:
URL: https://github.com/apache/logging-log4j2/pull/916#issuecomment-1152702789

   Duplicate of #Open
   [[LOG4J2-2921] Parallel execution for JUnit5 tests](https://github.com/apache/logging-log4j2/pull/916#)
   #916
   [ppkarwasz](https://github.com/ppkarwasz) wants to merge 1 commit into [apache:release-2.x](https://github.com/apache/logging-log4j2) from [ppkarwasz:partial-parallel-tests](https://github.com/ppkarwasz/logging-log4j2/tree/partial-parallel-tests)


-- 
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-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org