Fixes #57 using un-modifyable lists & judicious synchronization (#63)

chrisalbright · web-flow · commit c18a566d75ea · 2020-06-12T23:45:21.000-04:00
Co-authored-by: Chris Albright &lt;calbright@cj.com&gt;
diff --git a/pom.xml b/pom.xml
@@ -11,7 +11,7 @@
   <groupId>org.scalatest</groupId>
   <artifactId>scalatest-maven-plugin</artifactId>
   <packaging>maven-plugin</packaging>
-  <version>2.0.0</version>
+  <version>2.0.1-SNAPSHOT</version>
   <name>ScalaTest Maven Plugin</name>
   <description>Integrates ScalaTest into Maven</description>
 
diff --git a/src/main/java/org/scalatest/tools/maven/AbstractScalaTestMojo.java b/src/main/java/org/scalatest/tools/maven/AbstractScalaTestMojo.java
@@ -6,7 +6,21 @@
 import org.apache.maven.project.MavenProject;
 import org.codehaus.plexus.util.cli.*;
 
-import java.io.File;
+import static java.util.Collections.unmodifiableList;
+import static org.scalatest.tools.maven.MojoUtils.*;
+
+import java.io.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Map;
+
+import static java.util.Collections.singletonList;
+
+import java.net.MalformedURLException;
+import java.net.URLClassLoader;
+import java.net.URL;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.MalformedURLException;
@@ -388,7 +402,8 @@ private List<String> testClasspathElements() {
 
     // This is the configuration parameters shared by all concrete Mojo subclasses
     List<String> sharedConfiguration() {
-        return new ArrayList<String>() {{
+        return unmodifiableList(
+            new ArrayList<String>() {{
             addAll(runpath());
             addAll(config());
             addAll(tagsToInclude());
@@ -404,15 +419,15 @@ List<String> sharedConfiguration() {
             addAll(testsFiles());
             addAll(junitClasses());
             addAll(spanScaleFactor());
-        }};
+        }});
     }
 
     private List<String> config() {
         List<String> c = new ArrayList<String>();
         for(String pair : splitOnComma(config)){
             c.add("-D"+pair);
         }
-        return c;
+        return unmodifiableList(c);
     }
 
     private List<String> runpath() {
@@ -431,7 +446,7 @@ private List<String> tagsToExclude() {
     }
 
     private List<String> parallel() {
-        return parallel ? singletonList("-P") : Collections.<String>emptyList();
+        return parallel ? unmodifiableList(singletonList("-P")) : Collections.<String>emptyList();
     }
 
     //
@@ -459,7 +474,7 @@ private List<String> suites() {
                 }
             }
         }
-        return list;
+        return unmodifiableList(list);
     }
 
     //
@@ -526,7 +541,7 @@ private List<String> tests() {
         for (String test: splitOnComma(tests)) {
             addTest(list, test);
         }
-        return list;
+        return unmodifiableList(list);
     }
 
     private List<String> spanScaleFactor() {
@@ -535,7 +550,7 @@ private List<String> spanScaleFactor() {
             list.add("-F");
             list.add(spanScaleFactor + "");
         }
-        return list;
+        return unmodifiableList(list);
     }
 
     private List<String> suffixes() {
@@ -567,7 +582,7 @@ private List<String> testsFiles() {
                 list.add(param);
             }
         }
