You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/04/15 19:09:37 UTC

[GitHub] [maven-surefire] SiKing opened a new pull request #347: document test-suite

SiKing opened a new pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347


   @Tibor17 My actual solution was more involved; see this [SO answer](https://stackoverflow.com/a/67113491/3124333). I did not think you wanted that much info in your documentation, so I kept this brief.


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615165165



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       > Well, my explanation below this is "This would then run **up to** 4 threads" (added emphasis). So I would hope that people can deduce that using 3 or 2, would "then run up to 3 or 2, respectively, threads."
   
   If I understand you correctly, you want to achive the following. You want to run package `featureA` in thread T1, and `featureB` in thread T2. Am I right?
   




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614345431



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>
+  </configuration>
+</plugin>
++---+
+
+  This would then run up to 4 threads, and each thread would be fed a TestSuite.class
+  one at a time. The test calsses within that Suite would be executed sequentially

Review comment:
       Here is typo `calsses`. You mean `classes`.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824463491


   We cannot merge. Github is not able to squash and rebase, that's the problem. I will do it apart of this PR.
   Regarding the threadCount, you do not know the history. We cannot change as you want to, otherwise we broke the users.


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824203604


   > Can you pls squash these commits? Thx
   
   I cannot find how to do this. I thought the squash is usually done during merge?
   
   Also: How would you prefer to handle my "poc" case? Should I just open a bug to be investigated? I am kinda hoping this is not a bug, but a user error. But I just do not know what I did wrong.


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614966189



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>

Review comment:
       sounds good




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614344603



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:

Review comment:
       Please rewrite `like so` to `as follows`:




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615042662



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       > Could you please explain what you mean. Especially "use another number but the effect would be different". I do not understand.
   > In my actual use-case I am running into problems where a bunch of tests pass and a bunch fail, and I do not know why.
   
   I wanted to say that Surefire nested suites in third level are not running in parallel.
   Your example is okay, it has package `featureA` and it means that `featureB` is concurrent. But I want to say that our users should not expect that a nested package with new nested suites in `featureA.pkg1` and `featureA.pkg2` would be concurrent. The Surefire would run `pkg1` and `pkg2` in a sequence.  I guess you know what I mean. If somebody expects making them parallel then he would use 4 threads or 8 threads but I think it would not be... So hopefully your case works but the users should not expect any generic approach with infinite depth of parallel suites.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615042662



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       > Could you please explain what you mean. Especially "use another number but the effect would be different". I do not understand.
   > In my actual use-case I am running into problems where a bunch of tests pass and a bunch fail, and I do not know why.
   
   I wanted to say that Surefire nested suites in third level are not running in parallel.
   Your example is okay, it has package `featureA` and it means that `featureB` is concurrent. But I want to say that our users should *not* expect that a nested package with new nested suites in `featureA.pkg1` and `featureA.pkg2` would be concurrent. The Surefire would run `pkg1` and `pkg2` in a sequence.  I guess you know what I mean. If somebody expects making them parallel then he would use 4 threads or 8 threads but I think it would not be... So hopefully your case works but the users should not expect any generic approach with infinite depth of parallel suites.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614980349



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       I wrote the `ParallelComputer` for Surefire. But it was in 2013. Now I do not exactly remember whether the suites are searched recursively or they are flat. If the search is recursive then both packages and their content would run in parallel completely.
   
   Therefore I was asking for the number 4 because this sounds to me like when all runs in parallel.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614980349



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       I wrote the `ParallelComputer` for Surefire. But it was in 2013. Now I do not exactly remember whether the suites are searched recursively of they are flat. If the search is recursive then both packages and their content would run in parallel completely.
   
   Therefore I was asking for the number 4 because this sounds to me like when all runs in parallel.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615042662



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       > Could you please explain what you mean. Especially "use another number but the effect would be different". I do not understand.
   > In my actual use-case I am running into problems where a bunch of tests pass and a bunch fail, and I do not know why.
   
   I wanted to say that Surefire nested suites in third level are not running in parallel.
   Your example is okay, it has package `featureA` and it means that `featureB` is concurrent. But I want to say that our users should not expect that a nested package `featureA.pkg1` and `featureA.pkg2` would be concurrent. The Surefire would run `pkg1` and `pkg2` in a sequence.  I guess you know what I mean. If somebody expects making them parallel then he would use 4 threads or 8 threads but I think it would not be... So hopefully your case works but the users should not expect any generic approach with infinite depth of parallel suites.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824680075


   @SiKing 
   Thank You for contributing!


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824420161


   > We forgot to add this configuration parameter in the example. Please add `<perCoreThreadCount>false</perCoreThreadCount>`. Ths
   
   I added the information.
   
   As I mentioned above: I do not know how to squash everything. I thought the squash is usually done during merge?


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-823581345


   @SiKing 
   This is the problem `System.getProperty("thread.count", "1")`. You will never have this system property available in the JVM. It is processed only in the plugin which is the Maven process.
   
   Why you use 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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614998848



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */

Review comment:
       done




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614343816



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>

Review comment:
       Here it should be `<include>features/*/TestSuite.java</include>`.
   We have only one file name `TestSuite.java` in two directories, right?




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615376383



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       So it means that there would be no difference between two and four threads, because there are two packages and there would be still only two parallel executions and not four. Why you write 4 in the documentation? If the users has a reason, he would change it, he can setup parallel classes to but it is up to him - not us.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614345679



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       What happens with `3` or `2`. Did you make an experiment?




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614469046



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */

Review comment:
       I have seen documentation where comments explain why a class, constructor, etc. has an empty body, but is still needed. For example: https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615270744



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       In a word: yes.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615042662



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       > Could you please explain what you mean. Especially "use another number but the effect would be different". I do not understand.
   > In my actual use-case I am running into problems where a bunch of tests pass and a bunch fail, and I do not know why.
   
   I wanted to say that Surefire nested suites in third level are not running in parallel.
   Your example is okay, it has package `featureA` and it means that `featureB` is concurrent. But I want to say that our users should *not* expect that a nested package with new nested suites in `featureA.pkg1` and `featureA.pkg2` would be concurrent. The Surefire would run `pkg1` and `pkg2` in a sequence with suites.  I guess you know what I mean. If somebody expects making them parallel then he would use 4 threads or 8 threads but I think it would not be... So hopefully your case works but the users should not expect any generic approach with infinite depth of parallel suites.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-822753117


   Can you pls squash these commits? Thx


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614980349



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       I wrote the `ParallelComputer` for Surefire. But it was in 2013. Now I do not exactly remember whether the suites are searched recursively of they are flat. If the search is recursive then both packages and their content would run in parallel completely.
   
   Therefore I was asking fot the number 4 because this sounds to me like all is parallel.

##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       I wrote the `ParallelComputer` for Surefire. But it was in 2013. Now I do not exactly remember whether the suites are searched recursively of they are flat. If the search is recursive then both packages and their content would run in parallel completely.
   
   Therefore I was asking for the number 4 because this sounds to me like all is parallel.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614961705



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>

Review comment:
       If you use `TestSuite` the users would understand more what you mean. And the same way you declare the rule of your feature that this is the root of the tree where all the children - suites continue right after.




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614465814



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:

Review comment:
       done




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824484462


   @SiKing 
   Thx, the PR was pushed https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=commit;h=27ef162711425807a3647dc33e8c5267566d46dc


-- 
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.

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



[GitHub] [maven-surefire] SiKing edited a comment on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing edited a comment on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-823610997


   > This is the problem `System.getProperty("thread.count", "1")`. You will never have this system property available in the JVM. It is processed only in the plugin which is the Maven process.
   > 
   > Why you use it?
   
   But in my pom I have:
   ```
   <systemPropertyVariables>
     <thread.count>${thread.count}</thread.count>
   </systemPropertyVariables>
   ```
   
   I just wanted to prove the number of tests that are running at any one time is no more than the number of threads.


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-823419522


   I think my understanding of parallel=suites is wrong.
   
   I created a trivial project, attached. If you run it with `mvn test`, it fails. I do not understand why.
   [poc.zip](https://github.com/apache/maven-surefire/files/6345137/poc.zip)
   


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614465561



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>

Review comment:
       Could go either way.
   
   - First, I am using [fully-qualified name](https://maven.apache.org/surefire/maven-failsafe-plugin/examples/inclusion-exclusion.html#Fully_qualified_class_name); although that feature has [issues](https://issues.apache.org/jira/browse/SUREFIRE-1789).
   - Second, I do not see a problem with the wildcard `*Suite`, but `TestSuite` would work as well in this case.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614346664



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,

Review comment:
       I know that you use BDD or Selenium or Cucumber but our documentation has standard conventions. And so the test classes have postfix `Test`, e.g. `SomeTest.java`.




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615001136



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       Could you please explain what you mean. Especially "use another number but the effect would be different". I do not understand.
   In my actual use-case I am running into problems where a bunch of tests pass and a bunch fail, and I do not know why.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615029372



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       > Also, am I correct in that, in this specific case, `threadCount=X` would have the exact same effect as `threadCountSuites=X`?
   > Should I add that to the docs here?
   
   You'r right, it has identical effect **in this case**.




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614465984



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>
+  </configuration>
+</plugin>
++---+
+
+  This would then run up to 4 threads, and each thread would be fed a TestSuite.class
+  one at a time. The test calsses within that Suite would be executed sequentially

Review comment:
       done




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824416496


   > You have to add `<perCoreThreadCount>false</perCoreThreadCount>`.
   > The reason is that the thread counts are always multiplied by the number of CPU.
   
   So this is similar to using 'C' for fork count, but completely different.
   For threads this multiplier is on by default; for forks it is off by default.
   For threads you specify it with another (boolean) parameter; for forks you specify it by adding the character "C" after the count.
   Why the inconsistencies?
   :(


-- 
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.

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



[GitHub] [maven-surefire] SiKing edited a comment on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing edited a comment on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824420161


   > We forgot to add this configuration parameter in the example. Please add `<perCoreThreadCount>false</perCoreThreadCount>`. Ths
   
   I added the information.
   
   As I mentioned above: I do not know how to squash everything. I thought the squash is usually done during merge?
   https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squash-and-merge-your-pull-request-commits


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615376383



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       So it means that there would be no difference between two and four threads, because there are two packages and there would still only two parallel executions and not four. Why you write 4 in the documentation? If the users has a reason, he would change it, he can setup parallel classes to but it is up to him - not us.




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824162001


   I just tried it with version=3.0.0-M5, and got the same result.
   
   I also tried it with parallel=suites, like this:
   ```
   <configuration>
   	<parallel>classes</parallel>
   	<threadCount>${thread.count}</threadCount>
   	<systemPropertyVariables>
   		<thread.count>${thread.count}</thread.count>
   	</systemPropertyVariables>
   </configuration>
   ```
   so, with no includes to pick up only the `OneTest.java`, and still got the same results.


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614467357



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,

Review comment:
       done




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614466930



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       Well, my explanation below this is "This would then run **up to** 4 threads" (added emphasis). So I would hope that people can deduce that using 3 or 2, would "then run up to 3 or 2, respectively, threads."




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614967949



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */

Review comment:
       The developers are well skilled, they already know that this class is only a meta code. Not necessary to repeat well known facts from another tutorials.




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615971541



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       done




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r616030120



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       I am trying to figure out why my tests keep failing.
   Right now, just to figure out what is going on, I have 4 *Suite.java, and I am using threadCountSuites=2. Please note that my tests are using Selenium. When I run my tests, it looks like 4 browsers (tests) are launched.
   If I change the number of *Suite.java files, then I get the same 4 browsers.
   However, if I change the number of threads, for example threadCountSuites=4, I get twice as many browsers launched.
   What am I doing wrong?




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r615002875



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       Also, am I correct in that, in this specific case, `threadCount=X` would have the exact same effect as `threadCountSuites=X`?
   Should I add that to the docs here?




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-824233634


   @SiKing 
   You have to add `<perCoreThreadCount>false</perCoreThreadCount>`.
   The reason is that the thread counts are always multiplied by the number of CPU.
   We forgot to add this configuration parameter in the example. Please add `<perCoreThreadCount>false</perCoreThreadCount>`. Ths


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 closed pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 closed pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347


   


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-823855882


   @SiKing 
   Probably the version `2.22.2` has a bug which looks strange to me. Check it out with the latest version `3.0.0-M5`.


-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614466930



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       Well, my explanation below this is "This would then run **up to** 4 threads" (added emphasis). So I would hope that people can deduce that using 3 or 2, would "then run up to 3 or 2, respectively, threads.




-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614995200



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */
+}
++---+
+
+  and similarly for <<<package features.areaB>>>. You would then configure
+  ${project.artifactId} like so:
+
++---+
+<plugin>
+  <groupId>${project.groupId}</groupId>
+  <artifactId>${project.artifactId}</artifactId>
+  <version>${project.version}</version>
+  <configuration>
+    <includes>
+      <include>features.*.*Suite</include>
+    </includes>
+    <parallel>suites</parallel>
+    <threadCount>4</threadCount>

Review comment:
       @rehevkor5 
   After seen the code in `ParallelComputerBuilder` the search is not recursive and searches the runners only in two levels. Therefore you should use `2` and not `4`. Then the users would understand it. They can use another number but the effect would be different. You can explain this in the document because then the users would understand what they may expect.




-- 
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.

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



[GitHub] [maven-surefire] SiKing commented on pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
SiKing commented on pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#issuecomment-823610997


   > @SiKing
   > This is the problem `System.getProperty("thread.count", "1")`. You will never have this system property available in the JVM. It is processed only in the plugin which is the Maven process.
   > 
   > Why you use it?
   
   But in my pom I have:
   ```
   <systemPropertyVariables>
     <thread.count>${thread.count}</thread.count>
   </systemPropertyVariables>
   ```
   
   I just wanted to prove the number of tests that are running at any one time is no more than the number of threads.


-- 
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.

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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #347: document test-suite

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #347:
URL: https://github.com/apache/maven-surefire/pull/347#discussion_r614351962



##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */

Review comment:
       This line is not necessary for general purpose.

##########
File path: maven-surefire-plugin/src/site/apt/examples/fork-options-and-parallel-execution.apt.vm
##########
@@ -181,6 +181,67 @@ Fork Options and Parallel Test Execution
   See the keywords: <volatile>, <synchronized>, <<immutable>> and <final> in
   {{{https://jcp.org/en/jsr/detail?id=133}Java Memory Model - JSR-133}}.
 
+* Parallel Test-Suite Execution
+
+  ${project.artifactId}'s notion of "suite" is related to
+  {{{https://github.com/junit-team/junit4/wiki/Aggregating-tests-in-suites}junit4 Suite}}. 
+
+  For example, say your tests are laid out like this:
+
++---+
+src/test/java
++-features.areaA
+| +-SomeStory.java
+| +-AnotherStory.java
+| ...
++-features.areaB
+| +-DifferentStory.java
+| +-OtherStory.java
+| ...
+...
++---+
+
+  You would add a TestSuite.java for each package, that would look something like:
+
++---+
+package features.areaA;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    SomeStory.class,
+    AnotherStory.class,
+    ...
+})
+public class TestSuite {
+    /* Suite holder */

Review comment:
       This line is not necessary for a general purpose.




-- 
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.

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