-        return list;
+        return unmodifiableList(list);
     }
 
     private List<String> junitClasses() {
diff --git a/src/main/java/org/scalatest/tools/maven/MojoUtils.java b/src/main/java/org/scalatest/tools/maven/MojoUtils.java
@@ -1,11 +1,14 @@
 package org.scalatest.tools.maven;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.ArrayList;
 import java.io.File;
 
 import static org.apache.commons.lang3.StringUtils.isEmpty;
 
+import static java.util.Collections.unmodifiableList;
+
 /**
  * Provides internal utilities for the Mojo's operations.
  * 
@@ -47,7 +50,7 @@ public String f(String in){
     }
 
     // sideeffect!
-    private static void createIfNotExists(File dir) {
+    private static synchronized void createIfNotExists(File dir) {
         if(!dir.exists() && !dir.mkdirs()){
             throw new IllegalStateException("Cannot create directory " + dir);
         }
@@ -70,7 +73,7 @@ static List<String> compoundArg(String name, String...strings){
             }
             list.add(a);
         }
-        return list;
+        return unmodifiableList(list);
     }
 
     static List<String> suiteArg(String name, String commaSeparated) {
@@ -79,7 +82,7 @@ static List<String> suiteArg(String name, String commaSeparated) {
             list.add(name);
             list.add(param);
         }
-        return list;
+        return unmodifiableList(list);
     }
 
     static List<String> reporterArg(String name, String commaSeparated, F map) {
@@ -94,7 +97,7 @@ static List<String> reporterArg(String name, String commaSeparated, F map) {
                 r.add(map.f(split[1]));
             }
         }
-        return r;
+        return unmodifiableList(r);
     }
 
     //
@@ -105,13 +108,13 @@ static List<String> reporterArg(String name, String commaSeparated, F map) {
     static List<String> splitOnComma(String cs) {
         List<String> args = new ArrayList<String>();
         if (cs == null) {
-            return args;
+            return unmodifiableList(args);
         } else {
             String[] split = cs.split("(?<!\\\\),");
             for (String arg : split) {
                 args.add(arg.trim().replaceAll("\\\\,", ","));
             }
-            return args;
+            return unmodifiableList(args);
         }
     }
 
diff --git a/src/main/java/org/scalatest/tools/maven/TestMojo.java b/src/main/java/org/scalatest/tools/maven/TestMojo.java
@@ -7,6 +7,7 @@
 import java.io.File;
 import java.util.Collections;
 import static java.util.Collections.singletonList;
+import static java.util.Collections.unmodifiableList;
 import java.util.List;
 import java.util.ArrayList;
 
@@ -21,6 +22,7 @@
  * @author Bill Venners
  * @phase test
  * @goal test
+ * @threadSafe
  */
 public class TestMojo extends AbstractScalaTestMojo {
 
@@ -124,15 +126,15 @@ String[] configuration() {
     // These private methods create the relevant portion of the command line
     // to pass to Runner based on the corresponding Maven configuration parameter.
     private List<String> stdout() {
-        return singletonList(stdout == null ? "-o" : "-o" + stdout);
+        return unmodifiableList(singletonList(stdout == null ? "-o" : "-o" + stdout));
     }
 
     private List<String> stderr() {
-        return stderr == null ? Collections.<String>emptyList() : singletonList("-e" + stderr);
+        return stderr == null ? Collections.<String>emptyList() : unmodifiableList(singletonList("-e" + stderr));
     }
 
     private List<String> filereports() {
-        return reporterArg("-f", filereports, fileRelativeTo(reportsDirectory));
+        return unmodifiableList(reporterArg("-f", filereports, fileRelativeTo(reportsDirectory)));
     }
 
     private List<String> htmlreporters() {
@@ -151,7 +153,7 @@ private List<String> htmlreporters() {
                 }
             }
         }
-        return r;
+        return unmodifiableList(r);
     }
 
     private List<String> reporters() {

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,21 @@`
`6`	`6`	`import org.apache.maven.project.MavenProject;`
`7`	`7`	`import org.codehaus.plexus.util.cli.*;`
`8`	`8`
`9`		`-import java.io.File;`
	`9`	`+import static java.util.Collections.unmodifiableList;`
	`10`	`+import static org.scalatest.tools.maven.MojoUtils.*;`
	`11`	`+`
	`12`	`+import java.io.*;`
	`13`	`+import java.util.Arrays;`
	`14`	`+import java.util.Collections;`
	`15`	`+import java.util.List;`
	`16`	`+import java.util.ArrayList;`
	`17`	`+import java.util.Map;`
	`18`	`+`
	`19`	`+import static java.util.Collections.singletonList;`
	`20`	`+`
	`21`	`+import java.net.MalformedURLException;`
	`22`	`+import java.net.URLClassLoader;`
	`23`	`+import java.net.URL;`
`10`	`24`	`import java.lang.reflect.InvocationTargetException;`
`11`	`25`	`import java.lang.reflect.Method;`
`12`	`26`	`import java.net.MalformedURLException;`
`@@ -388,7 +402,8 @@ private List<String> testClasspathElements() {`
`388`	`402`
`389`	`403`	`// This is the configuration parameters shared by all concrete Mojo subclasses`
`390`	`404`	`List<String> sharedConfiguration() {`
`391`		`- return new ArrayList<String>() {{`
	`405`	`+ return unmodifiableList(`
	`406`	`+ new ArrayList<String>() {{`
`392`	`407`	`addAll(runpath());`
`393`	`408`	`addAll(config());`
`394`	`409`	`addAll(tagsToInclude());`
`@@ -404,15 +419,15 @@ List<String> sharedConfiguration() {`
`404`	`419`	`addAll(testsFiles());`
`405`	`420`	`addAll(junitClasses());`
`406`	`421`	`addAll(spanScaleFactor());`
`407`		`- }};`
	`422`	`+ }});`
`408`	`423`	`}`
`409`	`424`
`410`	`425`	`private List<String> config() {`
`411`	`426`	`List<String> c = new ArrayList<String>();`
`412`	`427`	`for(String pair : splitOnComma(config)){`
`413`	`428`	`c.add("-D"+pair);`
`414`	`429`	`}`
`415`		`- return c;`
	`430`	`+ return unmodifiableList(c);`
`416`	`431`	`}`
`417`	`432`
`418`	`433`	`private List<String> runpath() {`
`@@ -431,7 +446,7 @@ private List<String> tagsToExclude() {`
`431`	`446`	`}`
`432`	`447`
`433`	`448`	`private List<String> parallel() {`
`434`		`- return parallel ? singletonList("-P") : Collections.<String>emptyList();`
	`449`	`+ return parallel ? unmodifiableList(singletonList("-P")) : Collections.<String>emptyList();`
`435`	`450`	`}`
`436`	`451`
`437`	`452`	`//`
`@@ -459,7 +474,7 @@ private List<String> suites() {`
`459`	`474`	`}`
`460`	`475`	`}`
`461`	`476`	`}`
`462`		`- return list;`
	`477`	`+ return unmodifiableList(list);`
`463`	`478`	`}`
`464`	`479`
`465`	`480`	`//`
`@@ -526,7 +541,7 @@ private List<String> tests() {`
`526`	`541`	`for (String test: splitOnComma(tests)) {`
`527`	`542`	`addTest(list, test);`
`528`	`543`	`}`
`529`		`- return list;`
	`544`	`+ return unmodifiableList(list);`
`530`	`545`	`}`
`531`	`546`
`532`	`547`	`private List<String> spanScaleFactor() {`
`@@ -535,7 +550,7 @@ private List<String> spanScaleFactor() {`
`535`	`550`	`list.add("-F");`
`536`	`551`	`list.add(spanScaleFactor + "");`
`537`	`552`	`}`
`538`		`- return list;`
	`553`	`+ return unmodifiableList(list);`
`539`	`554`	`}`
`540`	`555`
`541`	`556`	`private List<String> suffixes() {`
`@@ -567,7 +582,7 @@ private List<String> testsFiles() {`
`567`	`582`	`list.add(param);`
`568`	`583`	`}`
`569`	`584`	`}`
`570`		`- return list;`
	`585`	`+ return unmodifiableList(list);`
`571`	`586`	`}`
`572`	`587`
`573`	`588`	`private List<String> junitClasses() {`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,14 @@`
`1`	`1`	`package org.scalatest.tools.maven;`
`2`	`2`
	`3`	`+import java.util.Collections;`
`3`	`4`	`import java.util.List;`
`4`	`5`	`import java.util.ArrayList;`
`5`	`6`	`import java.io.File;`
`6`	`7`
`7`	`8`	`import static org.apache.commons.lang3.StringUtils.isEmpty;`
`8`	`9`
	`10`	`+import static java.util.Collections.unmodifiableList;`
	`11`	`+`
`9`	`12`	`/**`
`10`	`13`	`* Provides internal utilities for the Mojo's operations.`
`11`	`14`	`*`
`@@ -47,7 +50,7 @@ public String f(String in){`
`47`	`50`	`}`
`48`	`51`
`49`	`52`	`// sideeffect!`
`50`		`- private static void createIfNotExists(File dir) {`
	`53`	`+ private static synchronized void createIfNotExists(File dir) {`
`51`	`54`	`if(!dir.exists() && !dir.mkdirs()){`
`52`	`55`	`throw new IllegalStateException("Cannot create directory " + dir);`
`53`	`56`	`}`
`@@ -70,7 +73,7 @@ static List<String> compoundArg(String name, String...strings){`
`70`	`73`	`}`
`71`	`74`	`list.add(a);`
`72`	`75`	`}`
`73`		`- return list;`
	`76`	`+ return unmodifiableList(list);`
`74`	`77`	`}`
`75`	`78`
`76`	`79`	`static List<String> suiteArg(String name, String commaSeparated) {`
`@@ -79,7 +82,7 @@ static List<String> suiteArg(String name, String commaSeparated) {`
`79`	`82`	`list.add(name);`
`80`	`83`	`list.add(param);`
`81`	`84`	`}`
`82`		`- return list;`
	`85`	`+ return unmodifiableList(list);`
`83`	`86`	`}`
`84`	`87`
`85`	`88`	`static List<String> reporterArg(String name, String commaSeparated, F map) {`
`@@ -94,7 +97,7 @@ static List<String> reporterArg(String name, String commaSeparated, F map) {`
`94`	`97`	`r.add(map.f(split[1]));`
`95`	`98`	`}`
`96`	`99`	`}`
`97`		`- return r;`
	`100`	`+ return unmodifiableList(r);`
`98`	`101`	`}`
`99`	`102`
`100`	`103`	`//`
`@@ -105,13 +108,13 @@ static List<String> reporterArg(String name, String commaSeparated, F map) {`
`105`	`108`	`static List<String> splitOnComma(String cs) {`
`106`	`109`	`List<String> args = new ArrayList<String>();`
`107`	`110`	`if (cs == null) {`
`108`		`- return args;`
	`111`	`+ return unmodifiableList(args);`
`109`	`112`	`} else {`
`110`	`113`	`String[] split = cs.split("(?<!\\\\),");`
`111`	`114`	`for (String arg : split) {`
`112`	`115`	`args.add(arg.trim().replaceAll("\\\\,", ","));`
`113`	`116`	`}`
`114`		`- return args;`
	`117`	`+ return unmodifiableList(args);`
`115`	`118`	`}`
`116`	`119`	`}`
`117`	`120`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@`
`7`	`7`	`import java.io.File;`
`8`	`8`	`import java.util.Collections;`
`9`	`9`	`import static java.util.Collections.singletonList;`
	`10`	`+import static java.util.Collections.unmodifiableList;`
`10`	`11`	`import java.util.List;`
`11`	`12`	`import java.util.ArrayList;`
`12`	`13`
`@@ -21,6 +22,7 @@`
`21`	`22`	`* @author Bill Venners`
`22`	`23`	`* @phase test`
`23`	`24`	`* @goal test`
	`25`	`+ * @threadSafe`
`24`	`26`	`*/`
`25`	`27`	`public class TestMojo extends AbstractScalaTestMojo {`
`26`	`28`
`@@ -124,15 +126,15 @@ String[] configuration() {`
`124`	`126`	`// These private methods create the relevant portion of the command line`
`125`	`127`	`// to pass to Runner based on the corresponding Maven configuration parameter.`
`126`	`128`	`private List<String> stdout() {`
`127`		`- return singletonList(stdout == null ? "-o" : "-o" + stdout);`
	`129`	`+ return unmodifiableList(singletonList(stdout == null ? "-o" : "-o" + stdout));`
`128`	`130`	`}`
`129`	`131`
`130`	`132`	`private List<String> stderr() {`
`131`		`- return stderr == null ? Collections.<String>emptyList() : singletonList("-e" + stderr);`
	`133`	`+ return stderr == null ? Collections.<String>emptyList() : unmodifiableList(singletonList("-e" + stderr));`
`132`	`134`	`}`
`133`	`135`
`134`	`136`	`private List<String> filereports() {`
`135`		`- return reporterArg("-f", filereports, fileRelativeTo(reportsDirectory));`
	`137`	`+ return unmodifiableList(reporterArg("-f", filereports, fileRelativeTo(reportsDirectory)));`
`136`	`138`	`}`
`137`	`139`
`138`	`140`	`private List<String> htmlreporters() {`
`@@ -151,7 +153,7 @@ private List<String> htmlreporters() {`
`151`	`153`	`}`
`152`	`154`	`}`
`153`	`155`	`}`
`154`		`- return r;`
	`156`	`+ return unmodifiableList(r);`
`155`	`157`	`}`
`156`	`158`
`157`	`159`	`private List<String> reporters() {`